[JENKINS-57548] Explicit inheritance should override implicit inheritance

[hypnoce]

No description provided.

[carlossg]

question, why would you nest two podTemplate if you don't want them to inherit each other?

[hypnoce]

In declarative pipeline

pipeline {
    agent {
        kubernetes {
            label "foo-${UUID.randomUUID().toString()}"
            yamlFile 'k8s/KubernetesLinuxPod.yaml'
        }
    }

    stages {
        ...
        # Other stages using parent container
        ...
        stage('windows-openjdk8') {
            agent {
                kubernetes {
                    label "foo-ojdk8-${UUID.randomUUID().toString()}"
                    yamlFile 'k8s/KubernetesWindowsPod.yaml'
                }
            }
            steps {
                ...
            }
        }
    }
}

In this situation,

• It is not obvious this is a nested configuration that will be merged with the parent
• I do not want it to be merged since I'm scheduling on windows containers via kubernetes

What do you think ?

[carlossg]

gotcha, makes sense. Maybe you want to also add a test for that declarative pipeline?

[hypnoce]

Test added for declarative pipeline.
Since the empty string is now used to prevent any inheritance, the getAsArgs method needed to allow empty inheritFrom.
I'm not sure about this. What do you think ?

[hypnoce]

Added README notice.

[carlossg]

yes, the empty inheritFrom is not great UX, maybe we should not inherit by default?
@Vlatombe WDYT ?

[Vlatombe]

Changing the default behaviour may be a migration problem for existing pod templates defined through pipeline. If we have a way to ensure this migration can be done smoothly, why not.

[hypnoce]

I'm not sure how to proceed with changing the default behavior. It will be a breaking change that might impact existing pipeline.
Unless the plugin version is bump to 2.. with clear documentation on the breaking changes.
Or keep the current behavior, and put changes of this default behavior in a backlog for 2.x.x with other possible breaking changes.
What do you think ?

[hypnoce]

Hi all,

Any idea on this ?

Thanks

[Vlatombe]

Considering the current declarative syntax, I think it should be implicit that the kubernetes pod template definition shouldn't be inherited in this case.
I think it is okay to change this behaviour (only for declarative). We need to add the corresponding coverage to KubernetesDeclarativeAgentTest to freeze the behaviour.

[hypnoce]

@Vlatombe the default behavior is now set for declarative agent templates.
The empty string is used to prevent implicit inheritance.
Let me know if that's ok with you.
README has been updated as well to reflect all changes from a usage perspective.

[deiwin]

question, why would you nest two podTemplate if you don't want them to inherit each other?

I have a use case that doesn't involve declarative either. I know this is OT on this PR.

I need to start a new pod within another podTemplate because I need both at the same time and they have to execute on different nodes (diferent nodeSelectors). This is especially difficult, if I need to run any commands in the pod created by the outer podTemplate, because the inner pod disappears when I exit its node block. Right now I'm using this hack to deal with this situation:

def withOtherPod(Closure body) {
  def podLabel = "other-pod-${UUID.randomUUID()}"
  podTemplate(
    name: podLabel,
    label: podLabel,
    // Enough to re-enter the `node` in the parallel step
    idleMinutes: 1,
    yaml: '..'
  ) {
    node(podLabel) {
      // Set up the new pod
    }

    def finishedToken = 'finished'
    try {
      parallel(
        'keepalive': {
          node(podLabel) {
            // Sleep essentially for forever, to keep this pod active
            sleep(time: 100, unit: 'DAYS')
          }
        },
        'work': {
          body()
          throw new Exception(finishedToken)
        },
        failFast: true
      )
    } catch(Exception e) {
      if (e.getMessage() != finishedToken) {
        throw e
      }
    }
  }
}

[Vlatombe]

LGTM. @carlossg is it okay for you as well?