Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed: JENKINS-40925 - dir context is not honored by shell step #146

Merged
merged 3 commits into from
Mar 10, 2017
Merged

Fixed: JENKINS-40925 - dir context is not honored by shell step #146

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c89b277
Fixed: JENKINS-40925 - dir context is not honored by shell step
electroma Mar 2, 2017
8c652cc
Fixed: @carlossg comment regarding unused argument
electroma Mar 8, 2017
7c9d7ab
Put back old constructor for backward compatibility
electroma Mar 8, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Fixed: JENKINS-40925 - dir context is not honored by shell step
  • Loading branch information
@electroma
electroma committed Mar 2, 2017
commit c89b2779469d07bebc1a20795d070114f3cc6eac
5 changes: 1 addition & 4 deletions src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import java.util.logging.Level;
import java.util.logging.Logger;

import static io.fabric8.kubernetes.client.Watcher.Action.MODIFIED;
import static org.csanchez.jenkins.plugins.kubernetes.pipeline.Constants.*;

public class ContainerExecDecorator extends LauncherDecorator implements Serializable, Closeable {
Expand All @@ -59,7 +58,6 @@ public class ContainerExecDecorator extends LauncherDecorator implements Seriali
private final transient KubernetesClient client;
private final String podName;
private final String containerName;
private final String path;
private final AtomicBoolean alive;

private transient CountDownLatch started;
Expand All @@ -72,7 +70,6 @@ public ContainerExecDecorator(KubernetesClient client, String podName, String co
this.client = client;
this.podName = podName;
this.containerName = containerName;
this.path = path;
this.alive = alive;
this.started = started;
this.finished = finished;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if path is removed, then the constructor should be deprecated and a new one without path argument added

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@carlossg there is only one usage of this constructor, so I can just change the signature inplace.
Do I still need to deprecate and create new one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've removed unused parameter

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, better to copy and deprecate to keep backwards binary compatibility

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@carlossg green light

Expand Down Expand Up @@ -122,7 +119,7 @@ public void onClose(int i, String s) {

//We need to get into the project workspace.
//The workspace is not known in advance, so we have to execute a cd command.
watch.getInput().write(String.format("cd \"%s\"%s", path, NEWLINE).getBytes(StandardCharsets.UTF_8));
watch.getInput().write(String.format("cd \"%s\"%s", starter.pwd(), NEWLINE).getBytes(StandardCharsets.UTF_8));
doExec(watch, launcher.getListener().getLogger(), getCommands(starter));
proc = new ContainerExecProc(watch, alive, finished);
return proc;
Expand Down
40 changes: 36 additions & 4 deletions src/test/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/KubernetesPipelineTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,6 @@

/**
* @author Carlos Sanchez
* @since
*
*/
public class KubernetesPipelineTest {

Expand Down Expand Up @@ -182,6 +180,40 @@ public void runJobWithSpaces() throws Exception {
r.assertLogContains("pwd is -/home/jenkins/workspace/p with spaces-", b);
}

@Test
public void runDirContext() throws Exception {
configureCloud(r);
WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "job with dir");
p.setDefinition(new CpsFlowDefinition("" //
+ "podTemplate(cloud: 'minikube', label: 'mypod', containers: [\n" //
+ " containerTemplate(name: 'busybox', image: 'busybox', ttyEnabled: true, command: '/bin/cat'),\n" //
+ " ]) {\n" //
+ "\n" //
+ " node ('mypod') {\n" //
+ " stage('Run') {\n" //
+ " container('busybox') {\n" //
+ " sh 'mkdir hz'\n" //
+ " sh 'echo \"initpwd is -$(pwd)-\"'\n" //
+ " dir('hz') {\n" //
+ " sh 'echo \"dirpwd is -$(pwd)-\"'\n" //
+ " }\n" //
+ " sh 'echo \"postpwd is -$(pwd)-\"'\n" //
+ " }\n" //
+ " }\n" //
+ "\n" //
+ " }\n" //
+ "}\n" //
, true));
WorkflowRun b = p.scheduleBuild2(0).waitForStart();
assertNotNull(b);
r.assertBuildStatusSuccess(r.waitForCompletion(b));
String workspace = "/home/jenkins/workspace/job with dir";
r.assertLogContains("initpwd is -" + workspace + "-", b);
r.assertLogContains("dirpwd is -" + workspace + "/hz-", b);
r.assertLogContains("postpwd is -" + workspace + "-", b);

}

// @Test
public void runInPodWithRestart() throws Exception {
story.addStep(new Statement() {
Expand All @@ -191,7 +223,7 @@ public void evaluate() throws Throwable {

story.j.jenkins.addNode(new DumbSlave("slave", "dummy", tmp.newFolder("remoteFS").getPath(), "1",
Node.Mode.NORMAL, "", story.j.createComputerLauncher(null), RetentionStrategy.NOOP,
Collections.<NodeProperty<?>> emptyList())); // TODO JENKINS-26398 clumsy
Collections.<NodeProperty<?>>emptyList())); // TODO JENKINS-26398 clumsy
WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p");
p.setDefinition(new CpsFlowDefinition("" //
+ "node('slave') {\n" //
Expand Down Expand Up @@ -223,7 +255,7 @@ public void evaluate() throws Throwable {
+ " }" //
+ " }" //
+ "}" //
, true));
, true));
WorkflowRun b = p.scheduleBuild2(0).waitForStart();
SemaphoreStep.waitForStart("withDisplayAfterRestart/1", b);
}
Expand Down