Fix kustomize5 warnings #2534
Fix kustomize5 warnings #2534
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold |
|
@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. |
|
Check also #2511 (comment) |
@juliusvonkohout gimme time till this weekend, i have narroed down the root cause to |
|
@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
force-pushed
the
fix_kustomize5_warnings
branch
from
1733004 to
4fc8120
Compare
|
/unhold |
|
@Electronic-Waste can I please get some help in identify the root cause of the test failures? |
|
/retest |
|
@juliusvonkohout all issues resolved as promised... i had to completely get rid of |
@Electronic-Waste all good now!! All tests have passed!! Can you please review this PR |
|
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 |
|
for |
|
@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 |
@juliusvonkohout same with |
|
|
|
same with |
|
@Electronic-Waste @anencore94 @andreyvelich can you please provide your approval. @juliusvonkohout has already reviewed the change |
manifests/v1beta1/installs/katib-cert-manager/kustomization.yaml
Outdated
Show resolved
Hide resolved
manifests/v1beta1/installs/katib-external-db/kustomization.yaml
Outdated
Show resolved
Hide resolved
|
New changes are detected. LGTM label has been removed. |
|
@andreyvelich @juliusvonkohout requested changes have been made |
|
/retest |
1 similar comment
|
/retest |
vikas-saxena02
force-pushed
the
fix_kustomize5_warnings
branch
from
cb87a98 to
34e7f5b
Compare
|
@andreyvelich @juliusvonkohout all checks have passed after rebasing with master. Please review again and provide your feedback/approval. |
vikas-saxena02
force-pushed
the
fix_kustomize5_warnings
branch
from
34e7f5b to
8558848
Compare
|
/retest |
1 similar comment
|
/retest |
@juliusvonkohout same summary as above 
|
|
/retest |
3 similar comments
|
/retest |
|
/retest |
|
/retest |
| annotations: | ||
| cert-manager.io/inject-ca-from: KATIB_NAMESPACE_PLACEHOLDER/KATIB_CERT_NAME_PLACEHOLDER |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
@tenzen-y any feedback will really be appreciated
|
@vikas-saxena02 do you want to continue here? @kunal-511 might also help |
@juliusvonkohout leave this with me... i am back on board after travel and sickness... just need some guidance from @tenzen-y for one of his concerns. |
Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> fixed ui-virtual-service.yaml Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> replaced vars with replacements Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> repalced vars with replacements Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> replaced vars with replacements Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> accepted change of image repo to ghcr as part of rebase Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> accepted change of image repo to ghcr as part of rebase Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> accepted change of image repo to ghcr as part of rebase Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> accepted change to ghcr as part of rebase Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> fixed ui-virtual-service.yaml Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> replaced vars with replacements Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> repalced vars with replacements Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> replaced vars with replacements Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> ran kustomizr edit fix Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> ran kustomizr edit fix Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> ran kustomize edit fix Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> ran kustomize edit fix Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> trying to get rdi of error Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> removing braces in kustomization.yaml Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> refactoring code to get ridd of missing value Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> fixing code alignment issue Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> removed patches from customization file Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> added placeholder values Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> replaced placeholder value Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> removed patches dir Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> added skipping of . as mentioned at https://kubectl.docs.kubernetes.io/references/kustomize/kustomization/replacements/#field-path-format Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> fixed wanring in katib-standalone-postgres/kustomization.yaml Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> restored comments as per Andrey's feedback Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> restored comments as per Andrey's feedback Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> fixed up comments Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> fixed up comments Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> restored comments as per Andrey's feedback Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> fixed up comments Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com> adding wheel installation Signed-off-by: Vikas Saxena <Vikas.Saxena.2006@gmail.com>
vikas-saxena02
force-pushed
the
fix_kustomize5_warnings
branch
from
1ec4e81 to
806fc87
Compare
|
/retest |
|
close this PR as #2549 has been raised |
4 participants
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: