Support enableIngress for RayCluster #38
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jeffwan
force-pushed
the
support_ingress
branch
from
4b4a8cd
to
b4678e9
Compare
Jeffwan
force-pushed
the
support_ingress
branch
from
b4678e9
to
96dc521
Compare
Jeffwan
changed the title
Jeffwan
force-pushed
the
support_ingress
branch
5 times, most recently
from
dc914be
to
8e08e07
Compare
chenk008
reviewed
Dec 6, 2021
| annotation[key] = value | ||
| } | ||
|
|
||
| // The tricky thing is we need three ports. redis, dashboard, client. should we use /dashboard /redis to support that? |
There was a problem hiding this comment.
I think the dashboard port is necessary, the redis/client-server port should be optional. We use ingress to expose port to internet access, but the redis/client-server is for internal network.
There was a problem hiding this comment.
Agree. Another tricky thing is redis/client are TCP/UDP that most ingress can not easily support well. I will modify the first version to support dashboard only.
chenk008
reviewed
Dec 6, 2021
| IngressRuleValue: networkingv1.IngressRuleValue{ | ||
| HTTP: &networkingv1.HTTPIngressRuleValue{ | ||
| // https://github.com/kubernetes/ingress-nginx/issues/1655#issuecomment-79129310 | ||
| // It's ok to expose more ports on the same path |
There was a problem hiding this comment.
Will it expose more ports on the same path?
There was a problem hiding this comment.
since we only expose dashboard, we won't use this pattern now.
chenk008
reviewed
Dec 6, 2021
| Name: utils.GenerateServiceName(cluster.Name), | ||
| Namespace: cluster.Namespace, | ||
| Labels: labels, | ||
| Annotations: annotation, |
There was a problem hiding this comment.
Should we add owner reference with Raycluster? It can help to delete ingress automatically?
There was a problem hiding this comment.
That's a good idea. I will do
There was a problem hiding this comment.
owner reference is configured outside now
Add ingress resources in role Copy more configurations from cluster annotation Add ingress example Update ingress version from v1beta1 to v1
Jeffwan
force-pushed
the
support_ingress
branch
from
8e08e07
to
e324afe
Compare
Jeffwan
commented
Feb 16, 2022
| return err | ||
| } | ||
|
|
||
| if err := controllerruntime.SetControllerReference(instance, ingress, r.Scheme); err != nil { |
There was a problem hiding this comment.
@chenk008 I set reference here.
Jeffwan
force-pushed
the
support_ingress
branch
2 times, most recently
from
c6c8993
to
420c1bb
Compare
Jeffwan
force-pushed
the
support_ingress
branch
from
420c1bb
to
f36326d
Compare
|
LGTM, thanks! |
chenk008
approved these changes
Feb 17, 2022
chenk008
pushed a commit
to chenk008/kuberay
that referenced
this pull request
Mar 8, 2022
* Support enableIngress for RayCluster Add ingress resources in role Copy more configurations from cluster annotation Add ingress example Update ingress version from v1beta1 to v1 * Update to expose dashboard only
chenk008
added a commit
that referenced
this pull request
Mar 22, 2022
Merge branch add_pod_lables of git@gitlab.alipay-inc.com:Arc/kuberay.git into master https://code.alipay.com/Arc/kuberay/pull_requests/4?tab=diff Signed-off-by: 壮酱 <zhengchicheng.zcc@antgroup.com> * add operator config * add vendor * folder name change to kuberay-operator (#143) * Add CRD verb permission in helm (#144) * add crd verb permission in helm * fix ray cluster volume * Update helm-chart/kuberay-operator/README.md Co-authored-by: Oliver Mannion <125105+tekumara@users.noreply.github.com> Co-authored-by: wuhua.ck <wuhua.ck@alibaba-inc.com> Co-authored-by: Oliver Mannion <125105+tekumara@users.noreply.github.com> * Explanation and Best Practice for workers-head Reconnection (#142) * Add Explaination and Best Practice for workers-head Reconnection * Fixed formatting issue * Minor updates * updated to permlink and remove one empty line * minor fix Co-authored-by: Lin Ma <lin.ma1@bytedance.com> * refactor: rename kubray -> kuberay (#145) * docs: fix typo in README.md * refactor: rename kubray -> kuberay * Add nightly docker build workflow (#141) * no --all-tags for nightly build add nightly docker build workflow add nightly docker build workflow remove kubebuilder update to workflow * change docker build tag to repo revision * add more debug info for goimport issue (#151) * add more debug info for bug-150: goimport issue * update goimport failure message * Support enableIngress for RayCluster (#38) * Support enableIngress for RayCluster Add ingress resources in role Copy more configurations from cluster annotation Add ingress example Update ingress version from v1beta1 to v1 * Update to expose dashboard only * Add troubleshooting guide for ray version mismatch (#154) Co-authored-by: chenyu.jiang <chenyu.jiang@bytedance.com> * Enable gofmt and move goimports to linter job (#158) * Enable gofmt and move related work to linter job 1. Introduce gofmt github action 2. Move goimports from build job to lint job 3. Run gofmt -s -w to simplify test codes and fix all lint issues in apiserver and cli projects * Fix lint errors in apiserver * Fix lint errors in cli * Fix goimports error * Setup ci for apiserver (#162) * first release * add vendor * PullRequest: 2 add aci Merge branch aci of git@gitlab.alipay-inc.com:Arc/kuberay.git into master https://code.alipay.com/Arc/kuberay/pull_requests/2 Signed-off-by: 五花 <wuhua.ck@antgroup.com> * add aci * fix aci * fix aci * fix aci * fix aci * fix aci * fix aci * fix aci * fix aci * fix aci * fix aci * fix aci * fix aci * fix aci * fix aci * fix aci * fix aci * fix aci * fix aci * fix aci * fix aci * fix aci * fix aci * fix aci * fix aci * fix aci * fix aci * fix aci * test aci * test * test * fix comment * Add KubeRay release guideline (#161) * add flag watchNamespace (#165) Co-authored-by: chen kang <kongchen28@gmail.com> * [Feature]Add subcommand `--version` (#166) * add subcommand --version * fix * lint Co-authored-by: wuhua.ck <wuhua.ck@alibaba-inc.com> * Leader election need lease permission (#169) Co-authored-by: wuhua.ck <wuhua.ck@alibaba-inc.com> * [CLI] check viper error (#172) * check file exist before write * fix Co-authored-by: wuhua.ck <wuhua.ck@alibaba-inc.com> * fix cli typo (#173) Co-authored-by: wuhua.ck <wuhua.ck@alibaba-inc.com> * add vendor * fix * fix
lowang-bh
pushed a commit
to lowang-bh/kuberay
that referenced
this pull request
Sep 24, 2023
* Support enableIngress for RayCluster Add ingress resources in role Copy more configurations from cluster annotation Add ingress example Update ingress version from v1beta1 to v1 * Update to expose dashboard only
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
address #27
Add new reconcile loop for ingress object. This is only working for RayCluster with
spec.HeadGroupSpec.EnabledIngress = trueusers. By default, this is not enabled.