Add support for running promotions on inheritance projects. #75

J-cztery · 2015-11-21T06:12:25Z

This pull request contains an implementation of extension point to jenkins-inheritance-project-plugin and some changes to JobPropertyImpl construction to allow for interaction with jenkins-inheritance-project-plugin.
A new constructor has been added that allows reading promotion processes information from the location specified by an existing JobPropertyImpl but makes the JobPropertyImpl instance (and it's promotion processes) independent otherwise.

Initially I wanted to keep this extension separate (as a plugin) but this proved too difficult because of the hermetization of JobPropertyImpl and its insistence on loading promotion processes on construction.

Note that there is a problem in jenkins-inheritance-project-plugin and there is an accompanying fix to jenkins-inheritance-project-plugin plugin. That problem prevents from seeing promotions that are currently running. There might be some problems with getting it merged to mainline, so you might want to use my fork directly https://github.com/J-cztery/jenkins-inheritance-plugin.

This change introduces optional dependency to jenkins-inheritance-project.

It supersedes the following pull requests:
#74
#51 (thanks David Tanner for inspiration).

oleg-nenashev · 2015-11-21T10:05:41Z

src/main/java/hudson/plugins/promoted_builds/JobPropertyImpl.java

-        if(subdirs!=null) {
+        loadProcesses(subdirs);
+    }
+	private void loadProcesses(File[] subdirs) throws IOException {


tab/spaces mix

Ok. Thanks.

oleg-nenashev · 2015-11-21T10:26:07Z

@J-cztery Thanks for taking an action! the selected approach looks to be valid to me. From what I see the issue in inheritance plugin is not a blocker for this PR. Is it correct?

It would be also great to have a functional test verifying the new functionality

oleg-nenashev · 2015-11-21T10:27:53Z

src/main/java/hudson/plugins/promoted_builds/JobPropertyImpl.java

+            });
+
+    	loadProcesses(subdirs);
+    }


Mostly duplicates init(). Would be great to merge methods somehow

J-cztery · 2015-11-21T12:47:44Z

@oleg-nenashev: Thank you for the code review. I will work on your comments next. And yes, this change will make promotions usable without the change to jenkins-inheritance-plugin(but with some nuisances).

J-cztery · 2015-11-21T13:14:42Z

@oleg-nenashev: regarding the test. I struggle to write an unit tests that uses multiple plugins. Any ideas on how to set it up?

J-cztery · 2015-11-21T16:15:48Z

Yeah, I have just tested this fix with 1.5.3 version of inheritance build and the only problem i could find is that when the promotion build is running it is not shown on Promotion Status in either build context or project context. But it is shown in the queue.
The problem is due to the fact that inheritance plugin does not cache properties and generate properties on the fly and it so it generates many JobPropertyImpls....
I do not think this one is critical for the PR.

oleg-nenashev · 2015-11-21T20:16:04Z

@J-cztery
Thanks for the changes. The code looks good to me.
Inheritance plugin was in my personal blacklist for a long time due to functional issues, so unfortunately I'm not very familiar with its testing capabilities. BTW, in @JenkinsRule you will automatically get optional plugins installed by default. So you would be able to write a test there. If you describe the reproduction scenario, I probably can write a sample test.

Would you like to keep the test as a part of this PR? If no, I could merge this commit in order to test the behavior together with other changes.

J-cztery · 2015-11-22T02:55:47Z

@oleg-nenashev :
Thank you for reviewing this PR!
I have currently issues with running tests on Windows as this is my environment of choice.
CallStack.txt
I will try to get them working but this might take time.

If you would like to create a test then a scenario is :

Create an inheritance project BASE and add jobPropertyImpl with a promotion.
Create an inheritance project DERIVED and add a project reference (Ordered or Simple) to project BASE.
Now if you build DERIVED and if promotion works (does not crash) then it works ;)
So if you find time to write this test then ok, otherwise i will do it.

I do observe some functional issues with inheritance plugin as well but i am trying to kill them one by one. The problem i see with this plugin is that maintainer is not very responsive and the project is pretty much dead;(

What are your main concerns regarding the functionality of inheritance plugin?
I have added:

ability to specify node(label) of transient projects(in mating),
View full build flow actually shows full build flow.
Block when upstream and downstream project is building is now working.
Got parametrized trigger plugin working so that upstream/downstream relationship can be parameterized.
Added support to choose priorities of parents of transient projects(in mating)
Changed the way inheritance project decides if it should do full inheritance to blacklisting.
Recently added caching of properties (This is the thing that promotion-plugin needs).

oleg-nenashev · 2015-11-22T12:03:27Z

@J-cztery
this is a known issue with Windows tests on old Jenkins versions (locking of tmp dirs). You could run tests locally against new Jenkins core versions, then PR builder will run them against the actual version if you don't commit the version change.

We could also fork the Inheritance project plugin to jenkinsci org if @HedAurabesh wants to handover it. Maybe makes sense to start a thread in jenkinsci-dev and Cc owners there. The plugin is definitely useful, so it it great that somebody works on it.

J-cztery · 2015-11-22T12:51:35Z

@oleg-nenashev : Thanks for the hint. I will try to run tests against newer version of Jenkins. I have adapted one of the tests to use inheritance project but it would make sense(?) to run most of them if not all against FreeStyle project type as well as InheritanceProject. This would require making the unit tests slightly more complex...Do you think it would be worth it?

Let's deal with potential fork of inheritance project once this PR is finished.

oleg-nenashev · 2015-11-22T13:02:07Z

@J-cztery
Makes sense to create new tests instead of modifying the existing ones. But feel free to copy as much tests as you want if it allows to increase the coverage :)

For such integrations it also makes sense to create a spot-check in https://github.com/jenkinsci/acceptance-test-harness in order to catch regressions in Jenkins core, but it is optional && can be done after this PR.

oleg-nenashev · 2015-11-24T23:02:28Z

Performed manual spot-check and it seems to work.
Excluding it from the today's release to have a deeper look

J-cztery · 2015-11-25T11:29:51Z

Yeah.. I am still working on the unit tests and realized that @DavidTanner's change will still be needed (slightly modified to keep the binary compatibility as you requested) and i am not sure if i will be able to get unit tests working with inheritance-plugin 1.53.
So not releasing it just yet makes sense. I will let you know once i am done with tests.

1. Add unit tests. 2. Remove assert from PromotionProcess as in InheritanceProject it no longer holds. 3. Try to resolve build from jobName and build number first and if this fails, try others. 4. Implement an extension for inheritance-plugin

J-cztery · 2015-11-26T12:39:15Z

Ready @oleg-nenashev.

oleg-nenashev · 2015-12-11T08:09:31Z

👍

oleg-nenashev · 2015-12-11T08:09:45Z

@J-cztery thanks a lot for your efforts!

jenkinsadmin · 2015-12-28T17:48:02Z

Thank you for this pull request! Please check this document for how the Jenkins project handles pull requests.

Add support for running promotions on inheritance projects.

oleg-nenashev reviewed Nov 21, 2015
View reviewed changes

oleg-nenashev mentioned this pull request Nov 21, 2015

Change how build is resolved #51

Closed

J-cztery force-pushed the INHERITANCE_PROJECTS_SUPPORT branch from 8c7c388 to 63e1bfc Compare November 21, 2015 16:17

J-cztery mentioned this pull request Nov 25, 2015

Allow to hide some promotion processes based on parameter value. #76

Merged

J-cztery force-pushed the INHERITANCE_PROJECTS_SUPPORT branch 2 times, most recently from f8646cb to 1b5f7e6 Compare November 26, 2015 10:34

oleg-nenashev added this to the 2.25 milestone Dec 11, 2015

oleg-nenashev added the ready-for-merge label Dec 11, 2015

oleg-nenashev added a commit that referenced this pull request Feb 18, 2016

Merge pull request #75 from J-cztery/INHERITANCE_PROJECTS_SUPPORT

828da6d

Add support for running promotions on inheritance projects.

oleg-nenashev merged commit 828da6d into jenkinsci:master Feb 18, 2016

J-cztery deleted the INHERITANCE_PROJECTS_SUPPORT branch March 30, 2016 08:52

jglick mentioned this pull request May 10, 2021

Use newer project-inheritance dependency (19.08.02 instead of 1.5.3) #153

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for running promotions on inheritance projects. #75

Add support for running promotions on inheritance projects. #75

J-cztery commented Nov 21, 2015

oleg-nenashev Nov 21, 2015

J-cztery Nov 21, 2015

oleg-nenashev commented Nov 21, 2015

oleg-nenashev Nov 21, 2015

J-cztery commented Nov 21, 2015

J-cztery commented Nov 21, 2015

J-cztery commented Nov 21, 2015

oleg-nenashev commented Nov 21, 2015

J-cztery commented Nov 22, 2015

oleg-nenashev commented Nov 22, 2015

J-cztery commented Nov 22, 2015

oleg-nenashev commented Nov 22, 2015

oleg-nenashev commented Nov 24, 2015

J-cztery commented Nov 25, 2015

J-cztery commented Nov 26, 2015

oleg-nenashev commented Dec 11, 2015

oleg-nenashev commented Dec 11, 2015

jenkinsadmin commented Dec 28, 2015

Add support for running promotions on inheritance projects. #75

Add support for running promotions on inheritance projects. #75

Conversation

J-cztery commented Nov 21, 2015

oleg-nenashev Nov 21, 2015

Choose a reason for hiding this comment

J-cztery Nov 21, 2015

Choose a reason for hiding this comment

oleg-nenashev commented Nov 21, 2015

oleg-nenashev Nov 21, 2015

Choose a reason for hiding this comment

J-cztery commented Nov 21, 2015

J-cztery commented Nov 21, 2015

J-cztery commented Nov 21, 2015

oleg-nenashev commented Nov 21, 2015

J-cztery commented Nov 22, 2015

oleg-nenashev commented Nov 22, 2015

J-cztery commented Nov 22, 2015

oleg-nenashev commented Nov 22, 2015

oleg-nenashev commented Nov 24, 2015

J-cztery commented Nov 25, 2015

J-cztery commented Nov 26, 2015

oleg-nenashev commented Dec 11, 2015

oleg-nenashev commented Dec 11, 2015

jenkinsadmin commented Dec 28, 2015