-
Notifications
You must be signed in to change notification settings - Fork 697
[JENKINS-70093] Add Windows SSH agent implementation #1056
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.
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
base: master
Are you sure you want to change the base?
Conversation
Should be linked to https://issues.jenkins.io/browse/JENKINS-70093. |
|
||
@Override | ||
public boolean isWindows() { | ||
return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find this misleading.. It is a windows AMI. But describe Windows SSH attributes. Shouldn't this be true
. And condition that were using it should be updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see now that for each implementation we add a boolean isNewImplementation
method.. so maybe this is just following the design I guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added isSSHAgent/isWinRMAgent for checking the agent connection type and refactored isWindows/isMac/isUnix to only be used for checking the agent OS
@@ -1490,19 +1493,25 @@ public String getRemoteAdmin() { | |||
public String getRootCommandPrefix() { | |||
return (amiType.isUnix() | |||
? ((UnixData) amiType).getRootCommandPrefix() | |||
: (amiType.isMac() ? ((MacData) amiType).getRootCommandPrefix() : "")); | |||
: (amiType.isMac() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those could be simplified with maybe an abstract SSHData
? WindowsSSHData
, MacData
and UnixData
all have the same attributes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed and done
Adding support for launching Windows agents using SSH https://issues.jenkins.io/browse/JENKINS-70093
Testing done
mvn clean verify
Tested on Jenkins controller 2.500 launching and connecting to Windows Server 2019 agent via SSH.
Tested with init script
Submitter checklist