Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-66193

yamlMergeStrategy merge() clears not specified lists

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      Lists that are not specified in a merged yaml get overwritten with an empty list.

      Parent:

      apiVersion: "v1"
      kind: "Pod"
      spec:
       containers:
       - args:
         - "arg1"
         - "arg2"
         command:
         - "cat"
         image: "busybox"
         name: "container"
       tty: true
      

      Child:

      apiVersion: "v1"
      kind: "Pod"
      spec:
       containers:
       - name: "container"
         resources:
           memory: "2Gi"
      

       

      In this example I expect the merged PodTemplate to contain both the command and the arguments list from the parent template. Sadly this is not the case. After the merge both lists are empty and commands and arguments are lost.

      The cause for this seems to be the merging in {{ org.csanchez.jenkins.plugins.kubernetes.PodTemplateUtils#combine(io.fabric8.kubernetes.api.model.Container, io.fabric8.kubernetes.api.model.Container)}}

      The null check in https://github.com/jenkinsci/kubernetes-plugin/blob/3152480fafce7393235070f650c05f2022d42431/src/main/java/org/csanchez/jenkins/plugins/kubernetes/PodTemplateUtils.java#L171-L172 never works for pod templates loaded from yaml as even in those cases the Pod object contains an empty List instead of null for fields that where not specified in the yaml.

       

      here's an example unittest:

      @Test
      public void yamlOverrideRetainsCommandAndArgs() {
          PodTemplate parent = new PodTemplate();
          parent.setYaml(
              "apiVersion: v1\n" +
                  "kind: Pod\n" +
                  "metadata:\n" +
                  "  labels:\n" +
                  "    some-label: some-label-value\n" +
                  "spec:\n" +
                  "  containers:\n" +
                  "  - name: container\n" +
                  "    image: busybox\n" +
                  "    command:\n" +
                  "    - cat\n" +
                  "    args:\n" +
                  "    - arg1\n" +
                  "    - arg2\n" +
                  "    tty: true\n"
          );
      
          PodTemplate child = new PodTemplate();
          child.setYaml(
              "spec:\n" +
                  "  hostNetwork: true\n" +
                  "  containers:\n" +
                  "  - name: container\n" +
                  "    resources:\n" +
                  "      memory: 2Gi\n"
          );
          child.setInheritFrom("parent");
          child.setYamlMergeStrategy(merge());
          PodTemplate result = combine(parent, child);
          Pod pod = new PodTemplateBuilder(result, slave).build();
          List<Container> containers = pod.getSpec().getContainers();
          assertEquals(2, containers.size());
          Optional<Container> candidate = containers.stream().filter(c -> "container".equals(c.getName()))
              .findAny();
          assertTrue(candidate.isPresent());
          Container container = candidate.get();
          assertThat(container.getCommand(), contains("cat"));
          assertThat(container.getArgs(), contains("arg1", "arg2"));
      }
       

       

      Workaround:

      A known workaround is to explicitly set all values that I want to retain to null in the merging yaml:

      apiVersion: "v1"
      kind: "Pod"
      spec:
       containers:
       - name: "container"
         resources:
           memory: "2Gi"
         command: ~
         args: ~

       This is quite ugly though and would be something I'd like to avoid.

       

      It is also an issue that probably affects all Collections in the model and is not limited to command and args.

        Attachments

          Activity

          There are no comments yet on this issue.

            People

            Assignee:
            Unassigned Unassigned
            Reporter:
            cfraenkel Christian Fraenkel
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Dates

              Created:
              Updated: