From 4dad0976204f7270178a3f10b52e1a9c50e2a8f7 Mon Sep 17 00:00:00 2001 From: Jonathan Knight Date: Sat, 11 Mar 2023 16:32:24 +0300 Subject: [PATCH] Allow the Operator to be installed without web-hooks (#586) --- controllers/coherence_controller.go | 10 ++- docs/about/04_coherence_spec.adoc | 2 +- docs/installation/01_installation.adoc | 3 + docs/installation/07_webhooks.adoc | 63 ++++++++++++++++++- .../templates/deployment.yaml | 4 ++ helm-charts/coherence-operator/values.yaml | 5 ++ pkg/runner/cmd_operator.go | 2 + test/e2e/helm/helm_test.go | 17 +++++ 8 files changed, 99 insertions(+), 7 deletions(-) diff --git a/controllers/coherence_controller.go b/controllers/coherence_controller.go index 305c2fab6..ad03bd60b 100644 --- a/controllers/coherence_controller.go +++ b/controllers/coherence_controller.go @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020, 2022, Oracle and/or its affiliates. + * Copyright (c) 2020, 2023, Oracle and/or its affiliates. * Licensed under the Universal Permissive License v 1.0 as shown at * http://oss.oracle.com/licenses/upl. */ @@ -99,7 +99,7 @@ func (in *CoherenceReconciler) Reconcile(ctx context.Context, request ctrl.Reque return ctrl.Result{}, nil } // Error reading the current deployment state from k8s. - return reconcile.Result{}, err + return reconcile.Result{}, errors.Wrap(err, "getting Coherence resource") } // Check whether this is a deletion @@ -120,7 +120,11 @@ func (in *CoherenceReconciler) Reconcile(ctx context.Context, request ctrl.Reque controllerutil.RemoveFinalizer(deployment, coh.CoherenceFinalizer) err := in.GetClient().Update(ctx, deployment) if err != nil { - return ctrl.Result{}, err + if apierrors.IsNotFound(err) { + log.Info("Failed to remove the finalizer from the Coherence resource, it looks like it had already been deleted") + return ctrl.Result{}, nil + } + return ctrl.Result{}, errors.Wrap(err, "trying to remove finalizer from Coherence resource") } } else { log.Info("Coherence resource deleted at " + deleteTime.String() + ", finalizer already removed") diff --git a/docs/about/04_coherence_spec.adoc b/docs/about/04_coherence_spec.adoc index 9764e775c..82acaf0f8 100644 --- a/docs/about/04_coherence_spec.adoc +++ b/docs/about/04_coherence_spec.adoc @@ -345,7 +345,7 @@ m| useContainerLimits | If set to true Adds the -XX:+UseContainerSupport JVM op m| gc | Set JVM garbage collector options. m| *<> | false m| diagnosticsVolume | DiagnosticsVolume is the volume to write JVM diagnostic information to, for example heap dumps, JFRs etc. m| *https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.24/#volume-v1-core | false m| memory | Configure the JVM memory options. m| *<> | false -m| jmxmp | Configure JMX using JMXMP. m| *<> | false +m| jmxmp | Configure JMX using JMXMP. Note: This should only be used in development as JMXMP does not have support for encrypted connections via TLS. Use in production should ideally put the JMXMP port behind some sort of TLS enabled ingress or network policy. m| *<> | false m| useJibClasspath | A flag indicating whether to automatically add the default classpath for images created by the JIB tool https://github.com/GoogleContainerTools/jib If true then the /app/lib/* /app/classes and /app/resources entries are added to the JVM classpath. The default value fif not specified is true. m| *bool | false |=== diff --git a/docs/installation/01_installation.adoc b/docs/installation/01_installation.adoc index 9ccabd1d7..6eb504d8f 100644 --- a/docs/installation/01_installation.adoc +++ b/docs/installation/01_installation.adoc @@ -109,6 +109,9 @@ subtle issues. installing multiple web-hooks for the same resource may lead to issues. If an operator is uninstalled, but the web-hook configuration remains, then Kubernetes will not accept modifications to resources of that type as it will be unable to contact the web-hook. + +It is possible to run the Operator without web-hooks, but this has its own +caveats see the <> documentation for how to do this. ==== [IMPORTANT] diff --git a/docs/installation/07_webhooks.adoc b/docs/installation/07_webhooks.adoc index 77e0078bc..fb9619f2d 100644 --- a/docs/installation/07_webhooks.adoc +++ b/docs/installation/07_webhooks.adoc @@ -175,7 +175,7 @@ set the `webhookCertType` value. ---- helm install \ --namespace \ - --set webhookCertType=manual <1> + --set webhookCertType=manual \ <1> coherence-operator \ coherence/coherence-operator ---- @@ -190,8 +190,8 @@ name using the `webhookCertSecret` value. ---- helm install \ --namespace \ - --set webhookCertType=manual <1> - --set webhookCertSecret=operator-certs <2> + --set webhookCertType=manual \ <1> + --set webhookCertSecret=operator-certs \ <2> coherence-operator \ coherence/coherence-operator ---- @@ -202,3 +202,60 @@ helm install \ The Coherence Operator will now expect to find the keys and certs in a `Secret` named `operator-certs` in the same namespace that the Operator is deployed into. +[#no-hooks] +=== Install the Operator Without Web-Hooks + +It is possible to start the Operator without it registering any web-hooks with the API server. + +[CAUTION] +==== +Running the Operator without web-hooks is not recommended. +The admission web-hooks validate the `Coherence` resource yaml before it gets into the k8s cluster. +Without the web-hooks, invalid yaml will be accepted by k8s and the Operator will then log errors +when it tries to reconcile invalid yaml. Or worse, the Operator will create an invalid `StatefulSet` +which will then fail to start. +==== + +==== Install Using Manifest File + +If installing using the manifest yaml files, then you need to edit the `coherence-operator.yaml` manifest to add a +command line argument to the Operator. + +Update the `controller-manager` deployment and add an argument, edit the section that looks like this: +[source,yaml] +---- + args: + - operator + - --enable-leader-election +---- + +and add the additional `--enable-webhook=false` argument like this: + +[source,yaml] +---- + args: + - operator + - --enable-leader-election + - --enable-webhook=false +---- + + +apiVersion: apps/v1 +kind: Deployment +metadata: + name: controller-manager + + +==== Installing Using Helm + +If installing the Operator using Helm, the `webhooks` value can be set to false in the values file or +on the command line. + +[source,bash] +---- +helm install \ + --namespace \ + --set webhooks=false \ + coherence-operator \ + coherence/coherence-operator +---- diff --git a/helm-charts/coherence-operator/templates/deployment.yaml b/helm-charts/coherence-operator/templates/deployment.yaml index 3a0846f71..43bdf348b 100644 --- a/helm-charts/coherence-operator/templates/deployment.yaml +++ b/helm-charts/coherence-operator/templates/deployment.yaml @@ -99,6 +99,10 @@ spec: {{- if (eq .Values.clusterRoles false) }} - --enable-webhook=false - --install-crd=false +{{- else }} +{{- if (eq .Values.webhooks false) }} + - --enable-webhook=false +{{- end }} {{- end }} command: - "/files/runner" diff --git a/helm-charts/coherence-operator/values.yaml b/helm-charts/coherence-operator/values.yaml index d78c2c785..b58a72b4d 100644 --- a/helm-charts/coherence-operator/values.yaml +++ b/helm-charts/coherence-operator/values.yaml @@ -167,3 +167,8 @@ clusterRoles: true # to set theCoherence site and rack values so Coherence cluster will be unable to automatically achieve site-safety. # The default is true. nodeRoles: false +# webhooks controls whether the Coherence Operator registers admission web-hooks for the Coherence resource. +# If this is set to false, then it will be possible to install invalid Coherence resource into the Kubernetes +# cluster, that may cause errors when the Operator tries to reconcile them, or worse the Operator may create +# other invalid Kubernetes resources that fail to run. +webhooks: true \ No newline at end of file diff --git a/pkg/runner/cmd_operator.go b/pkg/runner/cmd_operator.go index d964ef26f..788d83fe2 100644 --- a/pkg/runner/cmd_operator.go +++ b/pkg/runner/cmd_operator.go @@ -153,6 +153,8 @@ func execute() error { if err = (&coh.Coherence{}).SetupWebhookWithManager(mgr); err != nil { return errors.Wrap(err, " unable to create webhook") } + } else { + setupLog.Info("Operator is running with web-hooks disabled") } // Create the REST server diff --git a/test/e2e/helm/helm_test.go b/test/e2e/helm/helm_test.go index 8f7947a81..cd585acdf 100644 --- a/test/e2e/helm/helm_test.go +++ b/test/e2e/helm/helm_test.go @@ -123,6 +123,23 @@ func TestNotCreateWebhookCertSecretIfManualCertManager(t *testing.T) { g.Expect(err).To(HaveOccurred()) } +func TestDisableWebhooks(t *testing.T) { + g := NewGomegaWithT(t) + result, err := helmInstall("--set", "webhooks=false") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(result).NotTo(BeNil()) + + dep := &appsv1.Deployment{} + err = result.Get("coherence-operator", dep) + g.Expect(err).NotTo(HaveOccurred()) + + c := findContainer("manager", dep) + g.Expect(c).NotTo(BeNil()) + + g.Expect(c.Args).NotTo(BeNil()) + g.Expect(c.Args).Should(ContainElements("operator", "--enable-leader-election", "--enable-webhook=false")) +} + func TestBasicHelmInstall(t *testing.T) { g := NewGomegaWithT(t) cmd, err := createHelmCommand()