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

JENKINS-21750 DSL support for the promoted-builds plugin (take 2 for #157) #291

Closed
wants to merge 4 commits into from

Conversation

steve-jansen
Copy link

Hi @dickerpulli and @daspilker

This is my humble attempt to help the project ship Thomas' awesome work in #157. I fully understand the motivation to create extension points in the DSL for other plugins in the future. Thomas' work is solid - I've been using a local .hpi build of his PR branch for many weeks now. It's been very reliable.

I think the Jenkins community overall would benefit quite a bit by shipping #157 as-is, and refactor the promoted-builds support in a separate PR for an extension point. The work on the extension point seems to have stalled out (no criticism - it's a heroic effort to do both the promoted builds and abstract that support to something generic at the same time).

In an effort to make help make you life easier as the maintainer of the project (and author of the original PR), I did a few things:

  1. rebased from the latest upstream master for v1.25 (this was no fun, thanks to @thommay for the first rebase back in July)
  2. squashed Thomas' work down to a single commit (maintaining Thomas as the author of course)
  3. added a small fix for parameterizedBuild actions in promotions

I hope you find this useful. The promoted-builds plugin can be a workhorse for continuous delivery in Jenkins, and the job-dsl plugin makes continuous delivery way easier to implement in Jenkins compared to point and click manual job creation in the UI. Having proper support for promotions in the job-dsl is a great step forward.

All the tests pass on my local build, however, the rebase was fairly complicated so I welcome some serious scrutiny on the changes.

Cheers,
Steve

@steve-jansen steve-jansen force-pushed the JENKINS-21750 branch 2 times, most recently from 5a4c733 to f11ed66 Compare September 25, 2014 04:44
@jenkinsadmin
Copy link
Member

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

@steve-jansen
Copy link
Author

Rebased my branch from the latest master.

@aehlke
Copy link

aehlke commented Oct 14, 2014

Thank you! So what is left here besides a code review, it sounds like there was unfinished work on adding some kind of extension point - is this necessary/important?

edit: Should have read more carefully, I understand now. Shipping it as-is and then refactoring to add proper extension point support sounds more in the spirit of continuous delivery!

@denschu
Copy link

denschu commented Oct 16, 2014

Will this change be included in the next version?

@b-dean
Copy link

b-dean commented Nov 13, 2014

@steve-jansen it looks like this might need to be rebased again (hoping that if you rebase it they'll release it) 😄

@bill-lin
Copy link

It will extremely helpful for us if this could be included in the next release
thanks!

@denschu
Copy link

denschu commented Nov 25, 2014

+1

2 similar comments
@dowhiletrue
Copy link

+1

@mattjones753
Copy link

+1

@steve-jansen
Copy link
Author

Hi All,

I've started rebasing this branch. The rebase is proving quite challenging due to the complexity required to implement support for promoted builds (credit @dickerpulli) along with some heavy refactoring of the dsl plugin in recent releases.

Hope to have this done in a few days after the Thanksgiving holiday in the US.

Cheers,
Steve

@aehlke
Copy link

aehlke commented Dec 3, 2014

@steve-jansen glad to hear!

@steve-jansen
Copy link
Author

@daspilker @dickerpulli rebased my branch from latest.

This rebase was time consuming as the internals of the plugin changed significantly from 1.24 to 1.27. I expect my refactoring to bring this up to date could use some attention, as I'm doing maintenance on commits made by other contributors.

Given the time commitment to rebase this branch multiple times, please take action on this PR, whether it be accept or reject.

@steve-jansen
Copy link
Author

@daspilker the pull request builder settings for this repo/cloudbees appears broken... the same PR build is being listed twice - one as complete, the second as forever "Jenkins is validating pull request ..."

The build passed @ https://jenkins.ci.cloudbees.com/job/plugins/job/job-dsl-plugin/587/

@steve-jansen
Copy link
Author

@denschu
Copy link

denschu commented Dec 23, 2014

I created a test environment with Docker and this pre-release. Works fine with my scripts...
See https://github.com/denschu/job-dsl-examples

dickerpulli and others added 4 commits December 23, 2014 09:44
Include the possibility to generate configs to use the
"Promoted Builds Plugin". Add new DSL components and generators
for creating XML files in the place where the plugin expects it to be.
a promoted-builds action of downstreamParameterized
is creating the wrong type of buildSteps;
it is creating `hudson.plugins.parameterizedtrigger.TriggerBuilder`
instead of the buildStep that the web ui creates,
`hudson.plugins.parameterizedtrigger.BuildTrigger`

this subtle difference breaks the expected behavior of
passing the current build parameters via `currentBuild()`
in DSL, resulting in no parameters being passed

this fix overrides the behavior of
`AbstractStepContext#downstreamParameterized`
to behave like the PublisherContext#downstreamParameterized by
calling `DownstreamContext#createDownstreamNode(false)`
@steve-jansen
Copy link
Author

@daspilker rebased my PR branch from latest.

Rebasing this PR is becoming a chore ... please accept or reject this PR 😃

@aehlke
Copy link

aehlke commented Dec 23, 2014

This would be wonderful to land, we're looking forward to adopting it here. Thanks for your work @steve-jansen

@daspilker
Copy link
Member

I'm not going to merge this pull request since we have chosen another approach to add support for the Promoted Builds Plugin by adding a Jenkins extension point, see https://groups.google.com/forum/#!topic/job-dsl-plugin/dEoZLcjqA0U.

That's because this pull request tries to implement a general solution for creating additional XML config files, but that problem only exists for the Promoted Builds plugin. At least as far as I know. The extension point is more versatile and will enable support for other plugins which require special configuration.

I'll leave this pull request open as a reference until the extension point is ready. Thanks to @steve-jansen for keeping this pull request up-to-date so that it's possible to run custom builds in the meantime.

@steve-jansen
Copy link
Author

Many thanks for weighing in.

How can I help to get job-dsl-promotions-plugin released?

The PR for JPI/JAR publishing by Gradle appears to be merged. Are there other dependencies preventing release of the promotions support?

@steve-jansen
Copy link
Author

For anyone following this thread on GitHub, the bottom line from the newsgroup thread is promoted-builds support could land in the DSL plugin sometime in March 2015 or later.

There are a number of major changes that need to happen, including:

  1. DSL plugin support for Jenkins extension points to allow other plugins to extend the behavior of the DSL plugin
  2. Updates to the Gradle build tooling for packaging plugins with Jenkins extension points
  3. Release of a new plugin, job-dsl-promotions-plugin, implements the extension point to generate promoted-builds config.xml files

Please leave comments if you have community interest in a binary install (.hpi file) that includes this PR's implementation of promoted-builds support. If there is enough interest, I can try to regularly rebase/release my fork.

@denschu
Copy link

denschu commented Feb 20, 2015

Any news here? Would be great, if this will be officially released in the near future :-)

@dickerpulli
Copy link
Contributor

@daspilker is still working on get the build process to run. Be patient ... :-)

@steve-jansen
Copy link
Author

An observation - this PR and it's parent (#157) have been open for one year.

@daspilker
Copy link
Member

As mentioned above, I'm not going to merge this PR, so I'm closing it now.

@daspilker daspilker closed this Feb 21, 2015
@fractal9
Copy link

Pity this PR hasn't been merged (yet?). At least as interim solution this will be extremely useful (even in case promoted-build DSL code needs a complete re-write with the general solution).

@daspilker, if there's no technical reasons preventing this "continuous" approach, then why not accept the PR for the time being?

Thomas & Steve, thanks a ton for developing and maintaining this!
I'm trying to apply the changes on top of 1.32 now -- unfortunately, there's some merge conflicts. So:

If there is enough interest, I can try to regularly rebase/release my fork.

This will be great.

Cheers
/alex

@denschu
Copy link

denschu commented Apr 17, 2015

+1

1 similar comment
@kgaitanos
Copy link

+1

@lionelve
Copy link

Seems like the extention points work is done.
@dickerpulli how's the job-dsl-promotions-plugin coming along?

Thanks.

@jewes
Copy link

jewes commented Apr 27, 2016

how is it going now?

@daspilker
Copy link
Member

This has been merged into the Promoted Builds plugin as a Job DSL extension, but the Promoted Builds plugin has not been released yet.

@oleg-nenashev, can you cut a release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet