Upgrade to use Kustomize v4+

[zijianjoy]

From the README of kubeflow/manifests: https://github.com/kubeflow/manifests#prerequisites, it is a prerequisite to use Kustomize v3.2.0 for building resources. However, Kustomize v4+ is available. Can we upgrade prerequisite to use new version of Kustomize?

One thing that needs to be changed is that: flag name load_restrictor is changed in Kustomize v4+, we need to make the following updates in docs and build commands:

Change from --load_restrictor=none to --load-restrictor LoadRestrictionsNone. Note the difference between _ and -.

[yanniszark]

@zijianjoy thanks a lot for opening this issue to switch our target version to latest Kustomize v4.
This was not in our plan for 1.3.0, the reason being that we didn't want to change too many things together.
But it can very well be in our plans for the point release or 1.4. Let's use this issue to track the effort.

[davidspek]

I am using Kustomize v4.0.5 for https://github.com/kubeflow-onprem/ArgoFlow and everything works as expected. The --load_restrictor flag is also not necessary except for the Katib manifests, which should be fixed and I have a PR open for this already. For the Argo installation I am not using load_restrictor (for Katib I am using my fixed version of the manifests) so this can be seen as proof that this flag is not necessary for deploying Kubeflow.

@zijianjoy One interesting anomaly I did encounter is that running kustomize build github.com/kubeflow/pipelines/manifests/kustomize/env/platform-agnostic-multi-user-pns works for kustomize 3.2.1 and 4.0.5, but not for 3.9.4 (all the other manifests did not have any issue with 3.9.4, the default for Argo CD v2). There was a breaking change in how remote bases are grabbed in kustomize between v3 and v4 (s3 isn't supported anymore), so this is something to potentially keep in mind (not that I think we should update to an outdated version though).

[berndverst]

This issue burns me over and over. I am used to installing the latest version of tools and I ran into this again trying to work out the latest release configs for Azure.

As far as I can tell 3.9.2+ are broken with Knative also:
kustomize build --load_restrictor=none common/knative/knative-eventing-install/base

errors with

... at path 'spec/template/metadata/labels': wrong Node Kind for spec.template.metadata.labels expected: MappingNode was AliasNode: value: {*ref_0}

The latest version that works for me is 3.9.1 or 3.8.10. I installed and tested about 20 different versions.

[berndverst]

FYI @davidspek with 4.0.5 Knative is also broken as is:

 ~/src/kubeflow/manifests-1.3-branch:  kustomize build --load-restrictor LoadRestrictionsNone common/knative/knative-eventing-install/base

wrong Node Kind for spec.template.metadata.labels expected: MappingNode was AliasNode: value: {*ref_0}

@yanniszark
I really think this is a release blocking issue because neither the latest 3.X version nor any 4.X versions of kustomize work. On the one hand we are telling people that kfdef is no longer the recommend tool, instead use kustomize, on the other hand we don't support any modern versions of kustomize.

[berndverst]

@yanniszark with the PR to remove pointers from the Knative YAML everything seems to be working fine.

I can generate everything with v4.0.5 by running from the example folder you provided:

kustomize build --load-restrictor LoadRestrictionsNone .

[davidspek]

@berndverst You are correct. I had forgotten about this one but I had already created a PR to fix this in the KNative manifests 4 days ago.

#1795

Also, you don't need the --load-restrictor LoadRestrictionsNone flag so you shouldn't be using it as it disables a security check in kustomize.

[berndverst]

@davidspek I noticed I apparently created 2 PRs that were dupes of yours when they were marked as dupes this morning sigh. At least we are getting those issues addressed now!

[berndverst]

@davidspek I found I needed the load restrictor flag for katib when I tried this yesterday. But I saw that this is also being addressed somewhere.

[davidspek]

@berndverst Indeed, Katig should be the only one. I am working with @andreyvelich to remove this requirement in kubeflow/katib#1498.

On my branch with these fixes you can run:
kustomize build github.com/davidspek/katib/manifests/v1beta1/installs/katib-with-kubeflow-cert-manager?ref=fix-manifests
And it will build the manifests directly, no need to download them locally.

[Bobgy]

I want to add another data point that sticking to kustomize 3.2.0 is not a good idea.

See kubernetes-sigs/kustomize#1500 (comment), starting from kubectl release 1.21, it will come with kustomize v3.9. In not far future, people will be able to use a relatively new version of kustomize from kubectl. That can be an onboarding experience improvement if kubectl is the only tool people need.

I think the urgency isn't so much considering the speed people upgrade kubernetes clusters, it could be enough that we start supporting later kustomize versions starting from Kubeflow 1.4.

[Bobgy]

@berndverst You are correct. I had forgotten about this one but I had already created a PR to fix this in the KNative manifests 4 days ago.

#1795

Also, you don't need the --load-restrictor LoadRestrictionsNone flag so you shouldn't be using it as it disables a security check in kustomize.

@davidspek I want to remind you why we decided to use load restrictor none, read https://bit.ly/kf_kustomize_v3. It was introduced mainly because we want people to build reusable patches that people can compose, instead of relying on overlays (which is inheritance model and people cannot compose).

So far, I think https://kubectl.docs.kubernetes.io/guides/config_management/components/ is a feature designed to solve this problem -- allow reusable composition of kustomization transformation without turning off security check.

but I haven't used it much and I don't see much of the community using it, I wonder how people think about it.

[davidspek]

@Bobgy I understand why the load-restrictor was useful, but currently only 1 component still needs it (and those manifests are better suited with overlays if you ask me). Indeed the components feature is something I ran into as well, but this cannot be used at this point because for some reason kustomize 3.2.1 is being used (the load restrictor can be using with newer version, and all the manifests build with newer versions).
The only reason the version of kustomize matters at this point is because the load-restrictor flag has changed in newer versions, and this flag is being used for all the manifests even though only the Katib manifests needs it (for now). If the load-restrictor flag is removed from the instructions, the kustomize version can also be removed.