Skip to content
This repository was archived by the owner on Mar 4, 2026. It is now read-only.

Use standard POD_LABEL rather than rolling your own - #3

Open
jglick wants to merge 2 commits into
EliLillyCo:masterfrom
jglick:POD_LABEL
Open

Use standard POD_LABEL rather than rolling your own#3
jglick wants to merge 2 commits into
EliLillyCo:masterfrom
jglick:POD_LABEL

Conversation

@jglick

@jglick jglick commented Jun 30, 2021

Copy link
Copy Markdown

Summary

As of jenkinsci/kubernetes-plugin#539 you should not need to explicitly come up with a pod label.

Submitter's Pledge

Reviewers, I have verified the following to the best of my knowledge:

  • I have added unit test cases for the changes where applicable.
  • I have updated CHANGELOG.md with the new target version (or with the Unreleased tag) and updated the version comparison url in the file.
  • I have updated documentation in README.md at the repo root and all of the applicable README.md files under docs/** along with necessary version change for code samples in those docs where applicable.
  • I have read and fully understand the process for submitting a pull request to the codebase.

Review Recommendation

Nothing specific; do changes of this kind get test coverage?

@jglick
jglick requested a review from a team as a code owner June 30, 2021 19:13
@NickLiffen

Copy link
Copy Markdown

@jglick what does the built in POD LABEL create? If it some form of uuid

@jglick

jglick commented Jun 30, 2021

Copy link
Copy Markdown
Author

Basically the idea is that if you are using the podTemplate step, you should never need to worry about Jenkins node labels—this is managed.

@NickLiffen

Copy link
Copy Markdown

@jglick I think some tests need updating 👍

def "createDockerImage should run the openshift startBuild command to build a docker image with custom build args"() {

def "runInPod runs the given closure in openshift pod"() {

def "creating a docker secret configures the pipeline with the secret"() {

I think the startsWith() needs updating 👍

…t` (acc. to openshift.com `OpenShift` would be better but that may be incompatible)
@jglick

jglick commented Jul 1, 2021

Copy link
Copy Markdown
Author

I tried

diff --git test/src/com/lilly/cirrus/jenkinsdsl/openshift/OpenshiftClientSpec.groovy test/src/com/lilly/cirrus/jenkinsdsl/openshift/OpenshiftClientSpec.groovy
index 7a79522..a7d1f67 100644
--- test/src/com/lilly/cirrus/jenkinsdsl/openshift/OpenshiftClientSpec.groovy
+++ test/src/com/lilly/cirrus/jenkinsdsl/openshift/OpenshiftClientSpec.groovy
@@ -70,7 +70,7 @@ class OpenshiftClientSpec extends CirrusDSLSpec {
     String podNamespace = "cje-slaves-freestyle-dmz"
     String containerName = "my-container"
     JenkinsSim pipeline = createJenkinsSim()
-    pipeline.withEnv([JOB_NAME: this.getTestString(45)])
+    pipeline.withEnv([JOB_NAME: this.getTestString(45), POD_LABEL: 'test'])
     pipeline.withCredentialBinding("cred-id", [USER: "user", PASSWORD: "password"])
 
     OpenshiftClient openShiftClient = new OpenshiftClient(jenkins: pipeline)
@@ -81,7 +81,7 @@ class OpenshiftClientSpec extends CirrusDSLSpec {
     openShiftClient.runInPod(podNamespace, containerName, { closureExecuted = true })
 
     then: "pipeline executes needed pipeline actions"
-    pipeline.anchor(SimFactory.node({ label -> label.startsWith("oc-dsl") }))
+    pipeline.anchor(SimFactory.node({ label -> label == 'test' }))
       .next(SimFactory.container(containerName))
 
     and: "the closure gets executed"

without success; the error messages from the mock framework are cryptic to me:

java.lang.IllegalStateException: No commands in the execution trace match the supplied command [node label: com.lilly.cirrus.jenkinsdsl.openshift.OpenshiftClientSpec$__spock_feature_1_3_closure2@ac4d876]
	at com.lilly.cirrus.jenkinsdsl.sim.JenkinsSim.trace(JenkinsSim.groovy:84)
	at com.lilly.cirrus.jenkinsdsl.sim.JenkinsSim.anchor(JenkinsSim.groovy:131)
	at com.lilly.cirrus.jenkinsdsl.openshift.OpenshiftClientSpec.runInPod runs the given closure in openshift pod(OpenshiftClientSpec.groovy:84)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants