Skip to content

Conversation

mtchoum1
Copy link

@mtchoum1 mtchoum1 commented May 29, 2025

https://issues.redhat.com/browse/RHOAIENG-11681

Description

update the kustomize to for each component i.e (notebook-controller, odh-notebook-controller)
using the command:
kustomize edit fix --vars
Execute the above command in each dir of the component.

How Has This Been Tested?

verify the new set of deployment manifests.
kustomize build <path>
verify this on a cluster using devflags:
inside the RHOAI operator, update the DSC (data science cluster) yaml, with Workbenches devflags included

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

Copy link

openshift-ci bot commented May 29, 2025

Hi @mtchoum1. Thanks for your PR.

I'm waiting for a opendatahub-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

Copy link
Member

@harshad16 harshad16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great start 💯
a small change is needed

@codecov-commenter
Copy link

codecov-commenter commented May 29, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 55.14%. Comparing base (3408b8c) to head (0f4a974).
Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #609      +/-   ##
==========================================
- Coverage   55.57%   55.14%   -0.44%     
==========================================
  Files          10       10              
  Lines        2789     2791       +2     
==========================================
- Hits         1550     1539      -11     
- Misses       1106     1126      +20     
+ Partials      133      126       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

commonAnnotations:
service.beta.openshift.io/inject-cabundle: "true"
configurations:
- kustomizeconfig.yaml
- kustomizeconfig.yaml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this indentation change would fail on notebook repository as we have the yaml indentation check in the CI there... but let's not bother with this here now since it's automatically generated

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sort of automatically generated; automatically formatted by tool that has different ideas what properly formatted yaml looks like, more like

@jstourac
Copy link
Member

@mtchoum1 thank you for these changes! For your convenience as I had some local changes on my fork already, I raised this PR #610 that contains a script that makes it quite easy to compare the results before and after your changes:

  1. Checkout the opendatahub-io/main (actually have the script from Add a script to run kustomize on our components to check all works #610)
  2. run the ./ci/kustomize.sh
  3. Checkout this feature branch
  4. run the ./ci/kustomize.sh
  5. Run the following to check the results:
$ meld /tmp/kustomize-M68zDmqKSW/kustomize-5.0.3-stdout.yaml /tmp/kustomize-tFgCzH6one/kustomize-5.0.3-stdout.yaml

$ meld /tmp/kustomize-M68zDmqKSW/kustomize-5.6.0-stdout.yaml /tmp/kustomize-tFgCzH6one/kustomize-5.6.0-stdout.yaml                                                     

$ meld /tmp/kustomize-M68zDmqKSW/kustomize-5.0.3-stderr.txt /tmp/kustomize-tFgCzH6one/kustomize-5.0.3-stderr.txt
  1. The stdout files are identical. Stderr files are now without any content. I am happy.

So I checked your branch with current main and there is no difference - just the warnings gone and we can now compile also with the latest kustomize version. So this is a big LGTM from myself.

Of course, it is very welcome to perform a check on a running cluster with this code being handled by an actual operator!

/lgtm

We don't need to do the following tests:

/override ci/prow/images

Copy link

openshift-ci bot commented May 30, 2025

@jstourac: Overrode contexts on behalf of jstourac: ci/prow/images

In response to this:

@mtchoum1 thank you for these changes! For your convenience as I had some local changes on my fork already, I raised this PR #610 that contains a script that makes it quite easy to compare the results before and after your changes:

  1. Checkout the opendatahub-io/main (actually have the script from Add a script to run kustomize on our components to check all works #610)
  2. run the ./ci/kustomize.sh
  3. Checkout this feature branch
  4. run the ./ci/kustomize.sh
  5. Run the following to check the results:
$ meld /tmp/kustomize-M68zDmqKSW/kustomize-5.0.3-stdout.yaml /tmp/kustomize-tFgCzH6one/kustomize-5.0.3-stdout.yaml

$ meld /tmp/kustomize-M68zDmqKSW/kustomize-5.6.0-stdout.yaml /tmp/kustomize-tFgCzH6one/kustomize-5.6.0-stdout.yaml                                                     

$ meld /tmp/kustomize-M68zDmqKSW/kustomize-5.0.3-stderr.txt /tmp/kustomize-tFgCzH6one/kustomize-5.0.3-stderr.txt
  1. The stdout files are identical. Stderr files are now without any content. I am happy.

So I checked your branch with current main and there is no difference - just the warnings gone and we can now compile also with the latest kustomize version. So this is a big LGTM from myself.

Of course, it is very welcome to perform a check on a running cluster with this code being handled by an actual operator!

/lgtm

We don't need to do the following tests:

/override ci/prow/images

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.

@jiridanek
Copy link
Member

/lgtm
too

Copy link
Member

@harshad16 harshad16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

very well done 💯
thanks for the work
Tested on fresh cluster, with devflags:

{"level":"info","ts":"2025-06-02T04:34:03Z","msg":"Executing action","controller":"workbenches","controllerGroup":"components.platform.opendatahub.io","controllerKind":"Workbenches","Workbenches":{"name":"default-workbenches"},"namespace":"","name":"default-workbenches","reconcileID":"9cd7803f-07ca-4d6f-a2ca-2958df57283e","action":"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/status/releases.(*Action).run-fm"}
{"level":"info","ts":"2025-06-02T04:34:03Z","msg":"Executing action","controller":"workbenches","controllerGroup":"components.platform.opendatahub.io","controllerKind":"Workbenches","Workbenches":{"name":"default-workbenches"},"namespace":"","name":"default-workbenches","reconcileID":"9cd7803f-07ca-4d6f-a2ca-2958df57283e","action":"github.com/opendatahub-io/opendatahub-operator/v2/internal/controller/components/workbenches.configureDependencies"}
{"level":"info","ts":"2025-06-02T04:34:03Z","msg":"Executing action","controller":"workbenches","controllerGroup":"components.platform.opendatahub.io","controllerKind":"Workbenches","Workbenches":{"name":"default-workbenches"},"namespace":"","name":"default-workbenches","reconcileID":"9cd7803f-07ca-4d6f-a2ca-2958df57283e","action":"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize.(*Action).run-fm"}
{"level":"info","ts":"2025-06-02T04:34:03Z","msg":"Executing action","controller":"auth","controllerGroup":"services.platform.opendatahub.io","controllerKind":"Auth","Auth":{"name":"auth"},"namespace":"","name":"auth","reconcileID":"4ba69cb2-9fee-48be-be66-0f5a058c8dd6","action":"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/reconciler.(*dynamicWatchAction).run-fm"}
{"level":"info","ts":"2025-06-02T04:34:05Z","msg":"Executing action","controller":"workbenches","controllerGroup":"components.platform.opendatahub.io","controllerKind":"Workbenches","Workbenches":{"name":"default-workbenches"},"namespace":"","name":"default-workbenches","reconcileID":"9cd7803f-07ca-4d6f-a2ca-2958df57283e","action":"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy.(*Action).run-fm"}

@harshad16
Copy link
Member

/retest-required

@openshift-ci openshift-ci bot added approved and removed size/l labels Jun 2, 2025
@jiridanek
Copy link
Member

/ok-to-test

@openshift-ci openshift-ci bot added size/l and removed size/l labels Jun 3, 2025
Copy link
Member

@harshad16 harshad16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the squashing
/lgtm
/approve

great work 🎖️

@openshift-ci openshift-ci bot added the lgtm label Jun 4, 2025
Copy link

openshift-ci bot commented Jun 4, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: harshad16

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

The pull request process is described 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

@harshad16 harshad16 merged commit 0247e40 into opendatahub-io:main Jun 4, 2025
6 of 8 checks passed
@jstourac
Copy link
Member

jstourac commented Jun 4, 2025

@mtchoum1 Now that we have this cleared out, should we activate the GHA to check the kustomize results for each PR as we do in notebooks repository? Any objections @harshad16 ? What to do for it was mentioned in #610 description.

I'm happy to do it but it looks like a nice easy and quick task to get some experience on.

@harshad16
Copy link
Member

Now that we have this cleared out, should we activate the GHA to check the kustomize results for each PR as we do in notebooks repository? Any objections ? What to do for it was mentioned in #610 description.

I'm happy to do it but it looks like a nice easy and quick task to get some experience on.

Excellent point, no objections 💯

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.

5 participants