Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why is seldon-app-svc of Deployment is set to the name of last PU of the inference graph #4302

Closed
zyxue opened this issue Aug 26, 2022 · 1 comment · Fixed by #4043
Closed
Labels

Comments

@zyxue
Copy link
Contributor

zyxue commented Aug 26, 2022

The seldon-app-svc key in Deployment labels keep being updated when creating service for each container:

deploy.ObjectMeta.Labels[containerServiceKey] = containerServiceValue
deploy.Spec.Selector.MatchLabels[containerServiceKey] = containerServiceValue
deploy.Spec.Template.ObjectMeta.Labels[containerServiceKey] = containerServiceValue

At the end, the value of that key becomes the name of the last container in the inference graph, which doesn't seem to make sense. E.g.

k get deployment.apps/my-predictor -o yaml 

will return something like

  12   │   labels:
  13   │     app: my-predictor
...
  17   │     seldon-app: my-predictor-svc
  18   │     seldon-app-svc: <NAME-OF-LAST-CONTAINER-IN-THE-INFERENCE-GRAPH>
...

Worse, it causes problems whenever I try to update the graph with an error like

  Description:             Deployment.apps "my-predictor" is invalid: spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"seldon-app":"my-predictor-svc", "seldon-app-svc":"<NAME-OF-LAST-CONTAINER-IN-THE-INFERENCE-GRAPH>", "seldon-deployment-id":"my-predictor-di"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable

In my case, I have just one Deployment, but many services are were created based on it.

I wonder why is seldon-app-svc set in the Deployment, is it a bug?

@zyxue zyxue added the bug label Aug 26, 2022
@ukclivecox
Copy link
Contributor

We create services that point to each container in the deployment so this may change if the names change on your SeldonDeployment spec.

We are working on a PR for the selector change as this can't be accomplished via an update as your saw as these are immutable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants