Skip to content
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

Upgrade kustomization files to Kustomize v5 #2352

Merged
merged 1 commit into from
Oct 21, 2024

Conversation

oksanabaza
Copy link
Contributor

Why are these changes needed?

This PR addresses the migration from Kustomize v4 to v5. Kuberay was generating warnings about deprecated parameters, which have now been updated to align with Kustomize v5 standards.

Related issue number

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • This PR is not tested :(

@oksanabaza oksanabaza changed the title Upgrade manifests kustomize v5 Upgrade kustomization files to Kustomize v5 Sep 4, 2024
@astefanutti
Copy link
Contributor

@kevin85421 could you please have a quick look?

@kevin85421
Copy link
Member

@kevin85421 could you please have a quick look?

Sure, sorry I didn’t know it was an urgent item for you all. I initially planned to review it after Ray Summit.

@kevin85421
Copy link
Member

@oksanabaza would you mind rebasing with the master branch? Thanks!

@kevin85421 kevin85421 self-assigned this Sep 18, 2024
Copy link
Member

@kevin85421 kevin85421 left a comment

Choose a reason for hiding this comment

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

I manually test deploying KubeRay operator by running:

  1. kubectl create -k ray-operator/config/default
  2. make deploy-with-webhooks

It works. @oksanabaza would you mind manually testing deploying KubeRay apiserver?

@kevin85421
Copy link
Member

It works. @oksanabaza would you mind manually testing deploying KubeRay apiserver?

Could you provide some details in the PR description to explain how you tested deploying the KubeRay API server? Thanks!

@oksanabaza
Copy link
Contributor Author

Hey @kevin85421 , I deployed using the following commands:

  1. kubectl create -k ray-operator/config/default
  2. make deploy-with-webhooks

Everything worked as expected

Also here are the steps I followed for manual testing:

  • Ran kustomize build . > original-v4.yaml on the original v4 manifests
  • Updated the manifests to use Kustomize v5 labels
  • Ran kustomize build . > updated-v5.yaml on the updated manifests
  • Compared the outputs using diff original-v4.yaml updated-v5.yaml
  • Verified that there were no differences and no warnings after the Kustomize build commands

Let me know if you need any more information

@oksanabaza
Copy link
Contributor Author

@kevin85421 I noticed that the KUSTOMIZE_VERSION wasn't updated, so I moved from version 3.8.7 to v5.4.3. After the upgrade, I tested it with a kind cluster using the following commands:

make install for the insecure setup
make install-secure for the secure setup
kubectl apply -k apiserver/deploy/local/secure for the secure configuration
kubectl apply -k apiserver/deploy/local/insecure for the insecure configuration

@kevin85421
Copy link
Member

cc @MortalHappiness could you also review this PR if you have time?

Copy link
Member

@MortalHappiness MortalHappiness left a comment

Choose a reason for hiding this comment

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

I ran kustomize edit fix on all old files and find these inconsistencies with your fix. Do you have any specific reasons not to use the kustomize edit fix command directly? If yes, could you give some reasons why they should be different?

apiserver/deploy/base/insecure/kustomization.yaml Outdated Show resolved Hide resolved
apiserver/deploy/base/insecure/kustomization.yaml Outdated Show resolved Hide resolved
apiserver/deploy/base/secure/kustomization.yaml Outdated Show resolved Hide resolved
ray-operator/config/manager/kustomization.yaml Outdated Show resolved Hide resolved
ray-operator/config/rbac/kustomization.yaml Outdated Show resolved Hide resolved
ray-operator/config/rbac/kustomization.yaml Show resolved Hide resolved
@kevin85421
Copy link
Member

@oksanabaza is this PR ready for another review?

@oksanabaza
Copy link
Contributor Author

@kevin85421 @MortalHappiness could please review it again, thanks!

Copy link
Member

@MortalHappiness MortalHappiness left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@MortalHappiness MortalHappiness left a comment

Choose a reason for hiding this comment

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

I found that some files are not changed. Please run kustomize edit fix for all of these files. For example, you didn't fix ray-operator/config/certmanager/kustomization.yaml in this PR.

apiserver/deploy/base/insecure/kustomization.yaml
apiserver/deploy/base/secure/kustomization.yaml
apiserver/deploy/local/insecure/kustomization.yaml
apiserver/deploy/local/secure/kustomization.yaml
ray-operator/config/certmanager/kustomization.yaml
ray-operator/config/crd/kustomization.yaml
ray-operator/config/default/kustomization.yaml
ray-operator/config/default-with-webhooks/kustomization.yaml
ray-operator/config/manager/kustomization.yaml
ray-operator/config/rbac/kustomization.yaml
ray-operator/config/webhook/kustomization.yaml

@oksanabaza
Copy link
Contributor Author

@MortalHappiness thank you for the comment. I've run kustomize edit fix for all of them. Could you please check it again? Thank you

Copy link
Member

@MortalHappiness MortalHappiness left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@kevin85421 kevin85421 left a comment

Choose a reason for hiding this comment

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

Thanks!

@kevin85421 kevin85421 merged commit 1213d15 into ray-project:master Oct 21, 2024
27 checks passed
ryanaoleary pushed a commit to ryanaoleary/kuberay that referenced this pull request Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants