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

modify kuberay operator crds in kuberay operator chart and add apiserver chart #354

Merged
merged 1 commit into from
Jul 15, 2022

Conversation

scarlet25151
Copy link
Collaborator

@scarlet25151 scarlet25151 commented Jul 7, 2022

Why are these changes needed?

Add a new chart for deploy kuberay apiserver

The crds in helm chart is generated by the controller gen in ray-operator folder by running make helm

Related issue number

Checks

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

@scarlet25151 scarlet25151 changed the title modify kuberay operator crds and add apiserver chart modify kuberay operator crds in kuberay operator chart and add apiserver chart Jul 7, 2022
@scarlet25151 scarlet25151 force-pushed the update-helm-chart branch 2 times, most recently from 9acb15a to 46b8fa9 Compare July 7, 2022 07:41
@Jeffwan
Copy link
Collaborator

Jeffwan commented Jul 7, 2022

I am thinking is it better to embed kuberay-apiserver in kuberay helm (remove kuberay-operator). Or keep it a separate chart? Ideally, we should move everything to single public chart #260

@scarlet25151 scarlet25151 force-pushed the update-helm-chart branch 2 times, most recently from 7c516b4 to 2fcfed1 Compare July 12, 2022 01:24
@scarlet25151
Copy link
Collaborator Author

Move the kuberay-operator to the subcharts of kuberay so we can use helm to deploy operator and apiserver at once use something like helm install kuberay ./kuberay without coupling two charts together.

@Jeffwan
Copy link
Collaborator

Jeffwan commented Jul 14, 2022

I feel it's better to keep kuberay-operator at this moment, once we finish the release and give the latest guidance, let's remove it. I know lots of user still use this folder and removing it will bring the breaking change.

Few things

  1. Please leave a deprecation notice there.
  2. Please update docs (README.md) and https://github.com/ray-project/kuberay/tree/master/docs/deploy
  3. Please help fix it in original kuberay-operator folder [Bug] Missing rayservices.ray.io CRD access rules in ClusterRole #284

@scarlet25151
Copy link
Collaborator Author

I feel it's better to keep kuberay-operator at this moment, once we finish the release and give the latest guidance, let's remove it. I know lots of user still use this folder and removing it will bring the breaking change

@Jeffwan yes, indeed, here I need to confirm that if we keep kuberay-operator folder and then add the apiserverchart into it, we need to change the structure in values.yaml and path in /template to separate indicate operator's and apiserver's parameters like: opertor.resource and apiserver.resource it will bring some effort for coupling and decoupling the charts if we would put the two chart together for now.

@scarlet25151 scarlet25151 force-pushed the update-helm-chart branch 2 times, most recently from 57f2302 to 79ac642 Compare July 14, 2022 18:40
@Jeffwan Jeffwan merged commit 3ea0113 into ray-project:master Jul 15, 2022
lowang-bh pushed a commit to lowang-bh/kuberay that referenced this pull request Sep 24, 2023
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.

2 participants