JENKINS-57252: Optional support for shelving projects instead of deleting them #1174
Conversation
|
Note: I tried to test the successful shelving in the |
cpfeiffer
force-pushed
the
shelve-plugin-support
branch
from
d9b609f
to
bd94e44
Compare
|
The test failure was unrelated to this change, so I just rebased on current master and force-pushed to trigger a new build. |
| private void shelve(Item project) { | ||
| Jenkins jenkins = Jenkins.get(); | ||
| if (! (project instanceof BuildableItem)) { | ||
| LOGGER.log(Level.WARNING, "Unable to shelve " + project + " since it is not a BuildableItem"); |
There was a problem hiding this comment.
This should be logged to the build output. No one will notice the problem when the info is hidden in the Jenkins log.
Would it also make sense to set the build result to unstable?
| } | ||
| BuildableItem item = (BuildableItem) project; | ||
| if (jenkins.getPlugin(SHELVE_PLUGIN_ID) == null) { | ||
| LOGGER.log(Level.WARNING, "Unable to shelve project " + item + " since the " + SHELVE_PLUGIN_ID + " plugin is not installed."); |
| LOGGER.log(Level.WARNING, "Unable to shelve project " + item + " since the " + SHELVE_PLUGIN_ID + " plugin is not installed."); | ||
| return; | ||
| } | ||
| jenkins.getQueue().schedule(new ShelveProjectTask(item), 0); |
There was a problem hiding this comment.
Hm, when shelving a lot of projects this will flood the build queue. Unfortunately there seems to be no bulk API in the shelve plugin. Can you open a feature request for the shelve plugin?
There was a problem hiding this comment.
Good point, I've opened https://issues.jenkins-ci.org/browse/JENKINS-58929.
|
|
||
| then: | ||
| jenkinsRule.jenkins.getItemByFullName('different-job') instanceof FreeStyleProject | ||
| // jenkinsRule.jenkins.getItemByFullName('test-job') == null |
There was a problem hiding this comment.
We need a proper test. Maybe you can use Spock's PollingConditions.
There was a problem hiding this comment.
Since deletion of parent folders doesn't work as long as the folder contains jobs, I had to change this to wait for the shelve task to finish and then delete any parent folders. That way I could actually test that the shelved project is gone afterwards.
I'm not 100% sure if the waiting for the shelve task completion will work in all scenarios.
ExecuteDslScripts is a build step, adding the ShelveProjectTask to the build queue and waiting for it to finish. AFAIU, the queue will always run Queue.FlyweightTask instances regardless of the number of executors configured. But I'm not a Jenkins developer, so if you have any input on that, please tell.
I don't know how to test that folders are getting deleted since "test-job" and "different-job" both have the same parent folder. One of them always exists, so the folder is never getting deleted.
| LOGGER.log(Level.WARNING, "Unable to shelve project " + item + " since the " + SHELVE_PLUGIN_ID + " plugin is not installed."); | ||
| return; | ||
| } | ||
| jenkins.getQueue().schedule(new ShelveProjectTask(item), 0); |
There was a problem hiding this comment.
ShelveProjectTask does not seem to check if the user has permission to delete a project. That is done upfront in ShelveProjectAction. So the permission must be checked here.
… separately Folders fail get removed when they still contain jobs, so we can only remove them when the jobs are gone. This means that we have to wait for the shelve task to finish before attempting to remove the folders.
cpfeiffer
force-pushed
the
shelve-plugin-support
branch
from
bd94e44
to
87d894d
Compare
|
@cpfeiffer can you ask the maintainer of the shelve plugin about the status of JENKINS-27734 and jenkinsci/shelve-project-plugin#14? That seems to be the better approach for handling folders and should be implemented first. Than Job DSL does not need any special handling for folders. |
|
Build finished. |
It would be nice to let the seed job archive unused projects instead of deleting them right away.