[JENKINS-53500] Windows support for container step #626

jglick · 2019-10-22T20:57:48Z

JENKINS-53500 and more generally JENKINS-57256.

Subsumes #629.

…es not work. Also needed to skip pods using nodeAffinity.

…cified.

….exe.

…nd certainly not “deprecated”.

…ields.

…bly thought and why you should probably ignore it.

…Windows.

…cDecorator.

jglick · 2019-10-24T20:46:24Z

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java

@@ -431,21 +427,22 @@ public void kill(Map<String, String> modelEnvVars) throws IOException, Interrupt

                int exitCode = doLaunch(
                        true, null, null, null, null,
+                        // TODO Windows


Have not managed to come up with a PowerShell equivalent. Get-Process and Stop-Process, fine, but how do I select just those processes matching the JENKINS_SERVER_COOKIE? You would think Process.StartInfo would offer this via its Environment or EnvironmentVariables (what is the difference?), but this property is always null in my tests, perhaps because

For example, you should not access the StartInfo property on a Process object returned by GetProcesses.

Hmm, that seems to be true from an interactive shell, but then from inside the build

Get-Process | Format-Table -DisplayError Id, ProcessName, @{Label="stuff"; Expression={$_.StartInfo.EnvironmentVariables["JENKINS_SERVER_COOKIE"]}}

works.

Or rather, it prints durable-XXX…but for every process, even for example 0 Idle and 4 System, so either the StartInfo is somehow poisoned by later state, or the set from cmd somehow mutated global state.

There is also the fact that PowershellScript overrides doLaunch when it should be overriding launchWithCookie, so even if we could identify only processes actually started with this JENKINS_SERVER_COOKIE, we would wind up killing the controller process and getting a -1 return code and losing any final output.

So for now I think this is just unfixable.

… System.err.

…ritFrom pending a working variant.

jglick · 2019-10-25T13:59:14Z

Build failure seems to be due to yet another general ci.jenkins.io outage.

Vlatombe · 2019-10-29T08:52:10Z

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java

@@ -376,6 +371,7 @@ public void onClose(int i, String s) {

                try {
                    OutputStream stdin = watch.getInput();
+                    // stdin = new TeeOutputStream(stdin, new LogTaskListener(LOGGER, Level.FINEST).getLogger());


Vlatombe · 2019-10-29T08:56:56Z

src/test/resources/org/csanchez/jenkins/plugins/kubernetes/pipeline/cascadingDelete.groovy

@@ -70,6 +70,8 @@ spec:
                        args:
                          - -c
                          - 'sleep infinity'
+                      nodeSelector:


Shouldn't be needed since you added https://github.com/jenkinsci/kubernetes-plugin/pull/626/files#diff-ec0e1841b0d5d888d11370d1a10eeabaR214-R220 ?

Vlatombe · 2019-10-29T08:57:54Z

test-in-k8s.yaml

+  - name: m2repo
+    persistentVolumeClaim:
+      claimName: m2repo
+  nodeSelector:


Same as above?

jglick added 10 commits October 22, 2019 16:55

test-in-k8s better suited to iterative use.

81bd573

Added LinuxFilter.

af5faad

[JENKINS-57256] Sketch of test of Windows pods.

a6cab42

Comment.

2a0a670

Adding some logging to LinuxFilter.

b27b537

Pending jenkinsci#623, using PodTemplateFilter to set nodeSelector do…

6619b89

…es not work. Also needed to skip pods using nodeAffinity.

Stray newline.

14f3792

Working around aws/containers-roadmap#542.

3e09dea

Oops, cannot delete jenkins pod or test collection in CI breaks.

f3d304e

Must use JENKINS_URL not JENKINS_DIRECT_CONNECTION.

47a77a2

This comment has been minimized.

Sign in to view

jglick mentioned this pull request Oct 23, 2019

Cleaned up the argumentlist and checked for exit code jenkinsci/docker-inbound-agent#120

Closed

This comment has been minimized.

Sign in to view

Switching to official image.

12b7069

This comment has been minimized.

Sign in to view

Following up 47a77a2 with a warning in documentation.

15568d7

This comment has been minimized.

Sign in to view

jglick added 9 commits October 23, 2019 11:51

Appending .svc.cluster.local to work around aws/containers-roadmap#236.

24c97c2

Reproduced JENKINS-53500 error.

230a289

Let us test both bat and powershell steps.

217af86

Just shell: cmd seems to suffice for basic functionality.

2c4916e

Autodetect OS so ContainerStep.shell does not normally need to be spe…

06479de

…cified.

Cheaper and easier to use Launcher.isUnix.

54771c4

Reverting some unnecessary changes.

179b9aa

Demonstrating that cd commands work.

f32db53

Fixing environment variable bindings.

b774e48

jglick added 12 commits October 24, 2019 14:33

So far as I can tell, \n works fine to delineate commands even to cmd…

0fd290d

….exe.

podTemplate and container are frequently used steps, not “advanced” a…

ebcfb88

…nd certainly not “deprecated”.

containerLog, on the other hand, is not normally useful.

c81198a

Config form for ContainerStep did not match its actual configurable f…

a61ba8c

…ields.

Documenting ContainerStep.shell and why it does not do what you proba…

d8992c5

…bly thought and why you should probably ignore it.

ContainerStep.shell blank means unset.

0e2e0be

Merge branch 'snippetizer' into windows-JENKINS-53500

293f1c3

More advice on user shell selection.

45bfb1c

Merge branch 'snippetizer' into windows-JENKINS-53500

dadd5f2

Documenting that ContainerStep.shell gets defaulted automatically on …

fd851c8

…Windows.

Established baseline Linux behavior for Launcher.kill in ContainerExe…

184667b

…cDecorator.

Launcher.kill in ContainerExecDecorator does not yet work in Windows.

5e9c181

jglick commented Oct 24, 2019

View reviewed changes

jglick added 4 commits October 24, 2019 17:39

Fixing SecretsMasker on Windows.

06ea50a

Status information from SecretsMasker exec should go to a logger, not…

3220391

… System.err.

SpotBugs

24215e2

Fixed assertion. (How did this pass before?)

aa7002d

jglick marked this pull request as ready for review October 24, 2019 23:16

jglick added 5 commits October 25, 2019 08:30

SpotBugs

8bcf318

Adding example of Windows build.

d3eaab0

Noting Windows support, and deleting apparently bogus example of inhe…

80ce964

…ritFrom pending a working variant.

Deleting misleading suggestion to use ContainerStep.shell.

b94478d

Merge branch 'snippetizer' into windows-JENKINS-53500

ac0123e

Helpful for test-in-k8s.sh to note any skip messages.

bcd7391

jglick requested a review from Vlatombe October 28, 2019 19:21

Vlatombe approved these changes Oct 30, 2019

View reviewed changes

Vlatombe merged commit 828f166 into jenkinsci:master Oct 30, 2019

Vlatombe added the feature New features label Oct 30, 2019

jglick deleted the windows-JENKINS-53500 branch October 30, 2019 13:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JENKINS-53500] Windows support for container step #626

[JENKINS-53500] Windows support for container step #626

jglick commented Oct 22, 2019 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

jglick Oct 24, 2019

jglick Oct 24, 2019 •

edited

Loading

jglick Oct 24, 2019

jglick commented Oct 25, 2019

Vlatombe Oct 29, 2019

Vlatombe Oct 29, 2019

Vlatombe Oct 29, 2019

[JENKINS-53500] Windows support for container step #626

[JENKINS-53500] Windows support for container step #626

Conversation

jglick commented Oct 22, 2019 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

jglick Oct 24, 2019

Choose a reason for hiding this comment

jglick Oct 24, 2019 • edited Loading

Choose a reason for hiding this comment

jglick Oct 24, 2019

Choose a reason for hiding this comment

jglick commented Oct 25, 2019

Vlatombe Oct 29, 2019

Choose a reason for hiding this comment

Vlatombe Oct 29, 2019

Choose a reason for hiding this comment

Vlatombe Oct 29, 2019

Choose a reason for hiding this comment

jglick commented Oct 22, 2019 •

edited

Loading

jglick Oct 24, 2019 •

edited

Loading