jenkinsci / last-changes-plugin

https://plugins.jenkins.io/last-changes
https://plugins.jenkins.io/last-changes/
31 stars 30 forks source link

Improvement: Don't copy repo to master, run diff on slave #66

Closed herrmanntom closed 4 years ago

herrmanntom commented 5 years ago

This is sort of related to Issue #29, but I am unsure if I should reopen it - so I created this new issue.

We have the following scenario:

Last-changes-plugin now copies .git from workspace on slave to the master in order to publish changes to the master.

In our use case we encounter "out of space" conditions in Jenkins Home on the master, because the repo is larger than the remaining space in Jenkins home (50+GB, vs ~30GB).

This has three problems with two quick fixes I can imagine: 1st) when this happens during the copy, the plugin does not clean up the repo clone anymore. I would say this should be fixed for sure. Otherwise basically all operations on Jenkins stop, as it cannot work without space in Jenkins home.

2nd) git submodules are copied (.git/modules) even though the plugin currently does not work with submodules at all. Maybe as long as submodules are not supported, this part of the repo could be skipped. Later when submodules are supported make there some option for it.

3rd) As the copy takes quite some time, multiple concurrent builds might copy their repos at the same time. This makes the problem worse, as it can happen even with smaller repos.

Here I cannot see some simple fix, but I wonder if one cannot just run appropriate commands to get the raw diff data on the slave within the workspace. Then transfer only this raw diff to the master and run the beautification etc there.

rmpestano commented 5 years ago

Hi @herrmanntom

I wonder if one cannot just run appropriate commands to get the raw diff data on the slave within the workspace. Then transfer only this raw diff to the master and run the beautification etc there.

I'll try something on that direction to avoid the copying, get back to you as soon as I have something working.

Thanks for the feedback and idea.

rmpestano commented 5 years ago

Alright now I remember why we need the copy: the plugin execution/step happens on the master, only the job execution happens on the slave.

I'll investigate further if there is a way to run the diff on the slave.

rmpestano commented 5 years ago

Hi again,

I've made an attempt to avoid the copy but without success. The code is on this branch.

Basically is basically invoke last changes remotely when the job runs on a slave.

But I'm getting java.nio.file.NoSuchFileException here when, trying to access the VCS dir inside the closure:

Invoking last changes remotely...

mai 20, 2019 10:05:39 AM INFORMAÇÕES com.github.jenkins.lastchanges.LastChangesPublisher$LastChangesCallable invoke

File absolute: /var/jenkins/builds/workspace/teste/.git

mai 20, 2019 10:05:39 AM GRAVE com.github.jenkins.lastchanges.LastChangesPublisher perform

Could not publish LastChanges.
java.nio.file.NoSuchFileException: /var/jenkins/builds/workspace/teste/.git
    at sun.nio.fs.UnixException.translateToIOException(UnixException.java:86)
    at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)
    at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107)

Maybe someone with more expertise in Jenkins can help us, any ideas @oleg-nenashev?

We want to avoid the coppying of VCS dir from slave to master here, do you think it is a good idea to invoke the plugin logic on the slave? if so, can you help making it working?

Thank you in advance!

herrmanntom commented 5 years ago

Hi,

I experimented a bit myself: https://github.com/herrmanntom/last-changes-plugin/commit/b1daeb799880238444b653a4fe6582aba33c5605

Here I could successfully run the git code on the slave and also extract the results. Main difference is usage of launcher.getChannel().call(task); to run the code. I think you should not/cannot call "invoke" yourself. If you do then you just invoke it on the master. Howerver, its quite the hackjob because I removed lots of cases and also used Callable instead of FileCallable. With more time you can probably adjust it to use FileCallable. Also I think you do not need to differentiate between slave and master. You let Jenkins run the callable on what ever executor and move the discovery logic from workspace down into the callable. This way code does not care if it runs on master or some slave and you have no duplication.

rmpestano commented 5 years ago

Thank you @herrmanntom! I'll have a look as soon find I some more time.

david-mntl commented 4 years ago

Hello @rmpestano , I implemented the fix for this issue. Now if the build is done on the slave then the source control folder (.git or .svn) will not be copied to master node and the diff will be done on the slave. The case, when the build is executed on the master node, is also handled properly.

I created the pull request: #73

Could you please review the changes and integrate them, if you find them ok. Otherwise, please give me your feedback about what should be improved in order to proceed with the integration.

The CI for the PR failed on windows environment, but it seems that the same error was happening before my changes.

Thanks!

rmpestano commented 4 years ago

Many thanks for the great contribution @david-mntl! The fix will be on v2.7.8 to be released soon