Skip to content

feat(kustomize): use LabelTransformer instead of deprecated commonLabels #1017

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

andyatmiami
Copy link
Contributor

@andyatmiami andyatmiami commented Apr 14, 2025

Description

This is a "piece" of a more comprehensive/interesting PR:

Unfortunately, that PR has grown wildly unwieldy in its size - and immediate feedback received was to try to break it into smaller pieces - so consider this one piece!

The ulitmate goal here on this targetted PR is two-fold:

  • standardization irrespective of image build "flavour" our kustomize labelling
  • get rid of following warning:
$ kubectl kustomize jupyter/minimal/ubi9-python-3.11/kustomize/base
# Warning: 'commonLabels' is deprecated. Please use 'labels' instead. Run 'kustomize edit fix' to update your Kustomization automatically.
...

No actual changes are introduced in this PR - simply leveraging the LabelTransformer to accomplish what commonLabels was previously doing.

Related-to: https://issues.redhat.com/browse/RHOAIENG-23291

How Has This Been Tested?

The following command (replacing the directory as appropriate) can be used to have kustomize process our manifests:

  • kubectl kustomize jupyter/minimal/ubi9-python-3.11/kustomize/base

For verification purposes - one can then simply compare the output of kustomize on this branch with that of main.

Here is a simple script that can do the verifications FOR YOU
labeltransformer-verify.sh.txt

  • remove .txt extension and make sure file executable
  • run file in its own directory
    • the script creates a couple transient directories... easier to keep this file in its own folder to then just safely blow away/clean everything up
  • do note that a specific version of kustomize (5.0.3) is downloaded and used by this script as later versions have a more strict parser
  • rudimentary support provided for MacOS Silicone and Linux AMD64 - albeit the Linux support was not actually tested.

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

@openshift-ci openshift-ci bot requested review from dibryant and jstourac April 14, 2025 16:17
Copy link
Contributor

openshift-ci bot commented Apr 14, 2025

[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 jstourac for approval. For more information see the 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:

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

@openshift-ci openshift-ci bot added the size/l label Apr 14, 2025
@jiridanek
Copy link
Member

jiridanek commented Apr 14, 2025

( I will share a script here shortly that can do this sequentially both for my own selfish verification needs as well as to help the review )

Great, when you manage to run it and it finds no changes, you can expect automatic lgtm from me.

Meanwhile, I tried to run kustomize on the project manifests

~/Downloads/kustomize build /Users/jdanek/IdeaProjects/notebooks/manifests/base
# Warning: 'vars' is deprecated. Please use 'replacements' instead. [EXPERIMENTAL] Run 'kustomize edit fix' to update > your Kustomization automatically.
Error: error unmarshaling JSON: while decoding JSON: json: unknown field "apiGroup"

and

~/Downloads/kustomize version
v5.5.0

not sure what I'm doing wrong, but will look tomorrow

@andyatmiami
Copy link
Contributor Author

andyatmiami commented Apr 14, 2025

Interesting observation @jiridanek ... i will look into it more.. but fwiw - I do not use standalone kustomize - but rather the kustomize that is bundled with kubectl...

kubectl kustomize .... is how I invoke (not that it should matter ideally...)

for now, to play around.. maybe avoid the manifests directory 😇 I just noticed as part of this targetted change.. that commonLabels was in the manifests/ directory... so aside from making the change - I haven't til now been building in that directory... more details to follow assuredly from me!

UPDATE

image

@andyatmiami andyatmiami force-pushed the fix/kustomize-labeltransformer branch from aacfa6c to 272f4ce Compare April 14, 2025 16:35
@openshift-ci openshift-ci bot added size/l and removed size/l labels Apr 14, 2025
@andyatmiami andyatmiami force-pushed the fix/kustomize-labeltransformer branch from 272f4ce to 65ed7d8 Compare April 14, 2025 20:05
@openshift-ci openshift-ci bot added size/l and removed size/l labels Apr 14, 2025
This is a "piece" of a more comprehensive/interesting PR:
- opendatahub-io#1015

Unfortunately, that PR has grown wildly unwieldy in its size - and immediate feedback received was to try to break it into smaller pieces - so consider this one piece!

The ulitmate goal here on this targetted PR is two-fold:
- standardization irrespective of image build "flavour" our kustomize labelling
- get rid of following warning:

```
$ kubectl kustomize jupyter/minimal/ubi9-python-3.11/kustomize/base
...
```

No actual changes are introduced in this PR - simply leveraging the `LabelTransformer` to accomplish what `commonLabels` was previously doing.

Related-to: https://issues.redhat.com/browse/RHOAIENG-23291
@andyatmiami andyatmiami force-pushed the fix/kustomize-labeltransformer branch from 65ed7d8 to f4a1c0a Compare April 14, 2025 20:11
@openshift-ci openshift-ci bot added size/l and removed size/l labels Apr 14, 2025
@andyatmiami
Copy link
Contributor Author

Output of running my ./labeltransformer-verify.sh script (on MacOS)

➜ labeltransformer-verify/ $ ./labeltransformer-verify.sh 
Cloning into 'original'...
remote: Enumerating objects: 521, done.
remote: Counting objects: 100% (521/521), done.
remote: Compressing objects: 100% (339/339), done.
remote: Total 521 (delta 157), reused 349 (delta 118), pack-reused 0 (from 0)
Receiving objects: 100% (521/521), 36.27 MiB | 35.04 MiB/s, done.
Resolving deltas: 100% (157/157), done.
Cloning into 'modified'...
remote: Enumerating objects: 532, done.
remote: Counting objects: 100% (532/532), done.
remote: Compressing objects: 100% (328/328), done.
remote: Total 532 (delta 169), reused 390 (delta 140), pack-reused 0 (from 0)
Receiving objects: 100% (532/532), 36.27 MiB | 34.68 MiB/s, done.
Resolving deltas: 100% (169/169), done.
--2025-04-14 17:00:31--  https://github.com/kubernetes-sigs/kustomize/releases/download/kustomize/v5.0.3/kustomize_v5.0.3_darwin_arm64.tar.gz
Resolving github.com (github.com)... 140.82.112.4
Connecting to github.com (github.com)|140.82.112.4|:443... connected.
HTTP request sent, awaiting response... 302 Found
Location: https://objects.githubusercontent.com/github-production-release-asset-2e65be/133067498/b1bbbb22-6703-43b7-a8a6-2f4ebbc43d50?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=releaseassetproduction%2F20250414%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20250414T210031Z&X-Amz-Expires=300&X-Amz-Signature=74281c14456361fe8ae55ecf3545152084219fad0736d393bc25f2a958404b76&X-Amz-SignedHeaders=host&response-content-disposition=attachment%3B%20filename%3Dkustomize_v5.0.3_darwin_arm64.tar.gz&response-content-type=application%2Foctet-stream [following]
--2025-04-14 17:00:31--  https://objects.githubusercontent.com/github-production-release-asset-2e65be/133067498/b1bbbb22-6703-43b7-a8a6-2f4ebbc43d50?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=releaseassetproduction%2F20250414%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20250414T210031Z&X-Amz-Expires=300&X-Amz-Signature=74281c14456361fe8ae55ecf3545152084219fad0736d393bc25f2a958404b76&X-Amz-SignedHeaders=host&response-content-disposition=attachment%3B%20filename%3Dkustomize_v5.0.3_darwin_arm64.tar.gz&response-content-type=application%2Foctet-stream
Resolving objects.githubusercontent.com (objects.githubusercontent.com)... 185.199.108.133, 185.199.110.133, 185.199.109.133, ...
Connecting to objects.githubusercontent.com (objects.githubusercontent.com)|185.199.108.133|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: 10470691 (10.0M) [application/octet-stream]
Saving to: ‘./kustomize-5.0.3.tar.gz’

./kustomize-5.0.3.tar.gz                           100%[================================================================================================================>]   9.99M  47.3MB/s    in 0.2s    

2025-04-14 17:00:31 (47.3 MB/s) - ‘./kustomize-5.0.3.tar.gz’ saved [10470691/10470691]

x kustomize
####
image: runtimes/rocm-tensorflow/ubi9-python-3.11/kustomize/base
####
**SUCCESS**: kustomize output identical


####
image: runtimes/rocm-pytorch/ubi9-python-3.11/kustomize/base
####
**SUCCESS**: kustomize output identical


####
image: runtimes/minimal/ubi9-python-3.11/kustomize/base
####
**SUCCESS**: kustomize output identical


####
image: runtimes/tensorflow/ubi9-python-3.11/kustomize/base
####
**SUCCESS**: kustomize output identical


####
image: runtimes/pytorch/ubi9-python-3.11/kustomize/base
####
**SUCCESS**: kustomize output identical


####
image: runtimes/datascience/ubi9-python-3.11/kustomize/base
####
**SUCCESS**: kustomize output identical


####
image: rstudio/rhel9-python-3.11/kustomize/base
####
**SUCCESS**: kustomize output identical


####
image: rstudio/c9s-python-3.11/kustomize/base
####
**SUCCESS**: kustomize output identical


####
image: codeserver/ubi9-python-3.11/kustomize/base
####
**SUCCESS**: kustomize output identical


####
image: manifests/overlays/additional
####
# Warning: 'vars' is deprecated. Please use 'replacements' instead. [EXPERIMENTAL] Run 'kustomize edit fix' to update your Kustomization automatically.
# Warning: 'vars' is deprecated. Please use 'replacements' instead. [EXPERIMENTAL] Run 'kustomize edit fix' to update your Kustomization automatically.
**SUCCESS**: kustomize output identical


####
image: manifests/base
####
# Warning: 'vars' is deprecated. Please use 'replacements' instead. [EXPERIMENTAL] Run 'kustomize edit fix' to update your Kustomization automatically.
# Warning: 'vars' is deprecated. Please use 'replacements' instead. [EXPERIMENTAL] Run 'kustomize edit fix' to update your Kustomization automatically.
**SUCCESS**: kustomize output identical


####
image: jupyter/trustyai/ubi9-python-3.11/kustomize/base
####
**SUCCESS**: kustomize output identical


####
image: jupyter/minimal/ubi9-python-3.11/kustomize/base
####
**SUCCESS**: kustomize output identical


####
image: jupyter/tensorflow/ubi9-python-3.11/kustomize/base
####
**SUCCESS**: kustomize output identical


####
image: jupyter/rocm/tensorflow/ubi9-python-3.11/kustomize/base
####
**SUCCESS**: kustomize output identical


####
image: jupyter/rocm/pytorch/ubi9-python-3.11/kustomize/base
####
**SUCCESS**: kustomize output identical


####
image: jupyter/pytorch/ubi9-python-3.11/kustomize/base
####
**SUCCESS**: kustomize output identical


####
image: jupyter/datascience/ubi9-python-3.11/kustomize/base
####
**SUCCESS**: kustomize output identical



@openshift-ci openshift-ci bot added size/l and removed size/l labels Apr 14, 2025
@jstourac
Copy link
Member

jstourac commented Apr 15, 2025

@andyatmiami I have a feeling we are duplicating our work here? #996

Also regarding the kustomize output comparison, I have something in my wip branch too for the followup of my raised 996 PR.

@jiridanek
Copy link
Member

I decided not to say anything before because Andy is quite good at pushing change forward. Better than you and me, if I say so.

@jiridanek
Copy link
Member

And this is nice isolated and focused change pr that can just slip in nicely, so why not just merge it and skip all the polite social chatter about being so sorry to have missed your or, and so on :p

@jiridanek
Copy link
Member

~/Downloads/kustomize version
v5.5.0

I knew I needed older kustomize, that's why I downloaded 5.5 when I have 5.6 on my system. Just completely did not realize I need very much older kustomize.

@jiridanek
Copy link
Member

image

/lgtm

@jiridanek
Copy link
Member

/override ci/prow/runtimes-ubi9-e2e-tests
/override ci/prow/rocm-runtimes-ubi9-e2e-tests
/override ci/prow/rocm-runtime-tf-ubi9-python-3-11-pr-image-mirror

Copy link
Contributor

openshift-ci bot commented Apr 15, 2025

@jiridanek: Overrode contexts on behalf of jiridanek: ci/prow/rocm-runtime-tf-ubi9-python-3-11-pr-image-mirror, ci/prow/rocm-runtimes-ubi9-e2e-tests, ci/prow/runtimes-ubi9-e2e-tests

In response to this:

/override ci/prow/runtimes-ubi9-e2e-tests
/override ci/prow/rocm-runtimes-ubi9-e2e-tests
/override ci/prow/rocm-runtime-tf-ubi9-python-3-11-pr-image-mirror

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@atheo89
Copy link
Member

atheo89 commented Apr 15, 2025

Hey folks, I just reviewed this PR as well as Jan's. I tend to prefer the approach to labels used in this PR. Jan, the other changes in your PR regarding placeholders still look relevant and seem like another piece of the ongoing transition toward a better kustomization declaration.
One note I’d add for this PR (since we’ve opened that workload): let’s update the images.name field on the kustomization yaml files to reflect the notebook/runtime name, as it currently uses the registry name, for better consistency.
Apart of this /lgtm

@jstourac
Copy link
Member

Hey folks, I just reviewed this PR as well as Jan's. I tend to prefer the approach to labels used in this PR.

My personal preference would be to go without the extra labels.yaml files, but unless I'm missing anything, this is probably just a preference thing, so let's see what others think.

One note I’d add for this PR (since we’ve opened that workload): let’s update the images.name field on the kustomization yaml files to reflect the notebook/runtime name, as it currently uses the registry name, for better consistency.

Definitely not as part of this PR but as a separate change.

Meanwhile, I tried to run kustomize on the project manifests

~/Downloads/kustomize build /Users/jdanek/IdeaProjects/notebooks/manifests/base
# Warning: 'vars' is deprecated. Please use 'replacements' instead. [EXPERIMENTAL] Run 'kustomize edit fix' to update > your Kustomization automatically.
Error: error unmarshaling JSON: while decoding JSON: json: unknown field "apiGroup"

This is due to the:

configurations:
  - params.yaml
  - commit.yaml

inclusion - once we get rid of the variables deprecation, kustomize 5.6 will work (as it works in the #996 PR already).

@jiridanek
Copy link
Member

I'm undecided regarding the extra files. I guess i slightly prefer to have extra files.

/lgtm

@jiridanek
Copy link
Member

Merging the other pr would be easier procedurally, no rebase needed... that's true

@andyatmiami
Copy link
Contributor Author

Closing this PR in favor of:

jiridanek pushed a commit to jiridanek/notebooks that referenced this pull request Jul 10, 2025
…flux/component-updates/component-update-odh-workbench-jupyter-datascience-cpu-py311-ubi9-n-v2-23

chore(deps): update odh-workbench-jupyter-datascience-cpu-py311-ubi9-n-v2-23 to 53a5394
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants