Added ingress support #712 #785
Conversation
abhishekdwivedi3060
force-pushed
the
Issue-712
branch
from
99ee7b6 to
2ef9d4e
Compare
abhishekdwivedi3060
force-pushed
the
Issue-712
branch
2 times, most recently
from
a475118 to
b8bd5c0
Compare
abhishekdwivedi3060
force-pushed
the
Issue-712
branch
3 times, most recently
from
1b21d00 to
1ff2b10
Compare
abhishekdwivedi3060
force-pushed
the
Issue-712
branch
from
1ff2b10 to
7ad9d5e
Compare
abhishekdwivedi3060
force-pushed
the
Issue-712
branch
from
7ad9d5e to
a428afd
Compare
abhishekdwivedi3060
changed the title
| @@ -82,13 +84,34 @@ func (r *CrdbCluster) Default() { | |||
| // ValidateCreate implements webhook.Validator so a webhook will be registered for the type. | |||
| func (r *CrdbCluster) ValidateCreate() error { | |||
| webhookLog.Info("validate create", "name", r.Name) | |||
| var errors []error | |||
There was a problem hiding this comment.
why do we need to have a slice of errors if there can be at most one error?
There was a problem hiding this comment.
Right now, there are minimal checks in webhook so returning single error is fine.
But as we keep on adding more validation checks in webhook, ideally we should validate the complete CR object and return the combined error in one go instead of returning one by one error in each iteration by the user.
Hence added slice of errors here.
| return false, nil | ||
| } | ||
|
|
||
| // this is the case of update when ingress is removed from CR |
There was a problem hiding this comment.
nit: maybe name the function needsIngressDeploy? it's a little confusing in this case when the name is needsIngress but we need to remove the ingress
| @@ -210,12 +216,19 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, req reconcile.Request | |||
|
|
|||
| // SetupWithManager registers the controller with the controller.Manager from controller-runtime | |||
| func (r *ClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { | |||
| var ingress client.Object | |||
| ingress = &v1.Ingress{} | |||
| if !util.CheckIfAPIVersionKindAvailable(mgr.GetConfig(), "networking.k8s.io/v1", "Ingress") { | |||
There was a problem hiding this comment.
why do we need this - does it depend on the version of k8s being used or something?
There was a problem hiding this comment.
Yes. We need to support both v1beta1 ingress and v1 ingress. So depending upon the version available in the cluster (depends on the k8s version), we have to watch corresponding version ingress.
This will be removed when our minimum supported version is 1.19
closes #712
Input to be given:
Make sure the host
ui.test.comis publicly accessible.There can be 2 ways to make the host locally resolvable are:
a. Make an entry in
/etc/hostswith the host and ingress-controller Public IPb. Make an entry in /etc/hosts` with the local host (127.0.0.1) and do port-forwarding on ingress-controller pod
NOTE: Ingress-controller is not deployed as part of this feature. It is pre-requisite that ingress-controller is running inside the Kubernetes cluster