Clarify stopping versus starting

Author: rjeberhard

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

@@ -612,8 +612,6 @@ static void makeRightDomainPresence(
       boolean explicitRecheck,
       boolean isDeleting,
       boolean isWillInterrupt) {
-    LOGGER.entering();
-
     DomainSpec spec = null;
     String ns;
     if (dom != null) {
@@ -624,25 +622,27 @@ static void makeRightDomainPresence(
       ns = existing.getNamespace();
     }
 
-    if (existing != null) {
-      Domain current = existing.getDomain();
-      if (current != null) {
-        // Is this an outdated watch event?
-        if (isOutdated(current.getMetadata(), dom.getMetadata())) {
-          LOGGER.fine(MessageKeys.NOT_STARTING_DOMAINUID_THREAD, domainUID);
-          return;
-        }
-        // Has the spec actually changed? We will get watch events for status updates
-        if (!explicitRecheck && spec != null && spec.equals(current.getSpec())) {
-          // nothing in the spec has changed, but status likely did; update current
-          existing.setDomain(dom);
-          LOGGER.fine(MessageKeys.NOT_STARTING_DOMAINUID_THREAD, domainUID);
-          return;
+    if (!Main.isNamespaceStopping(ns).get()) {
+      if (existing != null) {
+        Domain current = existing.getDomain();
+        if (current != null) {
+          // Is this an outdated watch event?
+          if (isOutdated(current.getMetadata(), dom.getMetadata())) {
+            LOGGER.fine(MessageKeys.NOT_STARTING_DOMAINUID_THREAD, domainUID);
+            return;
+          }
+          // Has the spec actually changed? We will get watch events for status updates
+          if (!explicitRecheck && spec != null && spec.equals(current.getSpec())) {
+            // nothing in the spec has changed, but status likely did; update current
+            existing.setDomain(dom);
+            LOGGER.fine(MessageKeys.NOT_STARTING_DOMAINUID_THREAD, domainUID);
+            return;
+          }
         }
       }
-    }
 
-    internalMakeRightDomainPresence(existing, dom, domainUID, ns, isDeleting, isWillInterrupt);
+      internalMakeRightDomainPresence(existing, dom, domainUID, ns, isDeleting, isWillInterrupt);
+    }
   }
 
   private static void internalMakeRightDomainPresence(

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

@@ -67,6 +67,8 @@ private static ThreadFactory getThreadFactory() {
 
   private static final LoggingFacade LOGGER = LoggingFactory.getLogger("Operator", "Operator");
 
+  private static final String NS_STARTING_NOW = "NS_STARTING_NOW";
+
   static final TuningParameters tuningAndConfig;
 
   static {
@@ -102,6 +104,7 @@ private static ThreadFactory getThreadFactory() {
 
   static final Engine engine = new Engine(wrappedExecutorService);
 
+  static final ConcurrentMap<String, AtomicBoolean> isNamespaceStarted = new ConcurrentHashMap<>();
   static final ConcurrentMap<String, AtomicBoolean> isNamespaceStopping = new ConcurrentHashMap<>();
 
   private static final Map<String, ConfigMapWatcher> configMapWatchers = new ConcurrentHashMap<>();
@@ -262,7 +265,11 @@ private static class StartNamespaceBeforeStep extends Step {
 
     @Override
     public NextAction apply(Packet packet) {
-      if (isNamespaceStopping(ns).get()) {
+      AtomicBoolean a = isNamespaceStarted.computeIfAbsent(ns, (key) -> new AtomicBoolean(false));
+      boolean startingNow = !a.getAndSet(true);
+      packet.put(NS_STARTING_NOW, (Boolean) startingNow);
+
+      if (startingNow) {
         try {
           HealthCheckHelper.performSecurityChecks(version, operatorNamespace, ns);
         } catch (Throwable e) {
@@ -281,11 +288,12 @@ private static void stopNamespaces(Collection<String> namespacesToStop) {
       if (stopping != null) {
         stopping.set(true);
       }
+      isNamespaceStarted.remove(ns);
     }
   }
 
   static AtomicBoolean isNamespaceStopping(String ns) {
-    return isNamespaceStopping.computeIfAbsent(ns, (key) -> new AtomicBoolean(true));
+    return isNamespaceStopping.computeIfAbsent(ns, (key) -> new AtomicBoolean(false));
   }
 
   static void runSteps(Step firstStep) {
@@ -526,15 +534,18 @@ public NextAction onFailure(Packet packet, CallResponse<DomainList> callResponse
 
     @Override
     public NextAction onSuccess(Packet packet, CallResponse<DomainList> callResponse) {
-      boolean starting = isNamespaceStopping.putIfAbsent(ns, new AtomicBoolean(false)) == null;
+      Boolean startingNow = (Boolean) packet.get(NS_STARTING_NOW);
+      if (startingNow == null) {
+        startingNow = Boolean.TRUE;
+      }
 
       Set<String> domainUIDs = new HashSet<>();
       if (callResponse.getResult() != null) {
         for (Domain dom : callResponse.getResult().getItems()) {
           String domainUID = dom.getSpec().getDomainUID();
           domainUIDs.add(domainUID);
           DomainPresenceInfo info = DomainPresenceInfoManager.getOrCreate(dom);
-          if (starting) {
+          if (startingNow) {
             // Update domain here if namespace is not yet running
             info.setDomain(dom);
           }

operator/src/test/java/oracle/kubernetes/operator/DomainPresenceTest.java

@@ -96,7 +96,9 @@ public void setUp() throws Exception {
     mementos.add(
         installStub(DomainProcessor.class, "FIBER_GATE", testSupport.createFiberGateStub()));
 
+    getStartedVariable();
     isNamespaceStopping = getStoppingVariable();
+    isNamespaceStopping.computeIfAbsent(NS, k -> new AtomicBoolean(true)).set(true);
   }
 
   private Map getDomainPresenceInfoMap() throws NoSuchFieldException {
@@ -109,14 +111,19 @@ private static Memento installStub(Class<?> containingClass, String fieldName, O
     return StaticStubSupport.install(containingClass, fieldName, newValue);
   }
 
+  private Map<String, AtomicBoolean> getStartedVariable() throws NoSuchFieldException {
+    Memento startedMemento = StaticStubSupport.preserve(Main.class, "isNamespaceStarted");
+    return startedMemento.getOriginalValue();
+  }
+
   private Map<String, AtomicBoolean> getStoppingVariable() throws NoSuchFieldException {
     Memento stoppingMemento = StaticStubSupport.preserve(Main.class, "isNamespaceStopping");
     return stoppingMemento.getOriginalValue();
   }
 
   @After
   public void tearDown() throws Exception {
-    isNamespaceStopping.forEach((key, value) -> value.set(true));
+    isNamespaceStopping.computeIfAbsent(NS, k -> new AtomicBoolean(true)).set(true);
     shutDownThreads();
 
     for (Memento memento : mementos) memento.revert();
@@ -133,7 +140,7 @@ public void whenNoPreexistingDomains_createEmptyDomainPresenceInfoMap() {
 
   private void readExistingResources() {
     createCannedListDomainResponses();
-    testSupport.runSteps(Main.readExistingResources("operator", NS));
+    testSupport.runStepsToCompletion(Main.readExistingResources("operator", NS));
   }
 
   @Test
@@ -477,6 +484,8 @@ public void whenStrandedResourcesExist_removeThem() {
         .ignoringBody()
         .returning(new V1Status());
 
+    isNamespaceStopping.get(NS).set(false);
+
     readExistingResources();
 
     testSupport.verifyAllDefinedResponsesInvoked();

operator/src/test/java/oracle/kubernetes/operator/work/FiberTestSupport.java

@@ -11,6 +11,7 @@
 import java.util.Queue;
 import java.util.SortedSet;
 import java.util.TreeSet;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
@@ -119,6 +120,24 @@ public FiberTestSupport addVersion(KubernetesVersion kubernetesVersion) {
    */
   public Packet runSteps(Step step) {
     fiber.start(step, packet, completionCallback);
+
+    return packet;
+  }
+
+  /**
+   * Starts a unit-test fiber with the specified step and runs until the fiber is done
+   *
+   * @param step the first step to run
+   */
+  public Packet runStepsToCompletion(Step step) {
+    fiber.start(step, packet, completionCallback);
+
+    // Wait for fiber to finish
+    try {
+      fiber.get();
+    } catch (InterruptedException | ExecutionException e) {
+      throw new RuntimeException(e);
+    }
     return packet;
   }