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.

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

Full Windows support, decreased network bandwitch usage and labels matched by reggex #11

Merged
merged 43 commits into from Oct 23, 2018

Conversation

jaesbit
Copy link
Contributor

@jaesbit jaesbit commented Jun 5, 2018

  • Now the execution procedure differs between Unix and Win32 Operating Systems and performs each execution with the native shell.
  • Each slave has its own label installation cache.
  • Subversioning by hashing of any script changes between last cached and last configured.
  • Each predefined setup config are checked individually.
  • If the slave has installed last setup config version it will avoid following executions.
  • If slave fails in some setup, will not restart all previous executions.
  • Do not copy files if they were previously copied (using subversioning).
  • Labels are now matched as a unique label like last versions but now, included regex support.
  • A lot of code review, comments and JavaDoc updating

Mikel Royo and others added 30 commits May 24, 2018 17:39
Updated to 1.11.2 rev.
Increased rev, now 1.11.3
…network traffic while we dont know which components are pendind to Setup on connecting nodes.
…ethods.

Remote path handling from master, added to launcher.
Config file excluding components to avoid useless pre-executions on node connections.
On development.
Update pom to add me as developer
* jenkins increased version to 3.4 from 1.442
* maven increased version to 2.5.3 from 2.2.2
* Java builds with JDK 8
* Update Properties section with new rules
* Move me and Mikel to contributors section
[¡!] Check it that clone can be compiled by default.
Removed .factorypath and added to gitignore
Fixed checks for existing installations.
Added more verbose when setDebug(true)
First steps to complete tasks
Rewrite copyFiles from setupDeployer
Change version delimiter from 3 chars to 1
Updated Deprecated call on listener
Components do entire flow, one per one, but, needs to have some checks.
I checked and fixed tipical errors and mistakes.
Increase in 1 revision for alpha versioncode
Update code comment
Upgraded newDeploy to be dinamically and not store nothing
Updated verbose data
Added at end Clearing Temporally folder
Better error handling by throwing to Jenkins in most cases
Remove unused methods
Fixed executeOnMaster, removed unused arguments
Recovered feature to getAllActiveSlaves
Added executeOnSlave to easy usages
Updated Config events to support new changes
English corrections in comments
Removed unused imports
Adding more debug messages
Create singleSetup in order to handle deployOnSave feature
Fixed ignore save_now check
SaveNow redirected to doSetup
Improved Close stream, only if had
something.
Add Not CaseSensitive at label matching
@peppelan
Copy link
Member

peppelan commented Jun 5, 2018

@atgiovannini thanks for the contribution, very appreciated, for both the long-awaited new feature and the housekeeping.

I will review and let you know asap - I'll try to make it by the end of this week if this works for you

@jaesbit
Copy link
Contributor Author

jaesbit commented Jun 6, 2018

Nice, I'm glad to hear that. I'll wait for merge.

@@ -6,51 +6,46 @@
import hudson.model.TaskListener;
import hudson.remoting.Channel;
import hudson.slaves.ComputerListener;
// import jenkins.model.Jenkins;
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason for keeping this?

public void preOnline(Computer c, Channel channel, FilePath root, TaskListener listener)
throws IOException, InterruptedException {

// Componentes printer will print only debug messages.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe a typo here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the future, I wan to add in global config a checkbox for enable/disable debug info.

@@ -91,7 +83,7 @@ public String getPrepareScript() {
/**
* Sets the prepare script code
*
* @param prepareScript
* @param prepareScript User values
Copy link
Member

Choose a reason for hiding this comment

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

not sure this change improves the reader's understanding

@@ -100,7 +92,7 @@ public void setPrepareScript(String prepareScript) {
/**
* Returns the directory containing the setup relevant files and sub directories
*
* @return
* @return User values
Copy link
Member

Choose a reason for hiding this comment

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

same here

@@ -173,6 +165,8 @@ public void setPrepareScriptExecuted(boolean prepareScriptExecuted) {

/**
* Gets the textual representation of the assigned label as it was entered by the user.
*
* @return User values
Copy link
Member

Choose a reason for hiding this comment

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

and here

// @Test
// public void testDeployToComputers2() {

// // TODO: I think this test isn't necesary, we don't copy in multiple slaves
Copy link
Member

Choose a reason for hiding this comment

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

agreed, maybe better removing it

Copy link
Member

@peppelan peppelan left a comment

Choose a reason for hiding this comment

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

Sorry for the extremely late review. Looks good to me, only minor non-functional changes suggested.

As this waited for way too much time, I would make a release out of it without waiting for these.

agiovannini and others added 6 commits August 21, 2018 19:01
Updated comments
Removed unused import
Removed Commented Code
Fixed Jenkins.MasterComputer null in tests
Simplify slave rootPath for Components Constructor
@nvasion
Copy link

nvasion commented Oct 22, 2018

Anything going on with this merge? I could really use this feature.

Thank you.

@peppelan
Copy link
Member

Hey, I missed the new commits fixing the CI.
Good to go!

@peppelan peppelan merged commit 2bbf6f1 into jenkinsci:master Oct 23, 2018
@MarkEWaite MarkEWaite added the enhancement New feature label Feb 12, 2023
MarkEWaite added a commit to MarkEWaite/slave-setup-plugin that referenced this pull request Feb 15, 2023
jenkinsci#11 was merged 5 years
ago but only released in 1.15 Feb 2023.  That change included a cleanup
step that removes specifically named files from the /tmp folder using
a Linux specific find command.  There is not enough benefit from that
cleanup step to justify the loss of portability to other Unix systems
like FreeBSD, OpenBSD, macOS, and Solaris.

https://issues.jenkins.io/browse/JENKINS-70622 reported the issue on
Solaris, AIX, and HP variants of Unix.
MarkEWaite added a commit that referenced this pull request Feb 16, 2023
#11 was merged 5 years
ago but only released in 1.15 Feb 2023.  That change included a cleanup
step that removes specifically named files from the /tmp folder using
a Linux specific find command.  There is not enough benefit from that
cleanup step to justify the loss of portability to other Unix systems
like FreeBSD, OpenBSD, macOS, and Solaris.

https://issues.jenkins.io/browse/JENKINS-70622 reported the issue on
Solaris, AIX, and HP variants of Unix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature
Projects
None yet
4 participants