Update config maps based on hash like other resources (#2321)

* Update config maps based on hash like other resources

Author: rjeberhard

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

@@ -28,6 +28,7 @@
 import io.kubernetes.client.openapi.models.V1beta1PodDisruptionBudgetList;
 import oracle.kubernetes.operator.TuningParameters.WatchTuning;
 import oracle.kubernetes.operator.helpers.ConfigMapHelper;
+import oracle.kubernetes.operator.helpers.SemanticVersion;
 import oracle.kubernetes.operator.watcher.WatchListener;
 import oracle.kubernetes.operator.work.Step;
 import oracle.kubernetes.operator.work.ThreadFactorySingleton;
@@ -64,6 +65,8 @@ public class DomainNamespaces {
   private final WatcherControl<V1beta1PodDisruptionBudget, PodDisruptionBudgetWatcher> podDisruptionBudgetWatchers
           = new WatcherControl<>(PodDisruptionBudgetWatcher::create, d -> d::dispatchPodDisruptionBudgetWatch);
 
+  private final SemanticVersion productVersion;
+
   AtomicBoolean isStopping(String ns) {
     return namespaceStoppingMap.computeIfAbsent(ns, (key) -> new AtomicBoolean(false));
   }
@@ -78,9 +81,10 @@ boolean isStarting(String ns) {
   /**
    * Constructs a DomainNamespace object.
    */
-  DomainNamespaces() {
+  DomainNamespaces(SemanticVersion productVersion) {
     namespaceStatuses.clear();
     namespaceStoppingMap.clear();
+    this.productVersion = productVersion;
   }
 
   /**
@@ -176,7 +180,7 @@ Step readExistingResources(String ns, DomainProcessor processor) {
     NamespacedResources resources = new NamespacedResources(ns, null);
     resources.addProcessing(new DomainResourcesValidation(ns, processor).getProcessors());
     resources.addProcessing(createWatcherStartupProcessing(ns, processor));
-    return Step.chain(ConfigMapHelper.createScriptConfigMapStep(ns), resources.createListSteps());
+    return Step.chain(ConfigMapHelper.createScriptConfigMapStep(ns, productVersion), resources.createListSteps());
   }
 
   public boolean shouldStartNamespace(String ns) {

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

@@ -49,6 +49,7 @@
 import oracle.kubernetes.operator.helpers.NamespaceHelper;
 import oracle.kubernetes.operator.helpers.PodDisruptionBudgetHelper;
 import oracle.kubernetes.operator.helpers.PodHelper;
+import oracle.kubernetes.operator.helpers.SemanticVersion;
 import oracle.kubernetes.operator.helpers.ServiceHelper;
 import oracle.kubernetes.operator.logging.LoggingContext;
 import oracle.kubernetes.operator.logging.LoggingFacade;
@@ -98,6 +99,7 @@ public class DomainProcessorImpl implements DomainProcessor {
   private static Map<String, Map<String, DomainPresenceInfo>> DOMAINS = new ConcurrentHashMap<>();
   private static final Map<String, Map<String, ScheduledFuture<?>>> statusUpdaters = new ConcurrentHashMap<>();
   private final DomainProcessorDelegate delegate;
+  private final SemanticVersion productVersion;
 
   // Map namespace to map of domainUID to KubernetesEventObjects; tests may replace this value.
   @SuppressWarnings({"FieldMayBeFinal", "CanBeFinal"})
@@ -108,7 +110,12 @@ public class DomainProcessorImpl implements DomainProcessor {
   private static Map<String, KubernetesEventObjects> namespaceEventK8SObjects = new ConcurrentHashMap<>();
 
   public DomainProcessorImpl(DomainProcessorDelegate delegate) {
+    this(delegate, null);
+  }
+
+  public DomainProcessorImpl(DomainProcessorDelegate delegate, SemanticVersion productVersion) {
     this.delegate = delegate;
+    this.productVersion = productVersion;
   }
 
   private static DomainPresenceInfo getExistingDomainPresenceInfo(String ns, String domainUid) {
@@ -530,7 +537,7 @@ public void dispatchConfigMapWatch(Watch.Response<V1ConfigMap> item) {
         case "DELETED":
           delegate.runSteps(
               ConfigMapHelper.createScriptConfigMapStep(
-                    c.getMetadata().getNamespace()));
+                    c.getMetadata().getNamespace(), productVersion));
           break;
 
         case "ERROR":

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

@@ -56,7 +56,7 @@ class DomainRecheck {
     this(domainProcessor, domainNamespaces, false);
   }
 
-  private DomainRecheck(DomainProcessor domainProcessor, DomainNamespaces domainNamespaces, boolean fullRecheck) {
+  DomainRecheck(DomainProcessor domainProcessor, DomainNamespaces domainNamespaces, boolean fullRecheck) {
     this.domainProcessor = domainProcessor;
     this.domainNamespaces = domainNamespaces;
     this.fullRecheck = fullRecheck;

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

@@ -134,7 +134,7 @@ static class MainDelegateImpl implements MainDelegate, DomainProcessorDelegate {
     private final KubernetesVersion kubernetesVersion;
     private final Engine engine;
     private final DomainProcessor domainProcessor;
-    private final DomainNamespaces domainNamespaces = new DomainNamespaces();
+    private final DomainNamespaces domainNamespaces;
 
     public MainDelegateImpl(Properties buildProps, ScheduledExecutorService scheduledExecutorService) {
       buildVersion = getBuildVersion(buildProps);
@@ -145,7 +145,9 @@ public MainDelegateImpl(Properties buildProps, ScheduledExecutorService schedule
       kubernetesVersion = HealthCheckHelper.performK8sVersionCheck();
 
       engine = new Engine(scheduledExecutorService);
-      domainProcessor = new DomainProcessorImpl(this);
+      domainProcessor = new DomainProcessorImpl(this, productVersion);
+
+      domainNamespaces = new DomainNamespaces(productVersion);
 
       PodHelper.setProductVersion(productVersion.toString());
     }

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

@@ -116,7 +116,8 @@ public NextAction apply(Packet packet) {
 
     @Nonnull
     private Step readExistingNamespaces() {
-      return mainDelegate.getDomainNamespaces().readExistingResources(namespace, mainDelegate.getDomainProcessor());
+      return mainDelegate.getDomainNamespaces().readExistingResources(
+          namespace, mainDelegate.getDomainProcessor());
     }
 
     private Step createForcedDeletePodStep(V1Pod pod) {

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

@@ -16,7 +16,6 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
-import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import javax.annotation.Nonnull;
 
@@ -69,7 +68,7 @@ public class ConfigMapHelper {
 
   private static final String SCRIPT_LOCATION = "/scripts";
   private static final String UPDATEDOMAINRESULT = "UPDATEDOMAINRESULT";
-  private static final ConfigMapComparator COMPARATOR = new ConfigMapComparatorImpl();
+  private static final ConfigMapComparator COMPARATOR = new ConfigMapComparator();
 
   private static final FileGroupReader scriptReader = new FileGroupReader(SCRIPT_LOCATION);
 
@@ -82,12 +81,8 @@ private ConfigMapHelper() {
    * @param domainNamespace the domain's namespace
    * @return Step for creating config map containing scripts
    */
-  public static Step createScriptConfigMapStep(String domainNamespace) {
-    return new ScriptConfigMapStep(domainNamespace);
-  }
-
-  static FileGroupReader getScriptReader() {
-    return scriptReader;
+  public static Step createScriptConfigMapStep(String domainNamespace, SemanticVersion productVersion) {
+    return new ScriptConfigMapStep(domainNamespace, productVersion);
   }
 
   static Map<String, String> parseIntrospectorResult(String text, String domainUid) {
@@ -162,23 +157,25 @@ public static String getIntrospectorConfigMapName(String domainUid) {
     return IntrospectorConfigMapConstants.getIntrospectorConfigMapName(domainUid, 0);
   }
 
-  abstract static class ConfigMapComparator {
-    boolean containsAll(V1ConfigMap actual, V1ConfigMap expected) {
-      return containsAllData(getData(actual), getData(expected));
-    }
+  static class ConfigMapComparator {
+    boolean isOutdated(SemanticVersion productVersion, V1ConfigMap actual, V1ConfigMap expected) {
+      // Check product version label
+      if (productVersion != null) {
+        SemanticVersion currentVersion = KubernetesUtils.getProductVersionFromMetadata(actual.getMetadata());
+        if (currentVersion == null || productVersion.compareTo(currentVersion) > 0) {
+          return true;
+        }
+      }
 
-    private Map<String,String> getData(V1ConfigMap map) {
-      return Optional.ofNullable(map).map(V1ConfigMap::getData).orElse(Collections.emptyMap());
+      return !AnnotationHelper.getHash(expected).equals(AnnotationHelper.getHash(actual));
     }
-
-    abstract boolean containsAllData(Map<String, String> actual, Map<String, String> expected);
   }
 
   static class ScriptConfigMapStep extends Step {
     final ConfigMapContext context;
 
-    ScriptConfigMapStep(String domainNamespace) {
-      context = new ScriptConfigMapContext(this, domainNamespace);
+    ScriptConfigMapStep(String domainNamespace, SemanticVersion productVersion) {
+      context = new ScriptConfigMapContext(this, domainNamespace, productVersion);
     }
 
     @Override
@@ -188,25 +185,23 @@ public NextAction apply(Packet packet) {
   }
 
   static class ScriptConfigMapContext extends ConfigMapContext {
-
-    ScriptConfigMapContext(Step conflictStep, String domainNamespace) {
-      super(conflictStep, SCRIPT_CONFIG_MAP_NAME, domainNamespace, loadScriptsFromClasspath(domainNamespace), null);
+    ScriptConfigMapContext(Step conflictStep, String domainNamespace, SemanticVersion productVersion) {
+      super(conflictStep, SCRIPT_CONFIG_MAP_NAME, domainNamespace,
+          loadScriptsFromClasspath(domainNamespace), null, productVersion);
 
       addLabel(LabelConstants.OPERATORNAME_LABEL, getOperatorNamespace());
     }
 
-    private static synchronized Map<String, String> loadScriptsFromClasspath(String domainNamespace) {
-      Map<String, String> scripts = scriptReader.loadFilesFromClasspath();
-      LOGGER.fine(MessageKeys.SCRIPT_LOADED, domainNamespace);
-      return scripts;
-    }
-
     @Override
     void recordCurrentMap(Packet packet, V1ConfigMap configMap) {
       packet.put(ProcessingConstants.SCRIPT_CONFIG_MAP, configMap);
     }
+  }
 
-
+  static synchronized Map<String, String> loadScriptsFromClasspath(String domainNamespace) {
+    Map<String, String> scripts = scriptReader.loadFilesFromClasspath();
+    LOGGER.fine(MessageKeys.SCRIPT_LOADED, domainNamespace);
+    return scripts;
   }
 
   abstract static class ConfigMapContext extends StepContextBase {
@@ -216,14 +211,21 @@ abstract static class ConfigMapContext extends StepContextBase {
     private final String namespace;
     private V1ConfigMap model;
     private final Map<String, String> labels = new HashMap<>();
+    protected final SemanticVersion productVersion;
 
     ConfigMapContext(Step conflictStep, String name, String namespace, Map<String, String> contents,
                      DomainPresenceInfo info) {
+      this(conflictStep, name, namespace, contents, info, null);
+    }
+
+    ConfigMapContext(Step conflictStep, String name, String namespace, Map<String, String> contents,
+                     DomainPresenceInfo info, SemanticVersion productVersion) {
       super(info);
       this.conflictStep = conflictStep;
       this.name = name;
       this.namespace = namespace;
       this.contents = contents;
+      this.productVersion = productVersion;
 
       addLabel(LabelConstants.CREATEDBYOPERATOR_LABEL, "true");
     }
@@ -253,15 +255,22 @@ protected V1ConfigMap getModel() {
     }
 
     protected final V1ConfigMap createModel(Map<String, String> data) {
-      return new V1ConfigMap().kind("ConfigMap").apiVersion("v1").metadata(createMetadata()).data(data);
+      return AnnotationHelper.withSha256Hash(
+          new V1ConfigMap().kind("ConfigMap").apiVersion("v1").metadata(createMetadata()).data(data), data);
     }
 
     private V1ObjectMeta createMetadata() {
-      return updateForOwnerReference(
+      V1ObjectMeta metadata = updateForOwnerReference(
           new V1ObjectMeta()
           .name(name)
           .namespace(namespace)
           .labels(labels));
+
+      if (productVersion != null) {
+        metadata.putLabelsItem(LabelConstants.OPERATOR_VERSION, productVersion.toString());
+      }
+
+      return metadata;
     }
 
     @SuppressWarnings("SameParameterValue")
@@ -283,8 +292,8 @@ Step verifyConfigMap(Step next) {
       return new CallBuilder().readConfigMapAsync(getName(), namespace, null, new ReadResponseStep(next));
     }
 
-    boolean isIncompatibleMap(V1ConfigMap existingMap) {
-      return !COMPARATOR.containsAll(existingMap, getModel());
+    boolean isOutdated(V1ConfigMap existingMap) {
+      return COMPARATOR.isOutdated(productVersion, existingMap, getModel());
     }
 
     V1ConfigMap withoutTransientData(V1ConfigMap originalMap) {
@@ -315,8 +324,8 @@ public NextAction onSuccess(Packet packet, CallResponse<V1ConfigMap> callRespons
         V1ConfigMap existingMap = withoutTransientData(callResponse.getResult());
         if (existingMap == null) {
           return doNext(createConfigMap(getNext()), packet);
-        } else if (isIncompatibleMap(existingMap)) {
-          return doNext(updateConfigMap(getNext(), existingMap), packet);
+        } else if (isOutdated(existingMap)) {
+          return doNext(replaceConfigMap(getNext()), packet);
         } else if (mustPatchCurrentMap(existingMap)) {
           return doNext(patchCurrentMap(existingMap, getNext()), packet);
         } else {
@@ -335,9 +344,9 @@ private void logConfigMapExists() {
         LOGGER.fine(MessageKeys.CM_EXISTS, getName(), namespace);
       }
 
-      private Step updateConfigMap(Step next, V1ConfigMap existingConfigMap) {
+      private Step replaceConfigMap(Step next) {
         return new CallBuilder().replaceConfigMapAsync(name, namespace,
-                                        createModel(getCombinedData(existingConfigMap)),
+                                        model,
                                         createReplaceResponseStep(next));
       }
 
@@ -370,12 +379,6 @@ private boolean labelsNotDefined(V1ConfigMap currentMap) {
       }
     }
 
-    private Map<String, String> getCombinedData(V1ConfigMap existingConfigMap) {
-      Map<String, String> updated = Objects.requireNonNull(existingConfigMap.getData());
-      updated.putAll(contents);
-      return updated;
-    }
-
     private ResponseStep<V1ConfigMap> createCreateResponseStep(Step next) {
       return new CreateResponseStep(next);
     }
@@ -440,15 +443,6 @@ public NextAction onSuccess(Packet packet, CallResponse<V1ConfigMap> callRespons
 
   }
 
-  /** Returns true if the actual map contains all of the entries from the expected map. */
-  static class ConfigMapComparatorImpl extends ConfigMapComparator {
-
-    @Override
-    boolean containsAllData(Map<String, String> actual, Map<String, String> expected) {
-      return actual.entrySet().containsAll(expected.entrySet());
-    }
-  }
-
   /**
    * Factory for a step that creates or updates the generated domain config map from introspection results.
    * Reads the following packet fields:
@@ -596,7 +590,8 @@ private IntrospectorConfigMapContext createIntrospectorConfigMapContext() {
       return createIntrospectorConfigMapContext(data, 0);
     }
 
-    private IntrospectorConfigMapContext createIntrospectorConfigMapContext(Map<String, String> data, int index) {
+    private IntrospectorConfigMapContext createIntrospectorConfigMapContext(
+        Map<String, String> data, int index) {
       return new IntrospectorConfigMapContext(conflictStep, info, data, index);
     }
 
@@ -662,11 +657,10 @@ private String perLine(List<String> errors) {
 
   public static class IntrospectorConfigMapContext extends ConfigMapContext implements SplitterTarget {
 
-    private static final Pattern ENCODED_ZIP_PATTERN = Pattern.compile("([A-Za-z_]+)\\.secure");
-
     private boolean patchOnly;
 
-    IntrospectorConfigMapContext(Step conflictStep, DomainPresenceInfo info, Map<String, String> data, int index) {
+    IntrospectorConfigMapContext(Step conflictStep, DomainPresenceInfo info,
+                                 Map<String, String> data, int index) {
       super(conflictStep, getConfigMapName(info, index), info.getNamespace(), data, info);
 
       addLabel(LabelConstants.DOMAINUID_LABEL, info.getDomainUid());
@@ -687,8 +681,8 @@ IntrospectorConfigMapContext patchOnly() {
     }
 
     @Override
-    boolean isIncompatibleMap(V1ConfigMap existingMap) {
-      return !patchOnly && super.isIncompatibleMap(existingMap);
+    boolean isOutdated(V1ConfigMap existingMap) {
+      return !patchOnly && super.isOutdated(existingMap);
     }
 
     @Override