Skip to content

Commit

Permalink
Allow the Operator to be installed without web-hooks (#586)
Browse files Browse the repository at this point in the history
  • Loading branch information
thegridman committed Mar 11, 2023
1 parent d213b90 commit 4dad097
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 7 deletions.
10 changes: 7 additions & 3 deletions controllers/coherence_controller.go
Original file line number Diff line number Diff line change
@@ -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.
*/
Expand Down Expand Up @@ -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
Expand All @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion docs/about/04_coherence_spec.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ m| useContainerLimits | If set to true Adds the -XX:+UseContainerSupport JVM op
m| gc | Set JVM garbage collector options. m| &#42;<<JvmGarbageCollectorSpec,JvmGarbageCollectorSpec>> | false
m| diagnosticsVolume | DiagnosticsVolume is the volume to write JVM diagnostic information to, for example heap dumps, JFRs etc. m| &#42;https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.24/#volume-v1-core | false
m| memory | Configure the JVM memory options. m| &#42;<<JvmMemorySpec,JvmMemorySpec>> | false
m| jmxmp | Configure JMX using JMXMP. m| &#42;<<JvmJmxmpSpec,JvmJmxmpSpec>> | 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| &#42;<<JvmJmxmpSpec,JvmJmxmpSpec>> | 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| &#42;bool | false
|===
Expand Down
3 changes: 3 additions & 0 deletions docs/installation/01_installation.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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 <<docs/installation/07_webhooks.adoc,Web Hooks>> documentation for how to do this.
====
[IMPORTANT]
Expand Down
63 changes: 60 additions & 3 deletions docs/installation/07_webhooks.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ set the `webhookCertType` value.
----
helm install \
--namespace <namespace> \
--set webhookCertType=manual <1>
--set webhookCertType=manual \ <1>
coherence-operator \
coherence/coherence-operator
----
Expand All @@ -190,8 +190,8 @@ name using the `webhookCertSecret` value.
----
helm install \
--namespace <namespace> \
--set webhookCertType=manual <1>
--set webhookCertSecret=operator-certs <2>
--set webhookCertType=manual \ <1>
--set webhookCertSecret=operator-certs \ <2>
coherence-operator \
coherence/coherence-operator
----
Expand All @@ -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 <namespace> \
--set webhooks=false \
coherence-operator \
coherence/coherence-operator
----
4 changes: 4 additions & 0 deletions helm-charts/coherence-operator/templates/deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
5 changes: 5 additions & 0 deletions helm-charts/coherence-operator/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 2 additions & 0 deletions pkg/runner/cmd_operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions test/e2e/helm/helm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit 4dad097

Please sign in to comment.