Metadata fields of predictor's component spec are being pruned in k8s

[ivan-valkov]

Describe the bug

When deploying a seldon deployment the metadata fields of predictor's component spec do not get persisted in k8s.
This seems to occur only when using the CRDs with apiextensions.k8s.io/v1 as per the docs unknown fields will be pruned.
The metadata field here https://github.com/SeldonIO/seldon-core/blob/master/operator/apis/machinelearning.seldon.io/v1/seldondeployment_types.go#L292 is probably not recognized when generating the CRDs.

Possible solutions:

• Add x-kubernetes-preserve-unknown-fields: true to that field. This will eventually be deprecated so we should probably avoid it.
• Define all the properties we need for the metadata field in the properties: block. Probably there is a better way.
• Use something like https://github.com/kubernetes-sigs/controller-tools/blob/557da250b856e92ac1cf0895b6953060de72b7e1/pkg/crd/markers/validation.go#L233 to get these fields auto registered.

Is there a chance other fields are also affected and not only the SeldonPodSpec's metadata?

To reproduce

1. Deploy something like this:

{
  "kind": "SeldonDeployment",
  "apiVersion": "machinelearning.seldon.io/v1",
  "metadata": {
    "name": "iris",
    "namespace": "seldon",
    "labels": {
      "fluentd": "true"
    }
  },
  "spec": {
    "name": "iris",
    "predictors": [
      {
        "name": "default",
        "graph": {
          "name": "iris-container",
          "implementation": "SKLEARN_SERVER",
          "modelUri": "gs://seldon-models/sklearn/iris",
          "logger": {
            "mode": "all"
          }
        },
        "componentSpecs": [
          {
            "metadata": {
              "creationTimestamp": "2021-02-09T14:05:02Z",
              "labels": {
                "test": "123"
              }
            },
            "spec": {
              "containers": [
                {
                  "name": "iris-container",
                  "resources": {}
                }
              ]
            }
          }
        ],
        "replicas": 1,
        "engineResources": {},
        "svcOrchSpec": {},
        "traffic": 100
      }
    ],
    "annotations": {
      "seldon.io/engine-seldon-log-messages-externally": "true"
    },
    "protocol": "seldon"
  },
  "status": {}
} 

1. check the sdep in k8s -

➜ kubectl get sdep iris -nseldon | grep metadata -B 3
  name: iris
  predictors:
  - componentSpecs:
    - metadata: {}

Expected behaviour

The metadata field in the component spec should contain the original fields:

"metadata": {
              "creationTimestamp": "2021-02-09T14:05:02Z",
              "labels": {
                "test": "123"
              }
            }

Environment

• Cloud Provider: GKE
• Kubernetes Cluster Version: v1.18.15-gke.800"
• Deployed Seldon System Images: n/a (this is a k8s issue)

[ivan-valkov]

A follow up. Adding x-kubernetes-embedded-resource: true won't work for us because it makes the SeldonPodSpec require more fields that we probably don't want to start enforcing. These are apiVersion and kind. Example validation error from my dummy example:

error: error validating "/home/ivan/code/kubernetes/cmd/kubectl/test-foo.json": error validating data: [ValidationError(Foo.spec.predictors[0].componentSpecs[0]): missing required field "apiVersion" in io.k8s.samplecontroller.v1.Foo.spec.predictors.componentSpecs, ValidationError(Foo.spec.predictors[1].componentSpecs[0]): missing required field "kind" in io.k8s.samplecontroller.v1.Foo.spec.predictors.componentSpecs, ValidationError(Foo.spec.predictors[1].componentSpecs[0]): missing required field "apiVersion" in io.k8s.samplecontroller.v1.Foo.spec.predictors.componentSpecs];

I will look at other solutions.

[ivan-valkov]

Two fixes that I tested and work:

1. Using x-kubernetes-preserve-unknown-fields - master...ivan-valkov:deploypodspec-metadata-fix
2. Using a custom ObjectMeta struct - master...ivan-valkov:deploypodspec-metadata-fix-custommeta

1. does not change anything other than the validation of the resources.
2. changes the metadata field in the SeldonPodSpec which could potentially affect people who are depending on this field in their own projects. The changes to the operator here were minimal to make it work with the new metadata field.