Merge pull request #24159 from vespa-engine/arnej/container-env-via-sentinel-config

pass container environment variables via new mechanism also

3 files changed, 16 insertions, 18 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java
index 7e375951c7f..d58d1f4ae5d 100755
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerCluster.java
@@ -160,7 +160,6 @@ public abstract class ContainerCluster<CONTAINER extends Container>
 
     private String hostClusterId = null;
     private String jvmGCOptions = null;
-    private String environmentVars = null;
 
     private boolean deferChangesUntilRestart = false;
 
@@ -521,9 +520,6 @@ public abstract class ContainerCluster<CONTAINER extends Container>
                 .heapsize(256)
                 .heapSizeAsPercentageOfPhysicalMemory(0)
                 .gcopts(Objects.requireNonNullElse(jvmGCOptions, G1GC));
-        if (environmentVars != null) {
-            builder.qrs.env(environmentVars);
-        }
     }
 
     @Override
@@ -643,8 +639,6 @@ public abstract class ContainerCluster<CONTAINER extends Container>
 
     public void setJvmGCOptions(String opts) { this.jvmGCOptions = opts; }
 
-    public void setEnvironmentVars(String environmentVars) { this.environmentVars = environmentVars; }
-
     public Optional<String> getJvmGCOptions() { return Optional.ofNullable(jvmGCOptions); }
 
     public final void setRpcServerEnabled(boolean rpcServerEnabled) { this.rpcServerEnabled = rpcServerEnabled; }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
index 16eac3bb34e..5e09dd4732d 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilder.java
@@ -93,6 +93,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
@@ -707,25 +708,27 @@ public class ContainerModelBuilder extends ConfigModelBuilder<ContainerModel> {
 
             extractJvmOptions(nodes, cluster, nodesElement, context);
             applyDefaultPreload(nodes, nodesElement);
-            String environmentVars = getEnvironmentVariables(XML.getChild(nodesElement, ENVIRONMENT_VARIABLES_ELEMENT));
-            if (!environmentVars.isEmpty()) {
-                cluster.setEnvironmentVars(environmentVars);
+            var envVars = getEnvironmentVariables(XML.getChild(nodesElement, ENVIRONMENT_VARIABLES_ELEMENT)).entrySet();
+            for (var container : nodes) {
+                for (var entry : envVars) {
+                    container.addEnvironmentVariable(entry.getKey(), entry.getValue());
+                }
             }
             if (useCpuSocketAffinity(nodesElement))
                 AbstractService.distributeCpuSocketAffinity(nodes);
-
             cluster.addContainers(nodes);
         }
     }
 
-    private static String getEnvironmentVariables(Element environmentVariables) {
-        StringBuilder sb = new StringBuilder();
+    private static Map<String, String> getEnvironmentVariables(Element environmentVariables) {
+        var map = new LinkedHashMap<String, String>();
         if (environmentVariables != null) {
             for (Element var: XML.getChildren(environmentVariables)) {
-                sb.append(var.getNodeName()).append('=').append(var.getTextContent()).append(' ');
+                var name = new com.yahoo.text.Identifier(var.getNodeName());
+                map.put(name.toString(), var.getTextContent());
             }
         }
-        return sb.toString();
+        return map;
     }
     
     private List<ApplicationContainer> createNodes(ApplicationContainerCluster cluster, Element containerElement, Element nodesElement, ConfigModelContext context) {
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java
index 4d9bd978883..272dfa19f64 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerModelBuilderTest.java
@@ -513,16 +513,17 @@ public class ContainerModelBuilderTest extends ContainerModelBuilderTestBase {
                 "  <nodes>",
                 "    <environment-variables>",
                 "      <KMP_SETTING>1</KMP_SETTING>",
+                "      <valid_name>some value</valid_name>",
                 "      <KMP_AFFINITY>granularity=fine,verbose,compact,1,0</KMP_AFFINITY>",
                 "    </environment-variables>",
                 "    <node hostalias='mockhost'/>",
                 "  </nodes>",
                 "</container>");
         createModel(root, clusterElem);
-        QrStartConfig.Builder qrStartBuilder = new QrStartConfig.Builder();
-        root.getConfig(qrStartBuilder, "container/container.0");
-        QrStartConfig qrStartConfig = new QrStartConfig(qrStartBuilder);
-        assertEquals("KMP_SETTING=1 KMP_AFFINITY=granularity=fine,verbose,compact,1,0 ", qrStartConfig.qrs().env());
+        var container = (AbstractService) root.getProducer("container/container.0");
+        var env = container.getEnvVars();
+        assertEquals("1", env.get("KMP_SETTING"));
+        assertEquals("granularity=fine,verbose,compact,1,0", env.get("KMP_AFFINITY"));
     }
 
     private void verifyAvailableprocessors(boolean isHosted, Flavor flavor, int expectProcessors) {