add support for svc backend #1363
Conversation
tanujd11
requested review from
donovanmuller,
k0da,
kuritka,
ytsarev and
jkremser
as code owners
Signed-off-by: tanujd11 <[email protected]>
tanujd11
force-pushed
the
feat/svc-backend
branch
from
df1fb91 to
be11523
Compare
There was a problem hiding this comment.
It is exciting, thanks a lot for your contribution!
Can you please add
- Reconciliation test covering new capability to https://github.com/k8gb-io/k8gb/blob/master/controllers/gslb_controller_reconciliation_test.go
- e2e tests to https://github.com/k8gb-io/k8gb/tree/master/terratest
- document the new feature in https://github.com/k8gb-io/k8gb/tree/master/docs
Thanks!
| } | ||
|
|
||
| if gslb.Spec.LoadBalancer != nil { | ||
| localTargets, err = r.DNSProvider.GslbServiceExposedIPs(gslb) |
There was a problem hiding this comment.
this way, if both gslb.spec.loadBalancer and gslb.spec.ingress are specified in the CR, then the localTargets will be obtained using the LB section only (the last will win). I am not sure how it should be designed, to be honest. Does it actually make sense that gslb CR is backed by both at once? If not we could implement the x-kubernetes-validations to catch this during resource admission, otherwise I'd go with simple if - else if here for now.
There was a problem hiding this comment.
yeah, I think adding a validation for it is better so that both can't be specified. It is a cleaner and better approach then making some blunders while missing some case in if - else approach. It is better to be clear at the start of the implementation and state that both can't be used at once. WDYT?
There was a problem hiding this comment.
makes sense to me. Would you mind exploring those x-kubernetes-validations? There should be an annotation for kubebuilder for that (link)
something like
// +kubebuilder:validation:XValidation:rule=( has(self.loadBalancer) && !has(self. ingress) ) || ( !has(self.loadBalancer) && has(self. ingress) )
|
Hi @tanujd11! Thank you so much for your contribution! Do you plan to finalize this PR? If not, please let us know, so we can takeover 👍 |
|
I will close this PR as the codebases progressed heavily since then. Thanks a lot for the contribution attempt, I really appreciate it! |
Adds LoadBalancer service as part of the backend for K8GB. Relate Issue is: #147
HOW TO RUN CI
---By default, all the checks will be run automatically. Furthermore, when changing website-related stuff, the preview will be generated by the netlify bot.
Heavy tests
Add the
heavy-testslabel on this PR if you want full-blown tests that include more than 2-cluster scenarios.Debug tests
If the test suite is failing for you, you may want to try triggering
Re-run all jobs(top right) with debug logging enabled. It will also make the print debug action more verbose.