Allows to pass JAVA_OPTS to the JVM on windows #227

Vlatombe · 2021-05-24T13:20:58Z

Just like the linux version

Make sure you are opening from a topic/feature/bugfix branch (right side) and not your master branch!
Ensure that the pull request title represents the desired changelog entry
Please describe what you did
Link to relevant issues in GitHub or Jira
Link to relevant pull requests, esp. upstream and downstream changes
Ensure you have provided tests - that demonstrates feature works or fixes the issue

Just like the linux version

slide · 2021-05-24T15:58:29Z

jenkins-agent.ps1

+    $AgentArguments = @()
+
+    if(![System.String]::IsNullOrWhiteSpace($JavaOpts)) {
+        $AgentArguments += @($JavaOpts.split(" "))


What happens if the JavaOpts contains a quoted string with spaces?

I guess it doesn't work. I found some scripts like https://github.com/beatcracker/Powershell-Misc/blob/master/Split-CommandLine.ps1 that would be able to do the splitting correctly but I don't have the correct environment to test it atm.

I can test out when I am back from my vacation.

Is it actually necessary to split it ? maybe it can just be added as is..

Per https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.management/start-process?view=powershell-7.2#parameters:

Arguments can be accepted as a single string with the arguments separated by spaces, or as an array of strings separated by commas. The cmdlet joins the array into a single string with each element of the array separated by a single space. The outer quotes of the PowerShell strings are not included when the ArgumentList values are passed to the new process. If parameters or parameter values contain a space or quotes, they need to be surrounded with escaped double quotes. For more information, see [about_Quoting_Rules](https://docs.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_quoting_rules?view=powershell-7.2).

So we should be able to do something like this no ?

if (![System.String]::IsNullOrWhiteSpace($javaOpts)) { $AgentArguments += @(`"$javaOpts`") }

I haven't tested that but noticed this as part of #261

I think the whole $AgentArguments should be a string in that case.

possible to write a test for this?

https://github.com/jenkinsci/docker-inbound-agent/blob/master/tests/inboundAgent.Tests.ps1

Ah, nice, this wasn't there when I initially wrote this PR. Will have a look.

For info: jenkins-infra/helpdesk#3274

jenkins-agent.ps1

dduportal · 2022-11-23T16:30:03Z

Ping @Vlatombe , what is the status of this PR? Do you need help to land it?

Vlatombe · 2022-11-23T16:48:07Z

I have no plans to work on it. Needs some tests based on latest comments.

slide · 2022-11-23T16:55:09Z

I will take this over, I think the feature would be useful. I'll try and get to it soon.

dduportal · 2022-11-23T17:08:42Z

Thanks a lot @Vlatombe @slide for the update ❤️

Co-authored-by: Tim Jacomb <21194782+timja@users.noreply.github.com>

… only (jenkinsci#261)

@lemeurherve

https://github.com/jenkinsci/docker-agent/releases/tag/4.13-2 lists: * fix: enable long paths for git in Windows images (jenkinsci#239) @lemeurherve * Use git lfs 3.1.4, not 3.1.2 (#246) @MarkEWaite * Use Java 17.0.2_8, not 17_35 on Nanoserver (jenkinsci#245) @MarkEWaite * Use git 2.35.3 for Windows (jenkinsci#244) @MarkEWaite * Use remoting 4.13, not 4.12 (jenkinsci#243) @MarkEWaite * Use Alpine 3.15.4, not 3.15.0 (#242) @MarkEWaite * Bump debian from bullseye-20220228 to bullseye-20220328 in /11/bullseye (#237, jenkinsci#240) @dependabot * Bump debian from bullseye-20220228 to bullseye-20220328 in /8/bullseye (jenkinsci#236, jenkinsci#241) @dependabot

Co-authored-by: Tim Jacomb <21194782+timja@users.noreply.github.com>

Pipeline will compute it based on tag string

Co-authored-by: Damien Duportal <damien.duportal@gmail.com>

Closes #272 Bumps version of upstream image too

Co-authored-by: Tim Jacomb <21194782+timja@users.noreply.github.com>

…/d... ... ocker-inbound-agent/11/debian/Dockerfile" Made with ❤️️ by updatecli

slide · 2022-11-24T17:29:52Z

Hmmm, that push did more than I thought...

slide · 2022-11-24T17:31:17Z

I'm going to submit a new PR

slide · 2022-11-24T17:38:35Z

Actually, I think this is fine.

dduportal

LGTM 👏

…atombe/windows_javaopts Allows to pass JAVA_OPTS to the JVM on windows

Allows to pass JAVA_OPTS to the JVM

bbefe48

Just like the linux version

Vlatombe requested a review from a team as a code owner May 24, 2021 13:20

Vlatombe changed the title ~~Allows to pass JAVA_OPTS to the JVM~~ Allows to pass JAVA_OPTS to the JVM on windows May 24, 2021

slide approved these changes May 24, 2021

View reviewed changes

slide suggested changes May 24, 2021

View reviewed changes

Merge branch 'master' into windows_javaopts

c2a2a0f

jtnord reviewed Mar 9, 2022

View reviewed changes

jenkins-agent.ps1 Outdated Show resolved Hide resolved

MarkEWaite and others added 19 commits November 23, 2022 12:44

Remoting 4.11.2

fbc67e1

Build agent newer agent image (jenkinsci#253)

b50c46d

Add Java 17 alpine (jenkinsci#252)

93e8f99

Co-authored-by: Tim Jacomb <21194782+timja@users.noreply.github.com>

Allow the use of multiple arguments (jenkinsci#254)

e6fa6a2

Revert "Allow the use of multiple arguments" (jenkinsci#257)

b473762

Specify JENKINS_JAVA_OPTS to pass JVM options that apply to the agent…

ec42eb0

… only (jenkinsci#261)

Move away from deprecated Release Drafter App (jenkinsci#262)

123d54d

Remove --add-opens directives on Java 17 (jenkinsci#263)

5c27e04

Bump remoting version (jenkinsci#264)

0737c8b

Clarify build number comment

fde53e5

Co-authored-by: Tim Jacomb <21194782+timja@users.noreply.github.com>

Don't need to change BUILD_NUMBER

11f80cd

Pipeline will compute it based on tag string

Now builds also for linux/arm/v7. (jenkinsci#269)

eca3988

Co-authored-by: Damien Duportal <damien.duportal@gmail.com>

Use build number 2 of agent image

ff9a12e

Remove "-preview" from Java 17 images

39b51ef

Closes #272 Bumps version of upstream image too

Remove preview from build-arg

08e2b9d

Stop publishing jdk 8 remoting on master branch

0aeed4c

Remove Java 8 (jenkinsci#278)

9b4d12a

Co-authored-by: Tim Jacomb <21194782+timja@users.noreply.github.com>

Update base image to latest remoting (jenkinsci#281)

11b10b5

github-actions bot and others added 10 commits November 23, 2022 12:44

chore: changed lines [23 26] of file "/tmp/updatecli/github/jenkinsci…

041099e

…/d... ... ocker-inbound-agent/11/debian/Dockerfile" Made with ❤️️ by updatecli

chore: changed lines [23 26] of file "/tmp/updatecli/github/jenkinsci…

579ba2c

…/d... ... ocker-inbound-agent/11/debian/Dockerfile" Made with ❤️️ by updatecli

chore: changed lines [23 26] of file "/tmp/updatecli/github/jenkinsci…

4faaaa2

…/d... ... ocker-inbound-agent/11/debian/Dockerfile" Made with ❤️️ by updatecli

chore: changed lines [23 26] of file "/tmp/updatecli/github/jenkinsci…

d817009

…/d... ... ocker-inbound-agent/11/debian/Dockerfile" Made with ❤️️ by updatecli

chore: changed lines [23 26] of file "/tmp/updatecli/github/jenkinsci…

bbb0c02

…/d... ... ocker-inbound-agent/11/debian/Dockerfile" Made with ❤️️ by updatecli

chore: changed lines [23 26] of file "/tmp/updatecli/github/jenkinsci…

b29e9d0

…/d... ... ocker-inbound-agent/11/debian/Dockerfile" Made with ❤️️ by updatecli

chore: changed lines [23 26] of file "/tmp/updatecli/github/jenkinsci…

005a073

…/d... ... ocker-inbound-agent/11/debian/Dockerfile" Made with ❤️️ by updatecli

chore: changed lines [23 26] of file "/tmp/updatecli/github/jenkinsci…

2ff5bc4

…/d... ... ocker-inbound-agent/11/debian/Dockerfile" Made with ❤️️ by updatecli

chore: changed lines [23 26] of file "/tmp/updatecli/github/jenkinsci…

49cc081

…/d... ... ocker-inbound-agent/11/debian/Dockerfile" Made with ❤️️ by updatecli

Fixup command line processing and add test

dd9d59d

Merge branch 'master' into windows_javaopts

2c26e1e

dduportal added the enhancement label Nov 24, 2022

dduportal approved these changes Nov 24, 2022

View reviewed changes

slide enabled auto-merge November 29, 2022 02:42

dduportal requested a review from slide November 29, 2022 08:24

slide merged commit b537b5a into jenkinsci:master Nov 29, 2022

This was referenced Dec 1, 2022

[ci.jenkins.io] ACI agent are all failing jenkins-infra/helpdesk#3274

Closed

Windows: missing documentation about passing JENKINS_JAVA_OPTS with mandatory double quotes jenkinsci/docker-agent#599

Closed

lemeurherve pushed a commit to lemeurherve/docker-agent that referenced this pull request Nov 28, 2023

[PATCH] Merge pull request jenkinsci/docker-inbound-agent#227 from Vl…

4b8064f

…atombe/windows_javaopts Allows to pass JAVA_OPTS to the JVM on windows

lemeurherve pushed a commit to lemeurherve/docker-agent that referenced this pull request Nov 28, 2023

[PATCH] Merge pull request jenkinsci/docker-inbound-agent#227 from Vl…

f8366d9

…atombe/windows_javaopts Allows to pass JAVA_OPTS to the JVM on windows

lemeurherve pushed a commit to lemeurherve/docker-agent that referenced this pull request Nov 28, 2023

[PATCH] Merge pull request jenkinsci/docker-inbound-agent#227 from Vl…

922ebdc

…atombe/windows_javaopts Allows to pass JAVA_OPTS to the JVM on windows

lemeurherve pushed a commit to lemeurherve/docker-agent that referenced this pull request Nov 28, 2023

[PATCH] Merge pull request jenkinsci/docker-inbound-agent#227 from Vl…

0d59128

…atombe/windows_javaopts Allows to pass JAVA_OPTS to the JVM on windows

lemeurherve pushed a commit to lemeurherve/docker-agent that referenced this pull request Nov 28, 2023

[PATCH] Merge pull request jenkinsci/docker-inbound-agent#227 from Vl…

7ba4ae4

…atombe/windows_javaopts Allows to pass JAVA_OPTS to the JVM on windows

lemeurherve pushed a commit to lemeurherve/docker-agent that referenced this pull request Nov 28, 2023

[PATCH] Merge pull request jenkinsci/docker-inbound-agent#227 from Vl…

7b02dfd

…atombe/windows_javaopts Allows to pass JAVA_OPTS to the JVM on windows

lemeurherve pushed a commit to lemeurherve/docker-agent that referenced this pull request Nov 28, 2023

[PATCH] Merge pull request jenkinsci/docker-inbound-agent#227 from Vl…

ecbf0fc

…atombe/windows_javaopts Allows to pass JAVA_OPTS to the JVM on windows

lemeurherve pushed a commit to lemeurherve/docker-agent that referenced this pull request Nov 28, 2023

[PATCH] Merge pull request jenkinsci/docker-inbound-agent#227 from Vl…

c4318f3

…atombe/windows_javaopts Allows to pass JAVA_OPTS to the JVM on windows

lemeurherve pushed a commit to lemeurherve/docker-agent that referenced this pull request Nov 28, 2023

[PATCH] Merge pull request jenkinsci/docker-inbound-agent#227 from Vl…

9659ec9

…atombe/windows_javaopts Allows to pass JAVA_OPTS to the JVM on windows

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allows to pass JAVA_OPTS to the JVM on windows #227

Allows to pass JAVA_OPTS to the JVM on windows #227

Vlatombe commented May 24, 2021

slide May 24, 2021

Vlatombe Jul 12, 2021

slide Jul 13, 2021

Dohbedoh Mar 9, 2022 •

edited

Loading

Vlatombe Mar 9, 2022

timja Mar 9, 2022

Vlatombe Mar 9, 2022

dduportal Dec 1, 2022

dduportal commented Nov 23, 2022

Vlatombe commented Nov 23, 2022 •

edited

Loading

slide commented Nov 23, 2022

dduportal commented Nov 23, 2022

slide commented Nov 24, 2022

slide commented Nov 24, 2022

slide commented Nov 24, 2022

dduportal left a comment

Allows to pass JAVA_OPTS to the JVM on windows #227

Allows to pass JAVA_OPTS to the JVM on windows #227

Conversation

Vlatombe commented May 24, 2021

slide May 24, 2021

Choose a reason for hiding this comment

Vlatombe Jul 12, 2021

Choose a reason for hiding this comment

slide Jul 13, 2021

Choose a reason for hiding this comment

Dohbedoh Mar 9, 2022 • edited Loading

Choose a reason for hiding this comment

Vlatombe Mar 9, 2022

Choose a reason for hiding this comment

timja Mar 9, 2022

Choose a reason for hiding this comment

Vlatombe Mar 9, 2022

Choose a reason for hiding this comment

dduportal Dec 1, 2022

Choose a reason for hiding this comment

dduportal commented Nov 23, 2022

Vlatombe commented Nov 23, 2022 • edited Loading

slide commented Nov 23, 2022

dduportal commented Nov 23, 2022

slide commented Nov 24, 2022

slide commented Nov 24, 2022

slide commented Nov 24, 2022

dduportal left a comment

Choose a reason for hiding this comment

Dohbedoh Mar 9, 2022 •

edited

Loading

Vlatombe commented Nov 23, 2022 •

edited

Loading