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

KubeRay: kubebuilder creat RayService Controller and CR #270

Merged
merged 12 commits into from
May 31, 2022

Conversation

brucez-anyscale
Copy link
Contributor

@brucez-anyscale brucez-anyscale commented May 19, 2022

Why are these changes needed?

We plan to add a new RayService controller. This pr does the kubebuilder create api with basic implement. No real logic yet. Once this pr merged. We will start to implement logic for RayService.

Related issue number

#214

Checks

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

@simon-mo
Copy link
Collaborator

@Jeffwan @edoakes ping for review, thank you!

ray-operator/Makefile Show resolved Hide resolved
ray-operator/PROJECT Outdated Show resolved Hide resolved
)

// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN!
// NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just curious, does KubeBuilder auto-generate this file as an empty template?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one will auto generated if init by kubebuilder. But I do not think it is total empty

ray-operator/config/rbac/role.yaml Show resolved Hide resolved
ray-operator/config/rbac/role.yaml Show resolved Hide resolved
ray-operator/config/crd/kustomizeconfig.yaml Show resolved Hide resolved
@Jeffwan
Copy link
Collaborator

Jeffwan commented May 24, 2022

Great! I will have a look later today.

Copy link
Collaborator

@DmitriGekhtman DmitriGekhtman left a comment

Choose a reason for hiding this comment

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

Looks good to me.
Let's wait on review from @edoakes and @Jeffwan .

@Jeffwan
Copy link
Collaborator

Jeffwan commented May 25, 2022

I suggest to move unrelated changes like renaming the backend service folder name to a separate PR and leave this PR for pure service controller changes. We are using squash merge and all these changes will be squash to single commit then.

@Jeffwan
Copy link
Collaborator

Jeffwan commented May 28, 2022

#275 is merged. Let's rebase the master and update this PR.

@brucez-anyscale
Copy link
Contributor Author

Thanks. I have rebased this pr.

@DmitriGekhtman
Copy link
Collaborator

Nice, the diff is much cleaner now.

Copy link
Collaborator

@Jeffwan Jeffwan left a comment

Choose a reason for hiding this comment

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

The changes look clean and straightforward.
/lgtm

@Jeffwan
Copy link
Collaborator

Jeffwan commented May 31, 2022

Seems Dmitri has approved the changed as well and now we can merge the PR

@Jeffwan Jeffwan merged commit b020271 into master May 31, 2022
@DmitriGekhtman DmitriGekhtman deleted the brucez/initRayServiceController branch December 3, 2022 00:01
lowang-bh pushed a commit to lowang-bh/kuberay that referenced this pull request Sep 24, 2023
)

* KubeRay: kubebuilder api create RayService Controller and CR

* Update

* update

* Auto generate role

* Update role

* go imports

* Revert rayservice change

* init ray group RayService

* Revert naming

* Revert naming

* Remove webhook
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.

5 participants