Proper shut down for operator and webhook; Have webhook delete VWC (#3761)

* Proper shut down for operator and webhook; Have webhook delete VWC

Author: rjeberhard

deployment/scripts/operator.sh

@@ -70,3 +70,7 @@ HEAP="-XshowSettings:vm"
 java $HEAP $MOCKING_WLS $DEBUG $LOGGING -jar /operator/weblogic-kubernetes-operator.jar &
 PID=$!
 wait $PID
+
+SHUTDOWN_COMPLETE_MARKER_FILE="${DEPLOYMENT_DIR}/marker.shutdown-complete"
+
+touch ${SHUTDOWN_COMPLETE_MARKER_FILE}

deployment/scripts/stop.sh

@@ -0,0 +1,18 @@
+#!/bin/bash
+# Copyright (c) 2022, Oracle and/or its affiliates.
+# Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl.
+
+echo "Setting stop signal"
+
+DEPLOYMENT_DIR="/deployment"
+SHUTDOWN_MARKER_FILE="${DEPLOYMENT_DIR}/marker.shutdown"
+SHUTDOWN_COMPLETE_MARKER_FILE="${DEPLOYMENT_DIR}/marker.shutdown-complete"
+
+touch ${SHUTDOWN_MARKER_FILE}
+
+while true; do
+  if [ -e ${SHUTDOWN_COMPLETE_MARKER_FILE} ] ; then
+    exit 0
+  fi
+  sleep 1
+done

deployment/scripts/webhook.sh

@@ -63,3 +63,8 @@ HEAP="-XshowSettings:vm"
 java -cp /operator/weblogic-kubernetes-operator.jar $HEAP $MOCKING_WLS $DEBUG $LOGGING oracle.kubernetes.operator.WebhookMain &
 PID=$!
 wait $PID
+
+DEPLOYMENT_DIR="/deployment"
+SHUTDOWN_COMPLETE_MARKER_FILE="${DEPLOYMENT_DIR}/marker.shutdown-complete"
+
+touch ${SHUTDOWN_COMPLETE_MARKER_FILE}

kubernetes/charts/weblogic-operator/templates/_operator-clusterrole-general.tpl

@@ -38,5 +38,5 @@ rules:
   verbs: ["create"]
 - apiGroups: ["admissionregistration.k8s.io"]
   resources: ["validatingwebhookconfigurations"]
-  verbs: ["get", "create", "update", "patch"]
+  verbs: ["get", "create", "update", "patch", "delete"]
 {{- end }}

kubernetes/charts/weblogic-operator/templates/_operator-dep.tpl

@@ -54,8 +54,11 @@ spec:
       - name: "weblogic-operator"
         image: {{ .image | quote }}
         imagePullPolicy: {{ .imagePullPolicy | quote }}
-        command: ["bash"]
-        args: ["/deployment/operator.sh"]
+        command: ["/deployment/operator.sh"]
+        lifecycle:
+          preStop:
+            exec:
+              command: ["/deployment/stop.sh"]
         env:
         - name: "OPERATOR_NAMESPACE"
           valueFrom:
@@ -131,17 +134,13 @@ spec:
         {{- if not .remoteDebugNodePortEnabled }}
         livenessProbe:
           exec:
-            command:
-            - "bash"
-            - "/probes/livenessProbe.sh"
+            command: ["/probes/livenessProbe.sh"]
           initialDelaySeconds: 40
           periodSeconds: 10
           failureThreshold: 5
         readinessProbe:
           exec:
-            command:
-            - "bash"
-            - "/probes/readinessProbe.sh"
+            command: ["/probes/readinessProbe.sh"]
           initialDelaySeconds: 2
           periodSeconds: 10
         {{- end }}
@@ -276,8 +275,11 @@ spec:
           - name: "weblogic-operator-webhook"
             image: {{ .image | quote }}
             imagePullPolicy: {{ .imagePullPolicy | quote }}
-            command: ["bash"]
-            args: ["/deployment/webhook.sh"]
+            command: ["/deployment/webhook.sh"]
+            lifecycle:
+              preStop:
+                exec:
+                  command: ["/deployment/stop.sh"]
             env:
             - name: "WEBHOOK_NAMESPACE"
               valueFrom:
@@ -341,16 +343,12 @@ spec:
             {{- if not .remoteDebugNodePortEnabled }}
             livenessProbe:
               exec:
-                command:
-                - "bash"
-                - "/probes/livenessProbe.sh"
+                command: ["/probes/livenessProbe.sh"]
               initialDelaySeconds: 40
               periodSeconds: 5
             readinessProbe:
               exec:
-                command:
-                - "bash"
-                - "/probes/readinessProbe.sh"
+                command: ["/probes/readinessProbe.sh"]
               initialDelaySeconds: 2
               periodSeconds: 10
             {{- end }}

kubernetes/charts/weblogic-operator/templates/_operator-role.tpl

@@ -1,4 +1,4 @@
-# Copyright (c) 2018, 2021, Oracle and/or its affiliates.
+# Copyright (c) 2018, 2022, Oracle and/or its affiliates.
 # Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl.
 
 {{- define "operator.operatorRole" }}
@@ -16,5 +16,5 @@ rules:
   verbs: ["get", "list", "watch", "create", "update", "patch", "delete", "deletecollection"]
 - apiGroups: ["admissionregistration.k8s.io"]
   resources: ["validatingwebhookconfigurations"]
-  verbs: ["get", "create", "update", "patch"]
+  verbs: ["get", "create", "update", "patch", "delete"]
 {{- end }}

kubernetes/src/test/java/oracle/kubernetes/operator/create/CreateOperatorGeneratedFilesTestBase.java

@@ -13,7 +13,10 @@
 import io.kubernetes.client.openapi.models.V1Deployment;
 import io.kubernetes.client.openapi.models.V1DeploymentStrategy;
 import io.kubernetes.client.openapi.models.V1EnvVarSource;
+import io.kubernetes.client.openapi.models.V1ExecAction;
 import io.kubernetes.client.openapi.models.V1LabelSelector;
+import io.kubernetes.client.openapi.models.V1Lifecycle;
+import io.kubernetes.client.openapi.models.V1LifecycleHandler;
 import io.kubernetes.client.openapi.models.V1Namespace;
 import io.kubernetes.client.openapi.models.V1PolicyRule;
 import io.kubernetes.client.openapi.models.V1Probe;
@@ -218,8 +221,10 @@ protected V1Deployment getExpectedWeblogicOperatorDeployment() {
                                         .image(getInputs().getWeblogicOperatorImage())
                                         .imagePullPolicy(
                                             getInputs().getWeblogicOperatorImagePullPolicy())
-                                        .addCommandItem("bash")
-                                        .addArgsItem("/deployment/operator.sh")
+                                        .addCommandItem("/deployment/operator.sh")
+                                        .lifecycle(
+                                            new V1Lifecycle().preStop(new V1LifecycleHandler().exec(
+                                                new V1ExecAction().addCommandItem("/deployment/stop.sh"))))
                                         .addEnvItem(
                                             newEnvVar()
                                                 .name("OPERATOR_NAMESPACE")
@@ -305,7 +310,7 @@ private V1Probe createProbe(Integer initialDelaySeconds, Integer periodSeconds,
         .initialDelaySeconds(initialDelaySeconds)
         .periodSeconds(periodSeconds)
         .failureThreshold(failureThreshold)
-        .exec(newExecAction().addCommandItem("bash").addCommandItem(shellScript));
+        .exec(newExecAction().addCommandItem(shellScript));
   }
 
   @Test
@@ -758,7 +763,8 @@ private V1PolicyRule newPolicyRuleForValidatingWebhookConfiguration() {
                 "get",
                 "create",
                 "update",
-                "patch"));
+                "patch",
+                "delete"));
   }
 
   @Test

operator/src/main/java/oracle/kubernetes/operator/BaseMain.java

@@ -9,6 +9,7 @@
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.io.PrintStream;
+import java.nio.file.Files;
 import java.security.KeyManagementException;
 import java.security.KeyStoreException;
 import java.security.NoSuchAlgorithmException;
@@ -35,6 +36,7 @@
 import oracle.kubernetes.operator.logging.LoggingFacade;
 import oracle.kubernetes.operator.logging.LoggingFactory;
 import oracle.kubernetes.operator.tuning.TuningParameters;
+import oracle.kubernetes.operator.utils.PathSupport;
 import oracle.kubernetes.operator.work.Component;
 import oracle.kubernetes.operator.work.Container;
 import oracle.kubernetes.operator.work.ContainerResolver;
@@ -130,6 +132,36 @@ void startDeployment(Runnable completionAction) {
     }
   }
 
+  void stopDeployment(Runnable completionAction) {
+    Step shutdownSteps = createShutdownSteps();
+    if (shutdownSteps != null) {
+      try {
+        delegate.runSteps(new Packet(), shutdownSteps, new ReleaseShutdownSignalRunnable(completionAction));
+        acquireShutdownSignal();
+      } catch (Throwable e) {
+        LOGGER.warning(MessageKeys.EXCEPTION, e);
+      }
+    } else if (completionAction != null) {
+      completionAction.run();
+    }
+  }
+
+  private class ReleaseShutdownSignalRunnable implements Runnable {
+    final Runnable inner;
+
+    ReleaseShutdownSignalRunnable(Runnable inner) {
+      this.inner = inner;
+    }
+
+    @Override
+    public void run() {
+      if (inner != null) {
+        inner.run();
+      }
+      releaseShutdownSignal();
+    }
+  }
+
   void markReadyAndStartLivenessThread() {
     try {
       new DeploymentReady(delegate).create();
@@ -153,6 +185,11 @@ void startRestServer(Container container)
 
   abstract BaseRestServer createRestServer();
 
+  // For test
+  AtomicReference<BaseServer> getRestServer() {
+    return restServer;
+  }
+
   void stopRestServer() {
     Optional.ofNullable(restServer.getAndSet(null)).ifPresent(BaseServer::stop);
   }
@@ -181,24 +218,56 @@ void stopMetricsServer() {
 
   abstract Step createStartupSteps();
 
+  Step createShutdownSteps() {
+    return null;
+  }
+
   abstract void logStartingLivenessMessage();
 
   void stopAllWatchers() {
     // no-op
   }
 
-  void waitForDeath() {
-    Runtime.getRuntime().addShutdownHook(new Thread(shutdownSignal::release));
-
+  private void acquireShutdownSignal() {
     try {
       shutdownSignal.acquire();
     } catch (InterruptedException ignore) {
       Thread.currentThread().interrupt();
     }
+  }
+
+  private void releaseShutdownSignal() {
+    shutdownSignal.release();
+  }
+
+  // For test
+  int getShutdownSignalAvailablePermits() {
+    return shutdownSignal.availablePermits();
+  }
+
+  void waitForDeath() {
+    Runtime.getRuntime().addShutdownHook(new Thread(this::releaseShutdownSignal));
+    scheduleCheckForShutdownMarker();
+
+    acquireShutdownSignal();
 
     stopAllWatchers();
   }
 
+  void scheduleCheckForShutdownMarker() {
+    wrappedExecutorService.scheduleWithFixedDelay(
+        () -> {
+          File marker = new File(delegate.getDeploymentHome(), "marker.shutdown");
+          if (isFileExists(marker)) {
+            releaseShutdownSignal();
+          }
+        }, 5, 2, TimeUnit.SECONDS);
+  }
+
+  private static boolean isFileExists(File file) {
+    return Files.isRegularFile(PathSupport.getPath(file));
+  }
+
   static Packet createPacketWithLoggingContext(String ns) {
     Packet packet = new Packet();
     packet.getComponents().put(

operator/src/main/java/oracle/kubernetes/operator/OperatorMain.java

@@ -172,8 +172,7 @@ public static void main(String[] args) {
       // now we just wait until the pod is terminated
       operatorMain.waitForDeath();
 
-      operatorMain.stopRestServer();
-      operatorMain.stopMetricsServer();
+      operatorMain.stopDeployment(operatorMain::completeStop);
     } finally {
       LOGGER.info(MessageKeys.OPERATOR_SHUTTING_DOWN);
     }
@@ -286,6 +285,11 @@ private void completeBegin() {
     }
   }
 
+  private void completeStop() {
+    stopRestServer();
+    stopMetricsServer();
+  }
+
   NamespaceWatcher getNamespaceWatcher() {
     return namespaceWatcher;
   }

operator/src/main/java/oracle/kubernetes/operator/WebhookMain.java

@@ -84,8 +84,7 @@ public static void main(String[] args) {
       // now we just wait until the pod is terminated
       main.waitForDeath();
 
-      main.stopRestServer();
-      main.stopMetricsServer();
+      main.stopDeployment(main::completeStop);
     } finally {
       LOGGER.info(MessageKeys.WEBHOOK_SHUTTING_DOWN);
     }
@@ -115,6 +114,11 @@ protected Step createStartupSteps() {
             WebhookHelper.createValidatingWebhookConfigurationStep(certs)));
   }
 
+  @Override
+  protected Step createShutdownSteps() {
+    return WebhookHelper.deleteValidatingWebhookConfigurationStep();
+  }
+
   private static Step createInitializeWebhookIdentityStep(WebhookMainDelegate delegate, Step next) {
     return new InitializeWebhookIdentityStep(delegate, next);
   }
@@ -139,6 +143,11 @@ void completeBegin() {
     }
   }
 
+  void completeStop() {
+    stopRestServer();
+    stopMetricsServer();
+  }
+
   Runnable recheckCrd() {
     return () -> delegate.runSteps(createCRDRecheckSteps());
   }