error validating "STDIN" ... unknown field "labelSelector" in io.k8s.metacontroller.v1alpha1.CompositeController.spec.parentResource

[rubbercable]

using

• kubectl/client v.1.21.0, 1.23.0
• server v.1.20, 1.21.0, 1.22.0 (created cluster, local, minikube, kind)
• git commit: f2726c7
  $ while ! kustomize build example | kubectl apply -f -; do echo "Retrying to apply resources"; sleep 20; done

Results: 64 pods created, all green
...
deployment.apps/ml-pipeline-viewer-crd configured
deployment.apps/ml-pipeline-visualizationserver unchanged
error: error validating "STDIN": error validating data: ValidationError(CompositeController.spec.parentResource): unknown field "labelSelector" in io.k8s.metacontroller.v1alpha1.CompositeController.spec.parentResource; if you choose to ignore these errors, turn validation off with --validate=false
Retrying to apply resources

Looks like the manifests/apps/pipeline/upstream/base/installs/multi-user/pipelines-profile-controller/composite-controller.yaml

NOTE: not sure if this is relevant, but:
$ kustomize build example | head
2022/02/13 20:29:15 nil value at valueFrom.configMapKeyRef.name ignored in mutation attempt
2022/02/13 20:29:15 nil value at valueFrom.secretKeyRef.name ignored in mutation attempt
2022/02/13 20:29:15 well-defined vars that were never replaced: kfp-app-name,kfp-app-version
apiVersion: v1
...

[kimwnasptd]

I also bumped into this, thanks for creating the issue @rubbercable! This happened to me when developing the e2e tests, using a KinD cluster.

@zijianjoy do you know why this would be the case? We can bypass the following error with --validate=false, but is this the expected behavior? Is there an issue in kubeflow/pipelines that tracks this?

We might need to update the example/kustomization.yaml file based on the answers to the above.

[zijianjoy]

Can you try to remove the labelSelector field from composite-controller file? I think it is introduced in https://github.com/kubeflow/pipelines/pull/6537/files#diff-44808406c30a62940b549e0bdaabe1cbcac5358f53d12b762fb3ff3ced83d50f but the CRD doesn't have this field: https://github.com/kubeflow/pipelines/blob/master/manifests/kustomize/third-party/metacontroller/base/crd.yaml#L207-L219

Adding @juliusvonkohout for help on this change.

[juliusvonkohout]

The original problem is here kubeflow/pipelines#6537 (comment)
It was exposed by a CRD update of compositecontroller. Sadly @sebastien-prudhomme just stopped working on it.

I checked the documentation at https://metacontroller.github.io/metacontroller/api/compositecontroller.html#label-selector again and it is a bit confusing. They really expect the parentresource that has been created by the profiles-controller (the namespace) to already have a spec.selector field instead of specifying it in the compositecontroller and handling it for us automatically.

The parent resource of a CompositeController is assumed to have a spec.selector that matches the form of spec.selector in built-in resources like Deployment and StatefulSet (with matchLabels and/or matchExpressions).

So i misinterpreted this and it worked because there was preserveUnknownFields: true before.
And whoever introduced a compositecontroller in the first place made a mistake too

CompositeController expects to have full control over this resource. That is, you shouldn't define a CompositeController with a parent resource that already has its own controller. See [DecoratorController](https://metacontroller.github.io/metacontroller/api/decoratorcontroller.html) for an API that's better suited for adding behavior to existing resources.

This this will be fixed properly anyway in the coming weeks because a colleague of mine is implementing kubeflow/pipelines#7219 (comment) and especially the referenced merging of the two controller kubeflow/pipelines#6629 (comment)

So we just need to drop the field labelselector in the mean time and move on. i will create a pull request
#2133 should fix it. @rubbercable please verify it first.

[kimwnasptd]

Thanks for the detailed explanation! I'm missing some context on the CompositeController/MetaController, but I'll get up to speed.

In the mean time, until kubeflow/pipelines#7311 is merged and a new RC for pipelines' manifests is cut I'll add a --validate=false in the README's one-liner, as a temporary solution.

I'm expecting folks will be trying this out as well when installing Kubeflow, and I want to ensure the command works.

When the PR for the updated KFP manifests with the fix in kubeflow/pipelines#7311 is included, then let's remove the --validate=false

[juliusvonkohout]

Thanks for the detailed explanation! I'm missing some context on the CompositeController/MetaController, but I'll get up to speed.

Metacontroller/compositecontroller and kubeflow-pipelines-controller is an ugly hack that tries to add additional stuff to the profile namespace that has been created by the "real" kubeflow profile controller

In the mean time, until kubeflow/pipelines#7311 is merged and a new RC for pipelines' manifests is cut I'll add a --validate=false in the README's one-liner, as a temporary solution.

I'm expecting folks will be trying this out as well when installing Kubeflow, and I want to ensure the command works.

I installed the new CRD and had to remove the three lines.

When the PR for the updated KFP manifests with the fix in kubeflow/pipelines#7311 is included, then let's remove the --validate=false

Yes it is a trivial fix and should be merged soon.