RFH: Pipeline support for saltstack build plugin

[Christian McHugh]

Greetings all,

I am attempting to add pipeline support to the saltstack build plugin. But I'm having some difficulty wrapping my mind around the Jenkins framework part of things. 

Previously the plugin was a bit of a hack. There was a dropdown in the gui that based on the selection, included jelly files. The variables from these files were then passed to the single class constructor and thus it had to parse the json that was given. In deploying pipeline support, the groovy generator is not able to figure out this json nonsense, so it it time to break that up that process. 

Now I am attempting to create a few classes and pass their names to the various includes. At this stage, I think things are mostly working (although for anyone that knows what they are doing it is likely very ugly). However, when running the pipeline groovy generator, I get errors related to the lack of descriptors on the new classes. I have looked over the DropdownList ui-sample as that does precisely what I'd like, but I don't quite understand everything and could really use some additional guidance. I've just uploaded my currently broken draft code to https://github.com/jenkinsci/saltstack-plugin/tree/classing should anyone be able to assist.

Thanks much!

[Daniel Beck]

Hi Christian,

Here's my attempt at this (accidentally starting from master):

https://github.com/daniel-beck/saltstack-plugin/commits/refactor

This is the bare minimum to get it to print a non-default 'jobPollTime' (parameter to 'local' client interface) when run in a pipeline job (or freestyle job).

- Make the client interface into a Fruit-like (see ui-samples) class hierarchy
- Change the constructor parameter and getter types
- Implement SimpleBuildStep
- Get rid of all actual 'perform' so I could try to run it :-)

Note that this needs 1.609 or newer for f:class-entry, but I went up to 1.642.x to be able to install recent Pipeline releases in hpi:run

Of course it looks like there's some more refactoring needed to make it really nice:

- Break up constructors into ones with fewer arguments, and do the rest with @DataboundSetters, so fields with sensible defaults can be omitted from pipeline script
- Really move parameters for specific client interfaces into their classes (making existing fields transient)
- add readResolve compatibility code so existing job configs can be read successfully
- @Symbol support for easier to read pipeline scripts
- I'm not really happy with the $class/f:class-entry but otherwise I got Stapler errors, will need to look into alternatives

> --
> You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/c351fe08-af5f-4387-b341-54fd04ccb46f%40googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[Jesse Glick]

On Sun, Oct 23, 2016 at 5:17 PM, Daniel Beck <m...@beckweb.net> wrote:
> https://github.com/daniel-beck/saltstack-plugin/commits/refactor

I.e.,

https://github.com/daniel-beck/saltstack-plugin/compare/master...refactor

though filing a WiP PR is the best way to solicit line-by-line
feedback (for example, extend `AbstractDescribableImpl`).

> I'm not really happy with the $class/f:class-entry

You are trying to use low-level controls and making it unnecessarily
hard. KISS and use `f:dropdownDescriptorSelector`. Also see
`ui-samples-plugin` for examples of doing things the straightforward
way.

https://github.com/jenkinsci/workflow-step-api-plugin/#writing-pipeline-steps

[Daniel Beck]

> On 24.10.2016, at 16:04, Jesse Glick <jgl...@cloudbees.com> wrote:
>
> You are trying to use low-level controls and making it unnecessarily
> hard. KISS and use `f:dropdownDescriptorSelector`. Also see
> `ui-samples-plugin` for examples of doing things the straightforward
> way.

Well, I didn't want to rewrite all of the UI as well :-)

But it seems I'll have to…

[Christian McHugh]

Thanks all! 

I've been trying to reconcile this version with what I had going in https://github.com/jenkinsci/saltstack-plugin/tree/classing. I'll keep poking at it. 

At the moment, I'm attempting to discern how to get my dropdown selection to instantiate my class. So if the dropdown displays "local" the appropriate config.jelly is loaded, and I'd like the values from that page to instantiate my com.waytta.clientinterface.LocalClient class, while the main form continues to create the normal com.waytta.SaltAPIBuilder one. At the moment I everything in LocalClient is null, so I've got some fiddling to do. 

As far as the make things better comments:

Break up constructors into ones with fewer arguments, and do the rest with @DataboundSetters, so fields with sensible defaults can be omitted from pipeline script

Some work has been done for this in the classing branch

 

- Really move parameters for specific client interfaces into their classes (making existing fields transient)

Also partially done in the classing branch 

- add readResolve compatibility code so existing job configs can be read successfully

 I don't know what this means. Do you have an example?

- @Symbol support for easier to read pipeline scripts

 Done in the classing branch

- I'm not really happy with the $class/f:class-entry but otherwise I got Stapler errors, will need to look into alternatives

Also not sure what this means, so I'll investigate Jesse's suggestion with f:dropdownDescriptorSelector.

Seriously, thanks everyone!

[Daniel Beck]

> On 24.10.2016, at 18:58, Christian McHugh <christia...@gmail.com> wrote:
>
> - add readResolve compatibility code so existing job configs can be read successfully
> I don't know what this means. Do you have an example?

Currently, existing projects will have all the configuration stored in a serialized SaltAPIBuilder. When users upgrade the plugin and load that from disk, they want all their data to still be there, even if you moved things around in code.

The solution is to keep all existing fields in SaltAPIBuilder, but making them transient (so they no longer get serialized). Then, add a (private) readResolve method that looks whether values are set on those fields, and if they are, creates ClientInterface instances and moves the values over.

https://wiki.jenkins-ci.org/display/JENKINS/Hint+on+retaining+backward+compatibility

[Christian McHugh]

Hey all,

I think I've made a little headway doing a cleanup of the class bits. I've been trying to reconcile Daniel's branch and ui-samples with a more current upstream. Before touching any jelly, I am now able in instantiate the basic class. Now I am currently working at doing a more correct dropdownlist which calls the appropriate sub class. However, this is where I seem to be stuck at the moment. In the ui-samples drowndown example, there is a:

<j:set var="currentFruit" value="${it.fruit}"/>

followed by a dropdownListBlock. But in my case I can't get my it.BasicClient to be evaluated to anything.

If anyone has any thoughts on the classing branch available at https://github.com/jenkinsci/saltstack-plugin/tree/classing, I'm all ears.

Thanks much!

[Daniel Beck]

You get a `public getClientInterface()` in `SaltAPIBuilder`, then you can access that from the Jelly view as `it.clientInterface`.

> --
> You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/646e9526-211a-4005-8372-034149e7122e%40googlegroups.com.

[Jesse Glick]

On Thu, Oct 27, 2016 at 8:49 PM, Christian McHugh
<christia...@gmail.com> wrote:
> If anyone has any thoughts on the classing branch available at https://github.com/jenkinsci/saltstack-plugin/tree/classing, I'm all ears.

A general comment: if you are requesting help/comments/reviews, it is
best to file a pull request with your branch. It is conventional to
add a `work-in-progress` label (#bfd… color).

[Christian McHugh]

Okey dokie, I think I'm nearly out of the woods and I can now create a pipeline job with the approperate Symbol aliases. Unfortunately, I also have a problem with a descriptor constructor. With this new code, we are now seeing errors calling the new subclass descripters. This output is shown in:

https://github.com/jenkinsci/saltstack-plugin/pull/40

If anyone is able to help with this problem, it would be appreciated.

Thank you very much!