seldon-core icon indicating copy to clipboard operation
seldon-core copied to clipboard

Pod Level Labels and Annotations are not applied correctly

Open asahalyft opened this issue 4 years ago • 12 comments

Hello Team, I find the Pod Level propagation of Annotations and Labels to be very confusing. Below is an example how am I setting the annotations and labels at each and every granular levels e.g. at seldondeployment, at seldondeploymentspec, at predictor level and even at componentspec/spec/metadata.

Below is the seldon-deployment.yaml that's I am using:

apiVersion: machinelearning.seldon.io/v1alpha2
kind: SeldonDeployment
metadata:
  labels:
    sdep_label: modelexec-sdep-label
  annotations:
    sdep_annot: modelexec-sdep-annot
  name: sklearn-iris-classifier-sdep
spec:
  annotations:
    project_name: seldon-poc
    deployment_version: v1
    sdep_spec_annot: modelexec-sdep-spec-annot
  labels:
    sdep_spec_label: modelexec-sdep-spec-label
  name: sklearn-iris-classifier-sdep
  oauth_key: oauth-key
  oauth_secret: oauth-secret
  predictors:
  - componentSpecs:
    - spec:
        metadata:
          labels:
            pod_spec_label: modelexec-pod-spec-label
          annotations:
            pod_spec_annot: modelexec-pod-spec-annot
        containers:
        - image: docker.io/anindyas/sklearn-iris-classifier-seldon-rest:0.1
          imagePullPolicy: IfNotPresent
          name: sklearn-iris-classifier
          resources:
            requests:
              memory: 1Mi
        terminationGracePeriodSeconds: 5
    graph:
      children: []
      name: sklearn-iris-classifier
      endpoint:
        type: REST
      type: MODEL
    name: sklearn-iris-single-model-graph
    replicas: 3
    annotations:
      predictor_annot: modelexec-predictor-annot
    labels:
      version: v0.1
      predictor_label: modelexec-predictor-label

And the actual Pod yaml generated after deployment.

(base) asaha-mbp151:sklearn-iris-classifier asaha$ kubectl get pod seldon-9d15d82c03e66bcfdce7d3482408face-7f499f9bcc-f4nm5 -n asaha -o yaml
apiVersion: v1
kind: Pod
metadata:
  annotations:
    deployment_version: v1
    kubernetes.io/psp: eks.privileged
    modelexec-predictor-annot: ""
    project_name: seldon-poc
    prometheus.io/path: prometheus
    prometheus.io/port: "8000"
    prometheus.io/scrape: "true"
    sdep_spec_annot: modelexec-sdep-spec-annot
  creationTimestamp: "2020-04-06T08:40:29Z"
  generateName: seldon-9d15d82c03e66bcfdce7d3482408face-7f499f9bcc-
  labels:
    app: seldon-9d15d82c03e66bcfdce7d3482408face
    fluentd: "true"
    pod-template-hash: 7f499f9bcc
    predictor_label: modelexec-predictor-label
    seldon-app: seldon-a87c44d5424616f96ba52f7e12f7ba02
    seldon-app-sklearn-iris-classifier: seldon-5f78c9a59273e22b1a8ebb4853617fd5
    seldon-deployment-id: sklearn-iris-classifier-sdep-sklearn-iris-classifier-sdep
    version: v0.1
  name: seldon-9d15d82c03e66bcfdce7d3482408face-7f499f9bcc-f4nm5
  namespace: asaha
  ownerReferences:
  - apiVersion: apps/v1
    blockOwnerDeletion: true
    controller: true
    kind: ReplicaSet
    name: seldon-9d15d82c03e66bcfdce7d3482408face-7f499f9bcc
    uid: 45083120-77e2-11ea-a0e3-06c80776fe38
  resourceVersion: "15761"
  selfLink: /api/v1/namespaces/asaha/pods/seldon-9d15d82c03e66bcfdce7d3482408face-7f499f9bcc-f4nm5
  uid: 450bb0bc-77e2-11ea-a0e3-06c80776fe38
spec:
  containers:
  - env:

I see the Pod Annotations are generated from seldondeploymentspec section as in sdep_spec_annot: modelexec-sdep-spec-annot only because as per https://github.com/SeldonIO/seldon-core/blob/c2a324a162b61ddec64cda98eaab8b765ead600c/operator/apis/machinelearning.seldon.io/v1/seldondeployment_types.go#L203 - it supports only Annotations. Why not Labels as well?

Again, although PredictorSpec claim to support both Annotations and Labels here https://github.com/SeldonIO/seldon-core/blob/c2a324a162b61ddec64cda98eaab8b765ead600c/operator/apis/machinelearning.seldon.io/v1/seldondeployment_types.go#L208 but in reality only Labels are propagated to Pods as in predictor_label: modelexec-predictor-label but not Annotations. Also, I see the annotations from predictor section is ill processed as modelexec-predictor-annot: "" instead of predictor_annot: modelexec-predictor-annot.

Thirdly, predictors/componentSpecs/spec does not support any metadata at all although I provided annotations and labels there and SeldonPodSpec here https://github.com/SeldonIO/seldon-core/blob/c2a324a162b61ddec64cda98eaab8b765ead600c/operator/apis/machinelearning.seldon.io/v1/seldondeployment_types.go#L262 seems to have support for Metadata metav1.ObjectMeta but I cannot get anything set on the Pods.

It is confusing to get Pod annotations and labels from different places. Am I missing something here? It is important for us to set the Annotations correctly because based on that we have other Web hooks that run e.g. We set annotations for AWS IAM Roles which causes another Web Hook to be invoked that injects secrets and credentials to access S3. There are many Web Hooks like these in our infrastructure. I believe these should come from only SeldonPodSpec which is the closest resemblance to PodTemplate.

asahalyft avatar Apr 06 '20 08:04 asahalyft

Yes, we need to look further into cleaning up these areas post 1.1

ukclivecox avatar Apr 09 '20 15:04 ukclivecox

The annotations and labels should be added with precedence given for inner ones over outer. See:

https://github.com/SeldonIO/seldon-core/blob/82a8317f2c37dc19705e4c57a56e0447938e18d1/operator/controllers/seldondeployment_controller.go#L813-L846

Can you check in 1.1.0 release.

ukclivecox avatar Apr 23 '20 06:04 ukclivecox

The annotations and labels should be added with precedence given for inner ones over outer. See:

https://github.com/SeldonIO/seldon-core/blob/82a8317f2c37dc19705e4c57a56e0447938e18d1/operator/controllers/seldondeployment_controller.go#L813-L846

Can you check in 1.1.0 release.

Thank you. I will test it out.

asahalyft avatar Apr 24 '20 19:04 asahalyft

We are also experiencing the issue described above: "the annotations from predictor section is ill processed as modelexec-predictor-annot: "" instead of predictor_annot: modelexec-predictor-annot"

It is happening for all our annotation fields and unfortunately for some of them the Kubernetes annotations Key validation (which in reality is a value) fails which makes it impossible to generate the deployment.

example: whatever/schedule-actions: tests/data/schedule.json results in "tests/data/schedule.json": "" which K8s does not accept as a valid key

A fix would be much appreciated, so that we don't have to workaround too much :) thanks

ejianu avatar Apr 30 '20 09:04 ejianu

I noticed something similar - I'm only using top level labels (in the metadata block), but they don't get copied to the pod definition. Sounds like using a label at the predictor level would work (for now) - I guess those would get propagated to the metadata block in the pod definition.

pdstrnadJC avatar Jun 03 '20 00:06 pdstrnadJC

Yes, if you can use annotations at the predictor level for now. We plan to deprecate the spec ones at some point: https://github.com/SeldonIO/seldon-core/issues/1895

ukclivecox avatar Jun 03 '20 06:06 ukclivecox

@ejianu can you provide an example with the issue for 1.1.0?

ukclivecox avatar Jun 03 '20 06:06 ukclivecox

This is causing me issues in 1.8.0. Annotations at the predictor level get corrupted in the deployment which can cause it to create an invalid deployment object. If you add a predictor-level annotation:

foo: "bar"

into a SeldonDeployment, it will create a Deployment with the following two annotations in the pod spec:

foo: "bar"
bar: ""

which is wrong, but benign as long as your annotation value is a qualified name. If you add something like:

foo: "@bar"

instead, then the controller will fail to create the deployment because it incorrectly attempts to add an annotation named @bar:

{"level":"error","ts":1623231524.402164,"logger":"controller","msg":"Reconciler error","reconcilerGroup":"machinelearning.seldon.io","reconcilerKind":"SeldonDeployment","controller":"seldon-controller-manager","name":"annotation-test","namespace":"seldon","error":"Deployment.apps \"annotation-test-main-0-classifier\" is invalid: spec.template.annotations: Invalid value: \"@bar\": name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName',  or 'my.name',  or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')","stacktrace":"github.com/go-logr/zapr.(*zapLogger).Error\n\t/go/pkg/mod/github.com/go-logr/[email protected]/zapr.go:128\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler\n\t/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:246\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\t/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:218\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).worker\n\t/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:197\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1\n\t/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:155\nk8s.io/apimachinery/pkg/util/wait.BackoffUntil\n\t/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:156\nk8s.io/apimachinery/pkg/util/wait.JitterUntil\n\t/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:133\nk8s.io/apimachinery/pkg/util/wait.Until\n\t/go/pkg/mod/k8s.io/[email protected]/pkg/util/wait/wait.go:90"}

I'm trying to get annotations injected into the Service definition and that works great - no corruption, but unfortunately the deployment object never gets created.

ollystephens avatar Jun 09 '21 10:06 ollystephens

I encountered the same problem: labels in predictors not appeared in deployment!!

predictors:
  - name: default
    replicas: 1
    labels:
      atms-app-v: "2"
    graph:

the labels of this deployment as below:

> kubectl get deployments.apps  -n seldon test-seldon-default-0-ocr-classifier -o json | jq '.metadata.labels'
{
  "app": "test-seldon-default-0-ocr-classifier",
  "app.kubernetes.io/managed-by": "seldon-core",
  "fluentd": "true",
  "seldon-app": "test-seldon-default",
  "seldon-app-svc": "test-seldon-default-ocr-classifier",
  "seldon-deployment-contains-svcorch": "true",
  "seldon-deployment-id": "test-seldon",
  "seldon.io/model": "true",
  "version": "default"
}

so2bin avatar Nov 23 '21 02:11 so2bin

@ollystephens @so2bin can you provide a full manifest that can allow us to replicate? You can use the mock-classifier:1.11.2 as the image (or any of the test models in the docs). That way we'll be able to understand what the issue is, as a lot of the times annotations/labels are just defined in different ways in different areas.

axsaucedo avatar Dec 03 '21 10:12 axsaucedo

Just following up with this issue, would you be able to provide some of the info above?

axsaucedo avatar Jan 17 '22 09:01 axsaucedo

Needs to be validated

ukclivecox avatar Jun 27 '22 06:06 ukclivecox

Tested with 1.16.0 and labels in predictor worked as expected.

ukclivecox avatar Dec 27 '22 18:12 ukclivecox