Fix kustomize5 warnings

[vikas-saxena02]

What this PR does / why we need it:
This PR aims at fixing deprecation warnings while runing kustomize build on manifest

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes: kubeflow/manifests#2985

Checklist:

• Docs included if any changes are user facing

[google-oss-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign johnugeorge for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[vikas-saxena02]

/hold

[vikas-saxena02]

@juliusvonkohout I am currently facing this issue

Any help in fixing this will be great!!

[juliusvonkohout]

@juliusvonkohout I am currently facing this issue

Any help in fixing this will be great!!

then please just omit the change there. Only implement what is simple and gets you a zero difference (post diff -u) in output. Fixing just 80 % of the warnings is totally fine. But i think you have to rebase to master first.

[juliusvonkohout]

Check also #2511 (comment)

[vikas-saxena02]

@juliusvonkohout I am currently facing this issue
Any help in fixing this will be great!!

then please just omit the change there. Only implement what is simple and gets you a zero difference (post diff -u) in output. Fixing just 80 % of the warnings is totally fine. But i think you have to rebase to master first.

@juliusvonkohout gimme time till this weekend, i have narroed down the root cause to the fact that replacements gets applied before patches and hence the issue. I am working on this and confident I will be able to sort this out. I had a deployment at work due yesterday but now I am back to this.

[vikas-saxena02]

@juliusvonkohout finally some success... i need to further test for any regressions which i will do later today and rebase with master has been done

[vikas-saxena02]

/unhold

[vikas-saxena02]

@Electronic-Waste can I please get some help in identify the root cause of the test failures?

[vikas-saxena02]

/retest

[vikas-saxena02]

@juliusvonkohout all issues resolved as promised... i had to completely get rid of katib-cert-manager . Below is output of git diff -u

[vikas-saxena02]

@Electronic-Waste can I please get some help in identify the root cause of the test failures?

@Electronic-Waste all good now!! All tests have passed!! Can you please review this PR

[juliusvonkohout]

can you post a diff -u to compare old and new rendered manifests for each kustomization.yaml and make sure that there is no difference?

[vikas-saxena02]

can you post a diff -u to compare old and new rendered manifests for each kustomization.yaml and make sure that there is no difference?

sorry Julius, I am unable to understand what to do here... do you want to me to compare the yamls generated as output of kustomize build

[vikas-saxena02]

for katib-with-kubeflow

[vikas-saxena02]

@juliusvonkohout @andreyvelich I have come accrss a situation where diff -u on the new rendered manifests are resulting in some difference in output as shown below

This is due to the fact that I had to get rid of patches for katib-cert-manager to make replacements work and add the aanotation to the ../components/webhook/ itself in order for this field to be found. The reason it was not working with replacements as reaplacements get applied prior to patches. While this work perfectly for katib-with-kubeflow this extra field is showing up in other modes e.g. katib-standalone-postgres. However, the newly added annotations field is not being used elsewhere and does not sound like a breaking change to me. Any inputs will really be great.

[vikas-saxena02]

@juliusvonkohout @andreyvelich I have come accrss a situation where diff -u on the new rendered manifests are resulting in some difference in output as shown below

This is due to the fact that I had to get rid of patches for katib-cert-manager to make replacements work and add the aanotation to the ../components/webhook/ itself in order for this field to be found. The reason it was not working with replacements as reaplacements get applied prior to patches. While this work perfectly for katib-with-kubeflow this extra field is showing up in other modes e.g. katib-standalone-postgres. However, the newly added annotations field is not being used elsewhere and does not sound like a breaking change to me. Any inputs will really be great.

@juliusvonkohout same with katib-standalone, the recently added annotation is showing up in diff -u

[vikas-saxena02]

katib-openshift is good

[vikas-saxena02]

katib-leader-election has same annotation coming up in diff

same with katib-external-db

[vikas-saxena02]

@andreyvelich @juliusvonkohout requested changes have been made

[vikas-saxena02]

/retest

[vikas-saxena02]

/retest

[vikas-saxena02]

@andreyvelich @juliusvonkohout all checks have passed after rebasing with master. Please review again and provide your feedback/approval.

[vikas-saxena02]

/retest

[vikas-saxena02]

/retest

[vikas-saxena02]

@juliusvonkohout Short Summary:

• Not affected: katib-with-kubeflow, katib-cert-manager, katib-openshift
• affected (showing annotations): katib-external-db, katib-leader-election, katib-standalone-postgres, katib-standalone

@juliusvonkohout same summary as above

[vikas-saxena02]

/retest

[vikas-saxena02]

/retest

[vikas-saxena02]

/retest

[vikas-saxena02]

/retest

[tenzen-y]

We should add cert-manager related parameters in https://github.com/kubeflow/katib/tree/master/manifests/v1beta1/installs/katib-with-kubeflow since in Katib project, the cert-manager is not required dependencies. The certmanager is required dependencies only for Katib with Kubeflow.

[vikas-saxena02]

@tenzen-y Apologies, I just saw this. The reason I added this here was this annotation was previously applied with patches and with move to replacements from vars, the replacements get applied first before patches and hence the builds were failing. Given this annotation was not used elsewhere in katib-standalone and other modules I thought of adding it to the main webhook configuration. But I do not have background on Katib so I left it for review so I am open to ideas, can you suggest something that would be more ideal and I can make that change as part of this PR.

PS: this is my first time doing heavylifting work on Kustomize so yes there is some skill gap.