-
Notifications
You must be signed in to change notification settings - Fork 19
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
feat: full kubernetes addr #489
Conversation
Codecov Report
@@ Coverage Diff @@
## main #489 +/- ##
==========================================
+ Coverage 67.67% 67.69% +0.01%
==========================================
Files 27 27
Lines 4210 4228 +18
==========================================
+ Hits 2849 2862 +13
- Misses 1312 1316 +4
- Partials 49 50 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Others LGTM!
// TODO according to @arkbriar, update compute advertise-addr may cause compatibility issue | ||
if component == consts.ComponentCompute { | ||
return componentName | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's okay since the default value of isFullKubernetesAddr
is false.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just remove this if-block
and let the users know that enableFullKubernetesAddr
will cause incompatibility.
dc5e5c6
to
ac22a07
Compare
What's changed and what's your intention?
Now the address of components (e.g. meta. compute) is
[<pod>.]<service>
.This pr add a flag
EnableFullKubernetesAddr
in RisingWave CRD. If's enabled, operator will use a full k8s address ([<pod>.]<service>.<namespace>.svc
) instead.It's useful if we need to call
risectl
at another namespace. (risectl
uses the meta advertise address to connect meta. In the previous address format, the connection cannot be established in a different namespace.)Tested it locally.
Checklist
Refer to a related PR or issue link (optional)