Owls88781 continue retry start a namespace in non-fullcheck case after hitting 403

documentation/staging/content/userguide/managing-domains/domain-events.md

@@ -38,6 +38,7 @@ The operator also generates these event types in the operator's namespace, which
 
 *  `StartManagingNamespace`: The operator has started managing domains in a namespace.
 *  `StopManagingNamespace`: The operator has stopped managing domains in a namespace. 
+*  `StartManagingNamespaceFailed`: The operator failed to start managing domains in a namespace because it does not have the required privileges.
 
 #### Operator-generated event details
 

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

@@ -179,10 +179,14 @@ Step readExistingResources(String ns, DomainProcessor processor) {
     return Step.chain(ConfigMapHelper.createScriptConfigMapStep(ns), resources.createListSteps());
   }
 
-  boolean shouldStartNamespace(String ns) {
+  public boolean shouldStartNamespace(String ns) {
     return getNamespaceStatus(ns).shouldStartNamespace();
   }
 
+  public void clearNamespaceStartingFlag(String ns) {
+    getNamespaceStatus(ns).clearNamespaceStartingFlag();
+  }
+
   interface WatcherFactory<T, W extends Watcher<T>> {
     W create(
           ThreadFactory threadFactory,

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

@@ -36,7 +36,6 @@
 
 import static oracle.kubernetes.operator.helpers.EventHelper.EventItem.NAMESPACE_WATCHING_STARTED;
 import static oracle.kubernetes.operator.helpers.NamespaceHelper.getOperatorNamespace;
-import static oracle.kubernetes.operator.logging.MessageKeys.BEGIN_MANAGING_NAMESPACE;
 
 class DomainRecheck {
   private static final LoggingFacade LOGGER = LoggingFactory.getLogger("Operator", "Operator");
@@ -218,6 +217,11 @@ private Step startNamespaceSteps(String ns) {
           domainNamespaces.readExistingResources(ns, domainProcessor));
   }
 
+  // for testing
+  public Step createStartNamespaceBeforeStep(String ns) {
+    return new StartNamespaceBeforeStep(ns);
+  }
+
   private class StartNamespaceBeforeStep extends Step {
 
     private final String ns;
@@ -228,11 +232,11 @@ private class StartNamespaceBeforeStep extends Step {
 
     @Override
     public NextAction apply(Packet packet) {
+      if (domainNamespaces.shouldStartNamespace(ns)) {
+        return doNext(addNSWatchingStartingEventsStep(), packet);
+      }
       if (fullRecheck) {
         return doNext(packet);
-      } else if (domainNamespaces.shouldStartNamespace(ns)) {
-        LOGGER.info(BEGIN_MANAGING_NAMESPACE, ns);
-        return doNext(addNSWatchingStartingEventsStep(), packet);
       } else {
         return doEnd(packet);
       }
@@ -241,7 +245,7 @@ public NextAction apply(Packet packet) {
     private Step addNSWatchingStartingEventsStep() {
       return Step.chain(
           EventHelper.createEventStep(
-              new EventData(NAMESPACE_WATCHING_STARTED).namespace(ns).resourceName(ns)),
+              domainNamespaces, new EventData(NAMESPACE_WATCHING_STARTED).namespace(ns).resourceName(ns), null),
           EventHelper.createEventStep(
               new EventData(EventHelper.EventItem.START_MANAGING_NAMESPACE)
                   .namespace(getOperatorNamespace()).resourceName(ns)),

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

@@ -45,4 +45,6 @@ public interface EventConstants {
   String START_MANAGING_NAMESPACE_PATTERN = "Start managing namespace %s";
   String STOP_MANAGING_NAMESPACE_EVENT = "StopManagingNamespace";
   String STOP_MANAGING_NAMESPACE_PATTERN = "Stop managing namespace %s";
+  String START_MANAGING_NAMESPACE_FAILED_EVENT = "StartManagingNamespaceFailed";
+  String START_MANAGING_NAMESPACE_FAILED_PATTERN = "Start managing namespace %s failed due to an authorization error";
 }

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

@@ -33,4 +33,8 @@ public AtomicReference<V1SubjectRulesReviewStatus> getRulesReviewStatus() {
   boolean shouldStartNamespace() {
     return !isNamespaceStarting.getAndSet(true);
   }
+
+  public void clearNamespaceStartingFlag() {
+    isNamespaceStarting.set(false);
+  }
 }

operator/src/main/java/oracle/kubernetes/operator/helpers/EventHelper.java

@@ -11,6 +11,7 @@
 import io.kubernetes.client.openapi.models.V1ObjectMeta;
 import io.kubernetes.client.openapi.models.V1ObjectReference;
 import jakarta.validation.constraints.NotNull;
+import oracle.kubernetes.operator.DomainNamespaces;
 import oracle.kubernetes.operator.DomainProcessorImpl;
 import oracle.kubernetes.operator.EventConstants;
 import oracle.kubernetes.operator.KubernetesConstants;
@@ -52,10 +53,12 @@
 import static oracle.kubernetes.operator.helpers.EventHelper.EventItem.DOMAIN_PROCESSING_ABORTED;
 import static oracle.kubernetes.operator.helpers.EventHelper.EventItem.DOMAIN_PROCESSING_COMPLETED;
 import static oracle.kubernetes.operator.helpers.EventHelper.EventItem.DOMAIN_PROCESSING_STARTING;
+import static oracle.kubernetes.operator.helpers.EventHelper.EventItem.NAMESPACE_WATCHING_STARTED;
 import static oracle.kubernetes.operator.helpers.EventHelper.EventItem.NAMESPACE_WATCHING_STOPPED;
 import static oracle.kubernetes.operator.helpers.NamespaceHelper.getOperatorNamespace;
 import static oracle.kubernetes.operator.helpers.NamespaceHelper.getOperatorPodName;
 import static oracle.kubernetes.operator.helpers.NamespaceHelper.getOperatorPodUID;
+import static oracle.kubernetes.operator.logging.MessageKeys.BEGIN_MANAGING_NAMESPACE;
 
 /** A Helper Class for the operator to create Kubernetes Events at the key points in the operator's workflow. */
 public class EventHelper {
@@ -81,6 +84,18 @@ public static Step createEventStep(EventData eventData) {
     return new CreateEventStep(eventData);
   }
 
+  /**
+   * Factory for {@link Step} that asynchronously create an event.
+   *
+   * @param domainNamespaces DomainSpaces instance
+   * @param eventData event item
+   * @param next next step
+   * @return Step for creating an event
+   */
+  public static Step createEventStep(DomainNamespaces domainNamespaces, EventData eventData, Step next) {
+    return new CreateEventStep(domainNamespaces, eventData, next);
+  }
+
   /**
    * Factory for {@link Step} that asynchronously create an event.
    *
@@ -89,19 +104,21 @@ public static Step createEventStep(EventData eventData) {
    * @return Step for creating an event
    */
   public static Step createEventStep(EventData eventData, Step next) {
-    return new CreateEventStep(eventData, next);
+    return new CreateEventStep(null, eventData, next);
   }
 
   public static class CreateEventStep extends Step {
     private final EventData eventData;
+    private final DomainNamespaces domainNamespaces;
 
     CreateEventStep(EventData eventData) {
-      this(eventData, null);
+      this(null, eventData, null);
     }
 
-    CreateEventStep(EventData eventData, Step next) {
+    CreateEventStep(DomainNamespaces domainNamespaces, EventData eventData, Step next) {
       super(next);
       this.eventData = eventData;
+      this.domainNamespaces = domainNamespaces;
     }
 
     @Override
@@ -166,6 +183,11 @@ private class CreateEventResponseStep extends ResponseStep<CoreV1Event> {
 
       @Override
       public NextAction onSuccess(Packet packet, CallResponse<CoreV1Event> callResponse) {
+        if (NAMESPACE_WATCHING_STARTED == eventData.eventItem) {
+          LOGGER.info(BEGIN_MANAGING_NAMESPACE, eventData.getNamespace());
+          domainNamespaces.shouldStartNamespace(eventData.getNamespace());
+        }
+
         Optional.ofNullable(packet.getSpi(DomainPresenceInfo.class))
             .ifPresent(dpi -> dpi.setLastEventItem(eventData.eventItem));
         return doNext(packet);
@@ -178,9 +200,31 @@ public NextAction onFailure(Packet packet, CallResponse<CoreV1Event> callRespons
               eventData.eventItem.getReason(), eventData.getNamespace());
           return doNext(packet);
         }
+
+        if (NAMESPACE_WATCHING_STARTED == eventData.eventItem) {
+          clearNamespaceStartingFlag();
+          if (isForbidden(callResponse)) {
+            LOGGER.warning(MessageKeys.CREATING_EVENT_FORBIDDEN,
+                eventData.eventItem.getReason(), eventData.getNamespace());
+            return doNext(createStartManagingNSFailedEventStep(), packet);
+          }
+        }
+
         return super.onFailure(packet, callResponse);
       }
 
+      private Step createStartManagingNSFailedEventStep() {
+        return createEventStep(
+            new EventData(EventItem.START_MANAGING_NAMESPACE_FAILED)
+                .namespace(getOperatorNamespace()).resourceName(eventData.getNamespace()));
+      }
+
+      private void clearNamespaceStartingFlag() {
+        if (domainNamespaces != null) {
+          domainNamespaces.clearNamespaceStartingFlag(eventData.getNamespace());
+        }
+      }
+
       private boolean isForbiddenForNamespaceWatchingStoppedEvent(CallResponse<CoreV1Event> callResponse) {
         return isForbidden(callResponse) && NAMESPACE_WATCHING_STOPPED == eventData.eventItem;
       }
@@ -483,6 +527,37 @@ protected String generateEventName(EventData eventData) {
         return generateOperatorNSEventName(eventData);
       }
     },
+    START_MANAGING_NAMESPACE_FAILED {
+      @Override
+      protected String getType() {
+        return EVENT_WARNING;
+      }
+
+      @Override
+      public String getReason() {
+        return EventConstants.START_MANAGING_NAMESPACE_FAILED_EVENT;
+      }
+
+      @Override
+      public String getPattern() {
+        return EventConstants.START_MANAGING_NAMESPACE_FAILED_PATTERN;
+      }
+
+      @Override
+      public void addLabels(V1ObjectMeta metadata, EventData eventData) {
+        addCreatedByOperatorLabel(metadata);
+      }
+
+      @Override
+      public V1ObjectReference createInvolvedObject(EventData eventData) {
+        return createOperatorEventInvolvedObject();
+      }
+
+      @Override
+      protected String generateEventName(EventData eventData) {
+        return generateOperatorNSEventName(eventData);
+      }
+    },
     STOP_MANAGING_NAMESPACE {
       @Override
       public String getReason() {

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

@@ -5,32 +5,44 @@
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
 
 import com.meterware.simplestub.Memento;
 import com.meterware.simplestub.Stub;
 import io.kubernetes.client.openapi.models.V1Namespace;
 import io.kubernetes.client.openapi.models.V1ObjectMeta;
 import oracle.kubernetes.operator.builders.StubWatchFactory;
+import oracle.kubernetes.operator.helpers.EventHelper;
+import oracle.kubernetes.operator.helpers.EventRetryStrategyStub;
 import oracle.kubernetes.operator.helpers.HelmAccessStub;
 import oracle.kubernetes.operator.helpers.KubernetesTestSupport;
 import oracle.kubernetes.operator.helpers.KubernetesVersion;
 import oracle.kubernetes.operator.helpers.SemanticVersion;
 import oracle.kubernetes.operator.helpers.TuningParametersStub;
 import oracle.kubernetes.utils.TestUtils;
 import oracle.kubernetes.weblogic.domain.model.Domain;
+import org.hamcrest.MatcherAssert;
 import org.jetbrains.annotations.NotNull;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import static com.meterware.simplestub.Stub.createStrictStub;
+import static java.net.HttpURLConnection.HTTP_FORBIDDEN;
 import static oracle.kubernetes.operator.DomainProcessorTestSetup.NS;
+import static oracle.kubernetes.operator.EventConstants.NAMESPACE_WATCHING_STARTED_EVENT;
+import static oracle.kubernetes.operator.helpers.EventHelper.EventItem.NAMESPACE_WATCHING_STARTED;
+import static oracle.kubernetes.operator.helpers.EventHelper.createEventStep;
 import static oracle.kubernetes.operator.helpers.HelmAccess.OPERATOR_DOMAIN_NAMESPACES;
 import static oracle.kubernetes.operator.helpers.KubernetesTestSupport.DOMAIN;
+import static oracle.kubernetes.operator.logging.MessageKeys.CREATING_EVENT_FORBIDDEN;
+import static oracle.kubernetes.utils.LogMatcher.containsWarning;
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.not;
@@ -49,6 +61,9 @@ public class NamespaceTest {
   private final DomainNamespaces domainNamespaces = new DomainNamespaces();
   private final DomainProcessorStub dp = Stub.createStub(DomainProcessorStub.class);
   private final MainDelegateStub delegate = createStrictStub(MainDelegateStub.class, dp, domainNamespaces);
+  private final TestUtils.ConsoleHandlerMemento loggerControl = TestUtils.silenceOperatorLogger();
+  private final Collection<LogRecord> logRecords = new ArrayList<>();
+  private final EventRetryStrategyStub retryStrategy = createStrictStub(EventRetryStrategyStub.class);
 
   @BeforeEach
   public void setUp() throws Exception {
@@ -174,6 +189,57 @@ public void whenDomainNamespaceDeleted_isNoLongerInManagedNamespaces() {
     assertThat(domainNamespaces.getNamespaces(), not(contains(NS)));
   }
 
+  @Test
+  public void whenStartNamespaceBeforeStepRunHit403OnEventCreation_namespaceStartingFlagCleared() {
+    String namespace = "TEST_NAMESPACE_1";
+    defineNamespaces(namespace);
+    specifyDomainNamespaces(namespace);
+
+    loggerControl.withLogLevel(Level.INFO).collectLogMessages(logRecords, CREATING_EVENT_FORBIDDEN);
+    testSupport.failOnCreate(KubernetesTestSupport.EVENT, null, namespace, HTTP_FORBIDDEN);
+    testSupport.runSteps(new DomainRecheck(delegate).createStartNamespaceBeforeStep(namespace));
+
+    MatcherAssert.assertThat(logRecords,
+        containsWarning(getMessage(CREATING_EVENT_FORBIDDEN, NAMESPACE_WATCHING_STARTED_EVENT, namespace)));
+    assertThat(domainNamespaces.isStarting(namespace), is(false));
+  }
+
+  @Test
+  public void whenStartNamespaceBeforeStepRunSucceeds_namespaceStartingFlagIsNotCleared() {
+    String namespace = "TEST_NAMESPACE_2";
+    defineNamespaces(namespace);
+    specifyDomainNamespaces(namespace);
+
+    loggerControl.withLogLevel(Level.INFO).collectLogMessages(logRecords, CREATING_EVENT_FORBIDDEN);
+    testSupport.runSteps(new DomainRecheck(delegate).createStartNamespaceBeforeStep(namespace));
+    assertThat(logRecords.isEmpty(), is(true));
+    assertThat(domainNamespaces.isStarting(namespace), is(true));
+  }
+
+  @Test
+  public void whenStartNamespaceBeforeStepRun403OnEventCreation_thenSucceed_namespaceStartingFlagSet() {
+    String namespace = "TEST_NAMESPACE_3";
+    testSupport.addRetryStrategy(retryStrategy);
+    defineNamespaces(namespace);
+    specifyDomainNamespaces(namespace);
+
+    loggerControl.collectLogMessages(logRecords, CREATING_EVENT_FORBIDDEN);
+    testSupport.failOnCreate(KubernetesTestSupport.EVENT, null, namespace, HTTP_FORBIDDEN);
+    testSupport.runSteps(new DomainRecheck(delegate).createStartNamespaceBeforeStep(namespace));
+    testSupport.cancelFailures();
+    testSupport.runSteps(createEventStep(delegate.domainNamespaces,
+        new EventHelper.EventData(NAMESPACE_WATCHING_STARTED)
+            .namespace(namespace)
+            .resourceName(namespace), null));
+    MatcherAssert.assertThat(logRecords,
+        containsWarning(getMessage(CREATING_EVENT_FORBIDDEN, NAMESPACE_WATCHING_STARTED_EVENT, namespace)));
+    assertThat(domainNamespaces.isStarting(namespace), is(true));
+  }
+
+  private String getMessage(String pattern, String event, String ns) {
+    return String.format(pattern, event, ns);
+  }
+
   private void addDomainNamespace(String namespace) {
     currentNamespaces.add(namespace);
     HelmAccessStub.defineVariable(OPERATOR_DOMAIN_NAMESPACES, String.join(",", currentNamespaces));