Chore: Remove enrichAll flag from ImageChangeTriggerEnricher config

CHANGELOG.md

@@ -11,14 +11,16 @@ We use semantic versioning in some slight variation until our feature set has st
 After this we will switch probably to real [Semantic Versioning 2.0.0](http://semver.org/)
 
 ### 4.1-SNAPSHOT
+* Fix #1638: Remove enrichAll parameter from ImageChangeTriggerEnricher
 * Update Docker Maven Plugin to 0.29.0
-* Fix 872: Using spring.application.name as service name
-* Fix 1632: imagePullPolicy configuration fixed as per documentation
+* Fix #872: Using spring.application.name as service name
+* Fix #1632: imagePullPolicy configuration fixed as per documentation
 * Fix #1591: Add support for custom resources creation via resource fragments
 * Fix #1628: Adding back support for templates in kubernetes mode to HelmMojo; Fix for NPE in UndeployMojo
-* Fix 1591: Add support for custom resources creation via resource fragments
-* Fix 1636: Added support to choose configMap name
-* Fix 1630: Fix CronJob Version
+* Fix #1591: Add support for custom resources creation via resource fragments
+* Fix #1636: Added support to choose configMap name
+* Fix #1630: Fix CronJob Version
+* Fix #1591: Add support for custom resources creation via resource fragments
 
 ### 4.1.0 (16-02-2019)
 * Fix 1578: Fmp is generating ImageChange triggers in the DeploymentConfig

core/src/main/java/io/fabric8/maven/core/util/kubernetes/KubernetesResourceUtil.java

@@ -686,6 +686,10 @@ public static void mergeSimpleFields(Object targetValues, Object defaultValues)
     }
 
     public static String mergePodSpec(PodSpecBuilder builder, PodSpec defaultPodSpec, String defaultName) {
+        return mergePodSpec(builder, defaultPodSpec, defaultName, false);
+    }
+
+    public static String mergePodSpec(PodSpecBuilder builder, PodSpec defaultPodSpec, String defaultName, boolean sidecarEnabled) {
         String defaultApplicationContainerName = null;
         List<Container> containers = builder.buildContainers();
         List<Container> defaultContainers = defaultPodSpec.getContainers();
@@ -694,20 +698,30 @@ public static String mergePodSpec(PodSpecBuilder builder, PodSpec defaultPodSpec
             if (containers == null || containers.isEmpty()) {
                 builder.addToContainers(defaultContainers.toArray(new Container[size]));
             } else {
+                int idx = 0;
                 for (Container defaultContainer : defaultContainers) {
                     Container container = null;
-                    for (Container fragmentContainer : containers) {
-                        if (fragmentContainer.getName() == null || fragmentContainer.getName().equals(defaultContainer.getName())) {
-                            container = fragmentContainer;
-                            defaultApplicationContainerName = defaultContainer.getName();
-                            break;
+                    if(sidecarEnabled) { // Consider container as sidecar
+                        for (Container fragmentContainer : containers) {
+                            if (fragmentContainer.getName() == null || fragmentContainer.getName().equals(defaultContainer.getName())) {
+                                container = fragmentContainer;
+                                defaultApplicationContainerName = defaultContainer.getName();
+                                break;
+                            }
+                        }
+                        if (container == null) {
+                            container = new Container();
+                            containers.add(container);
+                        }
+                    } else { // Old behavior
+                        if (idx < containers.size()) {
+                            container = containers.get(idx);
+                        } else {
+                            container = new Container();
+                            containers.add(container);
                         }
                     }
 
-                    if (container == null) {
-                        container = new Container();
-                        containers.add(container);
-                    }
                     mergeSimpleFields(container, defaultContainer);
                     List<EnvVar> defaultEnv = defaultContainer.getEnv();
                     if (defaultEnv != null) {
@@ -730,6 +744,7 @@ public static String mergePodSpec(PodSpecBuilder builder, PodSpec defaultPodSpec
                     if (container.getSecurityContext() == null) {
                         container.setSecurityContext(defaultContainer.getSecurityContext());
                     }
+                    idx++;
                 }
                 builder.withContainers(containers);
             }

enricher/api/src/main/java/io/fabric8/maven/enricher/api/BaseEnricher.java

@@ -135,6 +135,14 @@ protected Boolean isAutomaticTriggerEnabled(MavenEnricherContext enricherContext
         }
     }
 
+    protected Boolean enrichAllWithImageChangeTrigger(MavenEnricherContext enricherContext, Boolean defaultValue) {
+        if(enricherContext.getProperty("fabric8.openshift.enrichAllWithImageChangeTrigger") != null) {
+            return Boolean.parseBoolean(enricherContext.getProperty("fabric8.openshift.enrichAllWithImageChangeTrigger").toString());
+        } else {
+            return defaultValue;
+        }
+    }
+
     protected Long getOpenshiftDeployTimeoutInSeconds(MavenEnricherContext enricherContext, Long defaultValue) {
         if (enricherContext.getProperty("fabric8.openshift.deployTimeoutSeconds") != null) {
             return Long.parseLong(enricherContext.getProperty("fabric8.openshift.deployTimeoutSeconds").toString());
@@ -159,6 +167,14 @@ protected Boolean getTrimImageInContainerSpecFlag(Boolean defaultValue) {
         }
     }
 
+    protected Boolean getSidecarFlag(Boolean defaultValue) {
+        if (getContext().getProperty("fabric8.sidecar") != null) {
+            return Boolean.parseBoolean(getContext().getProperty("fabric8.sidecar").toString());
+        } else {
+            return defaultValue;
+        }
+    }
+
     /**
      * This method overrides the ImagePullPolicy value by the value provided in
      * XML config.

enricher/standard/src/main/java/io/fabric8/maven/enricher/standard/ControllerViaPluginConfigurationEnricher.java

@@ -96,7 +96,7 @@ public void visit(StatefulSetBuilder statefulSetBuilder) {
                         builder.accept(new TypedVisitor<PodSpecBuilder>() {
                             @Override
                             public void visit(PodSpecBuilder builder) {
-                                String defaultApplicationContainerName = KubernetesResourceUtil.mergePodSpec(builder, podSpec, name);
+                                String defaultApplicationContainerName = KubernetesResourceUtil.mergePodSpec(builder, podSpec, name, getSidecarFlag(false));
                                 if(defaultApplicationContainerName != null) {
                                     setProcessingInstruction(NEED_IMAGECHANGE_TRIGGERS, Collections.singletonList(defaultApplicationContainerName));
                                 }
@@ -120,7 +120,7 @@ public void visit(DeploymentBuilder deploymentBuilder) {
                         builder.accept(new TypedVisitor<PodSpecBuilder>() {
                             @Override
                             public void visit(PodSpecBuilder builder) {
-                                String defaultApplicationContainerName = KubernetesResourceUtil.mergePodSpec(builder, podSpec, name);
+                                String defaultApplicationContainerName = KubernetesResourceUtil.mergePodSpec(builder, podSpec, name, getSidecarFlag(false));
                                 if(defaultApplicationContainerName != null) {
                                     setProcessingInstruction(NEED_IMAGECHANGE_TRIGGERS, Collections.singletonList(defaultApplicationContainerName));
                                 }

enricher/standard/src/main/java/io/fabric8/maven/enricher/standard/openshift/ImageChangeTriggerEnricher.java

@@ -42,8 +42,7 @@ public class ImageChangeTriggerEnricher extends BaseEnricher {
 
 
     private enum Config implements Configs.Key {
-        containers {{ d = ""; }},
-        enrichAll {{ d = "false"; }};
+        containers {{ d = ""; }};
 
         public String def() { return d; } protected String d;
     }
@@ -69,7 +68,11 @@ public void visit(DeploymentConfigSpecBuilder builder) {
                         PodSpec podSpec = template.getSpec();
                         Objects.requireNonNull(podSpec, "No PodSpec for PodTemplate:" + template);
                         List<Container> containers = podSpec.getContainers();
-                        containerToImageMap = containers.stream().collect(Collectors.toMap(Container::getName, Container::getImage));
+                        for(Container container : containers) {
+                            if(container.getName() != null && container.getImage() != null) {
+                                containerToImageMap.put(container.getName(), container.getImage());
+                            }
+                        }
                     }
                     // add a new image change trigger for the build stream
                     if (containerToImageMap.size() != 0) {
@@ -95,19 +98,19 @@ public void visit(DeploymentConfigSpecBuilder builder) {
                                         .endImageChangeParams()
                                         .endTrigger();
                             }
+                            if(trimImageInContainerSpecFlag) {
+                                builder.editTemplate().editSpec().withContainers(trimImagesInContainers(template)).endSpec().endTemplate();
+                            }
                         }
 
-                        if(trimImageInContainerSpecFlag) {
-                            builder.editTemplate().editSpec().withContainers(trimImagesInContainers(template)).endSpec().endTemplate();
-                        }
                     }
                 }
             });
     }
 
     private Boolean isImageChangeTriggerNeeded(String containerName) {
         String containersFromConfig = Configs.asString(getConfig(Config.containers));
-        Boolean enrichAll = Configs.asBoolean(getConfig(Config.enrichAll));
+        Boolean enrichAll = enrichAllWithImageChangeTrigger((MavenEnricherContext)enricherContext, false);
 
         if(enrichAll) {
             return true;

it/src/it/sidecar/invoker.properties

@@ -15,6 +15,6 @@
 #
 
 invoker.goals.1=clean fabric8:resource
-invoker.mavenOpts=-Dfabric8.verbose -Dfabric8.mode=kubernetes
+invoker.mavenOpts=-Dfabric8.verbose -Dfabric8.mode=kubernetes -Dfabric8.sidecar=true
 #-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=5005
-invoker.debug=false
+invoker.debug=false

plugin/src/main/java/io/fabric8/maven/plugin/mojo/build/ResourceMojo.java

@@ -221,6 +221,9 @@ public class ResourceMojo extends AbstractFabric8Mojo {
     @Parameter(property = "fabric8.namespace")
     private String namespace;
 
+    @Parameter(property = "fabric8.sidecar", defaultValue = "false")
+    private Boolean sidecar;
+
     @Parameter(property = "fabric8.skipHealthCheck", defaultValue = "false")
     private Boolean skipHealthCheck;
 
@@ -263,6 +266,9 @@ public class ResourceMojo extends AbstractFabric8Mojo {
     @Parameter(property = "fabric8.openshift.imageChangeTrigger", defaultValue = "true")
     private Boolean enableImageChangeTrigger;
 
+    @Parameter(property = "fabric8.openshift.enrichAllWithImageChangeTrigger", defaultValue = "false")
+    private Boolean erichAllWithImageChangeTrigger;
+
     @Parameter(property = "docker.skip.resource", defaultValue = "false")
     protected boolean skipResource;