Add devfile property to allow enabling/disabling plugin merging

Add attribute 'mergePlugins' to devfile to manually enable/disable plugin
merging in the plugin broker. Default value when no property is present
is controlled by property che.workspace.plugin_broker.default_merge_plugins

Add a small note to the plugin broker images config lines in
che.properties to make it clear that the Che Operator overrides the
properties by default.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>

assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties

@@ -543,9 +543,18 @@ che.singleport.wildcard_domain.ipless=false
 
 # Docker image of Che plugin broker app that resolves workspace tooling configuration and copies
 # plugins dependencies to a workspace
+#
+# Note these images are overridden by the Che Operator by default; changing the images here will not
+# have an effect if Che is installed via Operator.
 che.workspace.plugin_broker.metadata.image=quay.io/eclipse/che-plugin-metadata-broker:v3.4.0
 che.workspace.plugin_broker.artifacts.image=quay.io/eclipse/che-plugin-artifacts-broker:v3.4.0
 
+# Configures the default behavior of the plugin brokers when provisioning plugins into a workspace.
+# If set to true, the plugin brokers will attempt to merge plugins when possible (i.e. they run in
+# the same sidecar image and do not have conflicting settings). This value is the default setting
+# used when the devfile does not specify otherwise, via the "mergePlugins" attribute.
+che.workspace.plugin_broker.default_merge_plugins=false
+
 # Docker image of Che plugin broker app that resolves workspace tooling configuration and copies
 # plugins dependencies to a workspace
 che.workspace.plugin_broker.pull_policy=Always

infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/KubernetesArtifactsBrokerApplier.java

@@ -50,10 +50,14 @@ public KubernetesArtifactsBrokerApplier(BrokerEnvironmentFactory<E> brokerEnviro
    * broker's configmap, machines, and volumes added in addition to the init container
    */
   public void apply(
-      E workspaceEnvironment, RuntimeIdentity runtimeID, Collection<PluginFQN> pluginFQNs)
+      E workspaceEnvironment,
+      RuntimeIdentity runtimeID,
+      Collection<PluginFQN> pluginFQNs,
+      boolean mergePlugins)
       throws InfrastructureException {
 
-    E brokerEnvironment = brokerEnvironmentFactory.createForArtifactsBroker(pluginFQNs, runtimeID);
+    E brokerEnvironment =
+        brokerEnvironmentFactory.createForArtifactsBroker(pluginFQNs, runtimeID, mergePlugins);
 
     Map<String, PodData> workspacePods = workspaceEnvironment.getPodsData();
     if (workspacePods.size() != 1) {

infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/PluginBrokerManager.java

@@ -102,14 +102,16 @@ public List<ChePlugin> getTooling(
       StartSynchronizer startSynchronizer,
       Collection<PluginFQN> pluginFQNs,
       boolean isEphemeral,
+      boolean mergePlugins,
       Map<String, String> startOptions)
       throws InfrastructureException {
 
     String workspaceId = identity.getWorkspaceId();
     KubernetesNamespace kubernetesNamespace = factory.getOrCreate(identity);
     BrokersResult brokersResult = new BrokersResult();
 
-    E brokerEnvironment = brokerEnvironmentFactory.createForMetadataBroker(pluginFQNs, identity);
+    E brokerEnvironment =
+        brokerEnvironmentFactory.createForMetadataBroker(pluginFQNs, identity, mergePlugins);
     if (isEphemeral) {
       EphemeralWorkspaceUtility.makeEphemeral(brokerEnvironment.getAttributes());
     }

infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/SidecarToolingProvisioner.java

@@ -11,12 +11,15 @@
  */
 package org.eclipse.che.workspace.infrastructure.kubernetes.wsplugins;
 
+import static org.eclipse.che.api.workspace.shared.Constants.MERGE_PLUGINS_ATTRIBUTE;
+
 import com.google.common.annotations.Beta;
 import com.google.common.collect.ImmutableMap;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import javax.inject.Inject;
+import javax.inject.Named;
 import org.eclipse.che.api.core.model.workspace.runtime.RuntimeIdentity;
 import org.eclipse.che.api.workspace.server.spi.InfrastructureException;
 import org.eclipse.che.api.workspace.server.wsplugins.ChePluginsApplier;
@@ -44,17 +47,20 @@ public class SidecarToolingProvisioner<E extends KubernetesEnvironment> {
   private final KubernetesArtifactsBrokerApplier<E> artifactsBrokerApplier;
   private final PluginFQNParser pluginFQNParser;
   private final PluginBrokerManager<E> pluginBrokerManager;
+  private final boolean defaultMergePlugins;
 
   @Inject
   public SidecarToolingProvisioner(
       Map<String, ChePluginsApplier> workspaceNextAppliers,
       KubernetesArtifactsBrokerApplier<E> artifactsBrokerApplier,
       PluginFQNParser pluginFQNParser,
-      PluginBrokerManager<E> pluginBrokerManager) {
+      PluginBrokerManager<E> pluginBrokerManager,
+      @Named("che.workspace.plugin_broker.default_merge_plugins") String defaultMergePlugins) {
     this.workspaceNextAppliers = ImmutableMap.copyOf(workspaceNextAppliers);
     this.artifactsBrokerApplier = artifactsBrokerApplier;
     this.pluginFQNParser = pluginFQNParser;
     this.pluginBrokerManager = pluginBrokerManager;
+    this.defaultMergePlugins = Boolean.parseBoolean(defaultMergePlugins);
   }
 
   @Traced
@@ -79,12 +85,21 @@ public void provision(
     }
 
     boolean isEphemeral = EphemeralWorkspaceUtility.isEphemeral(environment.getAttributes());
+    boolean mergePlugins = shouldMergePlugins(environment.getAttributes());
     List<ChePlugin> chePlugins =
         pluginBrokerManager.getTooling(
-            identity, startSynchronizer, pluginFQNs, isEphemeral, startOptions);
+            identity, startSynchronizer, pluginFQNs, isEphemeral, mergePlugins, startOptions);
 
     pluginsApplier.apply(identity, environment, chePlugins);
-    artifactsBrokerApplier.apply(environment, identity, pluginFQNs);
+    artifactsBrokerApplier.apply(environment, identity, pluginFQNs, mergePlugins);
     LOG.debug("Finished sidecar tooling provisioning workspace '{}'", identity.getWorkspaceId());
   }
+
+  private boolean shouldMergePlugins(Map<String, String> attributes) {
+    String devfileMergePlugins = attributes.get(MERGE_PLUGINS_ATTRIBUTE);
+    if (devfileMergePlugins != null) {
+      return Boolean.parseBoolean(devfileMergePlugins);
+    }
+    return defaultMergePlugins;
+  }
 }

infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/wsplugins/brokerphases/BrokerEnvironmentFactory.java

@@ -105,9 +105,11 @@ public BrokerEnvironmentFactory(
    * @param runtimeID ID of the runtime the broker would be started
    * @return kubernetes environment (or its extension) with the Plugin broker objects
    */
-  public E createForMetadataBroker(Collection<PluginFQN> pluginFQNs, RuntimeIdentity runtimeID)
+  public E createForMetadataBroker(
+      Collection<PluginFQN> pluginFQNs, RuntimeIdentity runtimeID, boolean mergePlugins)
       throws InfrastructureException {
-    BrokersConfigs brokersConfigs = getBrokersConfigs(pluginFQNs, runtimeID, metadataBrokerImage);
+    BrokersConfigs brokersConfigs =
+        getBrokersConfigs(pluginFQNs, runtimeID, metadataBrokerImage, mergePlugins);
     return doCreate(brokersConfigs);
   }
 
@@ -116,11 +118,14 @@ public E createForMetadataBroker(Collection<PluginFQN> pluginFQNs, RuntimeIdenti
    *
    * @param pluginFQNs fully qualified names of plugins that needs to be resolved by the broker
    * @param runtimeID ID of the runtime the broker would be started
+   * @param mergePlugins whether the broker should be configured to merge plugins where possible
    * @return kubernetes environment (or its extension) with the Plugin broker objects
    */
-  public E createForArtifactsBroker(Collection<PluginFQN> pluginFQNs, RuntimeIdentity runtimeID)
+  public E createForArtifactsBroker(
+      Collection<PluginFQN> pluginFQNs, RuntimeIdentity runtimeID, boolean mergePlugins)
       throws InfrastructureException {
-    BrokersConfigs brokersConfigs = getBrokersConfigs(pluginFQNs, runtimeID, artifactsBrokerImage);
+    BrokersConfigs brokersConfigs =
+        getBrokersConfigs(pluginFQNs, runtimeID, artifactsBrokerImage, mergePlugins);
     brokersConfigs
         .machines
         .values()
@@ -130,7 +135,10 @@ public E createForArtifactsBroker(Collection<PluginFQN> pluginFQNs, RuntimeIdent
   }
 
   protected BrokersConfigs getBrokersConfigs(
-      Collection<PluginFQN> pluginFQNs, RuntimeIdentity runtimeID, String brokerImage)
+      Collection<PluginFQN> pluginFQNs,
+      RuntimeIdentity runtimeID,
+      String brokerImage,
+      boolean mergePlugins)
       throws InfrastructureException {
 
     String configMapName = generateUniqueName(CONFIG_MAP_NAME_SUFFIX);
@@ -143,7 +151,8 @@ protected BrokersConfigs getBrokersConfigs(
                 authEnableEnvVarProvider.get(runtimeID), machineTokenEnvVarProvider.get(runtimeID))
             .map(this::asEnvVar)
             .collect(Collectors.toList());
-    Container container = newContainer(runtimeID, envVars, brokerImage, configMapVolume);
+    Container container =
+        newContainer(runtimeID, envVars, brokerImage, configMapVolume, mergePlugins);
     pod.getSpec().getContainers().add(container);
     pod.getSpec().getVolumes().add(newConfigMapVolume(configMapName, configMapVolume));
 
@@ -165,9 +174,10 @@ private Container newContainer(
       RuntimeIdentity runtimeId,
       List<EnvVar> envVars,
       String image,
-      @Nullable String brokerVolumeName) {
+      @Nullable String brokerVolumeName,
+      boolean mergePlugins) {
     String containerName = generateContainerNameFromImageRef(image);
-    String[] cmdArgs = getCommandLineArgs(runtimeId).toArray(new String[0]);
+    String[] cmdArgs = getCommandLineArgs(runtimeId, mergePlugins).toArray(new String[0]);
     final ContainerBuilder cb =
         new ContainerBuilder()
             .withName(containerName)
@@ -178,29 +188,34 @@ private Container newContainer(
     if (brokerVolumeName != null) {
       cb.withVolumeMounts(
           new VolumeMount(CONF_FOLDER + "/", null, brokerVolumeName, true, null, null));
-      cb.addToArgs("-metas", CONF_FOLDER + "/" + CONFIG_FILE);
+      cb.addToArgs("--metas", CONF_FOLDER + "/" + CONFIG_FILE);
     }
     Container container = cb.build();
     Containers.addRamLimit(container, "250Mi");
     Containers.addRamRequest(container, "250Mi");
     return container;
   }
 
-  protected List<String> getCommandLineArgs(RuntimeIdentity runtimeId) {
-    return new ArrayList<String>(
-        Arrays.asList(
-            "--push-endpoint",
-            cheWebsocketEndpoint,
-            "--runtime-id",
-            String.format(
-                "%s:%s:%s",
-                runtimeId.getWorkspaceId(),
-                MoreObjects.firstNonNull(runtimeId.getEnvName(), ""),
-                runtimeId.getOwnerId()),
-            "--cacert",
-            certProvisioner.isConfigured() ? certProvisioner.getCertPath() : "",
-            "--registry-address",
-            Strings.nullToEmpty(pluginRegistryUrl)));
+  protected List<String> getCommandLineArgs(RuntimeIdentity runtimeId, boolean mergePlugins) {
+    ArrayList<String> args =
+        new ArrayList<>(
+            Arrays.asList(
+                "--push-endpoint",
+                cheWebsocketEndpoint,
+                "--runtime-id",
+                String.format(
+                    "%s:%s:%s",
+                    runtimeId.getWorkspaceId(),
+                    MoreObjects.firstNonNull(runtimeId.getEnvName(), ""),
+                    runtimeId.getOwnerId()),
+                "--cacert",
+                certProvisioner.isConfigured() ? certProvisioner.getCertPath() : "",
+                "--registry-address",
+                Strings.nullToEmpty(pluginRegistryUrl)));
+    if (mergePlugins) {
+      args.add("--merge-plugins");
+    }
+    return args;
   }
 
   private Pod newPod() {

infrastructures/kubernetes/src/test/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesArtifactsBrokerApplierTest.java

@@ -13,6 +13,7 @@
 
 import static org.eclipse.che.workspace.infrastructure.kubernetes.Names.createMachineNameAnnotations;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.Mockito.doReturn;
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertFalse;
@@ -123,22 +124,22 @@ public void setUp() throws Exception {
             .build();
     doReturn(brokerEnvironment)
         .when(brokerEnvironmentFactory)
-        .createForArtifactsBroker(any(), any());
+        .createForArtifactsBroker(any(), any(), anyBoolean());
 
     applier = new KubernetesArtifactsBrokerApplier<>(brokerEnvironmentFactory);
   }
 
   @Test
   public void shouldAddBrokerMachineToWorkspaceEnvironment() throws Exception {
-    applier.apply(workspaceEnvironment, runtimeID, pluginFQNs);
+    applier.apply(workspaceEnvironment, runtimeID, pluginFQNs, false);
 
     assertNotNull(workspaceEnvironment.getMachines());
     assertTrue(workspaceEnvironment.getMachines().values().contains(brokerMachine));
   }
 
   @Test
   public void shouldAddBrokerConfigMapsToWorkspaceEnvironment() throws Exception {
-    applier.apply(workspaceEnvironment, runtimeID, pluginFQNs);
+    applier.apply(workspaceEnvironment, runtimeID, pluginFQNs, false);
 
     ConfigMap workspaceConfigMap = workspaceEnvironment.getConfigMaps().get(BROKER_CONFIGMAP_NAME);
     assertNotNull(workspaceConfigMap);
@@ -153,7 +154,7 @@ public void shouldAddBrokerConfigMapsToWorkspaceEnvironment() throws Exception {
 
   @Test
   public void shouldAddBrokerAsInitContainerOnWorkspacePod() throws Exception {
-    applier.apply(workspaceEnvironment, runtimeID, pluginFQNs);
+    applier.apply(workspaceEnvironment, runtimeID, pluginFQNs, false);
 
     List<Container> initContainers = workspacePod.getSpec().getInitContainers();
     assertEquals(initContainers.size(), 1);
@@ -162,7 +163,7 @@ public void shouldAddBrokerAsInitContainerOnWorkspacePod() throws Exception {
 
   @Test
   public void shouldAddBrokerVolumesToWorkspacePod() throws Exception {
-    applier.apply(workspaceEnvironment, runtimeID, pluginFQNs);
+    applier.apply(workspaceEnvironment, runtimeID, pluginFQNs, false);
 
     List<Volume> workspaceVolumes = workspacePod.getSpec().getVolumes();
     assertEquals(workspaceVolumes.size(), 1);