openshift / jenkins-plugin

Apache License 2.0
81 stars 50 forks source link

Make methods easier to call from Jenkinsfile pipeline build #36

Closed spadgett closed 8 years ago

spadgett commented 8 years ago

Most steps in a Jenkinsfile take named parameters and use a syntax that is very readable. See the following examples:

https://github.com/jenkinsci/pipeline-plugin/blob/master/TUTORIAL.md#understanding-syntax

Is it possible to make it that easy to call OpenShift plugin methods? Today the calls are verbose.

node('agent') {
stage 'build'
def builder = new com.openshift.jenkins.plugins.pipeline.OpenShiftBuilder("", "ruby-sample-build", "myproject", "", "", "", "", "true", "", "")
step builder
stage 'deploy'
def deployer = new com.openshift.jenkins.plugins.pipeline.OpenShiftDeployer("","frontend","myproject","","")
step deployer
}

I want to write something like this

openShiftBuilder buildConfig: 'ruby-sample-build', namespace: 'myproject'

with appropriate defaults for params I leave out. Since end users will need to write Jenkinsfiles, we should make it as easy as possible.

@bparees @gabemontero @sspeiche @jwforres

spadgett commented 8 years ago

See also https://github.com/jenkinsci/pipeline-plugin/blob/master/DEVGUIDE.md

gabemontero commented 8 years ago

@spadgett see https://trello.com/c/jOEuMkze/924-provide-jenkins-dsl-for-openshift-pipeline-plugin-steps-plug-points

I'll cross reference this issue there.

spadgett commented 8 years ago

Thanks @gabemontero I'll close this issue since we're tracking in Trello.

gabemontero commented 8 years ago

Hey @spadgett @bparees @sspeiche @fabianofranz @jwforres @siamaksade - I reopened this issue to facilitate the discussion on the precise syntax and naming for the DSL, given some of the relevant history is already documented here, as well as which jenkins plugin extension points to leverage (we have choices, and more than one has been suggested).

I'll post an update with some early results from my prototyping momentarily.

thanks.

gabemontero commented 8 years ago

So my initial prototype (and favorite) is along the lines of what @spadgett outlined in this issue's description, and builds on the README for building Pipeline steps as well some reference examples

Following this model, I have a DSL where a build can be started from a Jenkinsfile / pipeline groovy script with something as simple as this:

def builder = OpenShiftBuilder bldCfg: 'frontend'

A couple of notes: 1) you do NOT have to say step builder afterward; execution of the script with the full complement of environment varialbes, etc. will commence as part of constructing this object 2) Both "OpenShiftBuilder" and "bldCfg" are configurable names ... I'm already anticipating feedback on both the step name and parameter names (i.e. bldCfg vs. buildConfig) :-) 3) Currently within the context of a Jenkinsfile, this command will block the flow until its completion; however, there is a lot of recommendation / infrastructure around allowing these steps to be asynchronous. My initial sense if that for our flows we'll always want synchronous, but I fully admit that I very likely may not fully envision how user scenarios might involve here. That said, I see nothing preventing the addition of asynchronous versions of things down the line. Minimally, we could debate whether to provide an indication of synchronous vs. asynchronous in the name.

gabemontero commented 8 years ago

Use of the Job DSL plugin has also been suggested. It is separate from the pipeline/workflow plugins, and if I remember correctly, it is precursor of sorts to workflow/pipeline (though I'm not absolutely certain for sure).

Today at least, the dev version of the OpenShift Jenkins image is NOT installing this plugin. Just the pipeline/workflow related plugins and dependencies. And I'm fairly certain that if you construct a new job from the Jenkins UI, it is listed as a separate type.

Examples of its syntax look like this or this

@sspeiche agreed to do some research on how important it is to provide job-dsl-plugin style syntax in addition to the latest styles proposed under the workflow umbrella.

Pending findings / comments to contrary here, I'm inclined to NOT provide this style of DSL extension as part of https://trello.com/c/jOEuMkze/924-provide-jenkins-dsl-for-openshift-pipeline-plugin-steps-plug-points

gabemontero commented 8 years ago

Last initial item as part of reopening this issue for soliciting feedback / discussion: the kubernetes client plugin from the fabric guys promotes that it provides a "fluent DSL".

I'll clarify (and let me be clear that in no way is this a criticism) that their DSL does not build off of job-dsl-plugin, workflow-step-api, or workflow-cps DSL related plugins. The examples they cite there are in fact Java languages invocations that can be made from groovy scripts. And as I understand it, they don't provide classic Jenkins Freestyle Job build steps.

My basic take at this time for https://trello.com/c/jOEuMkze/924-provide-jenkins-dsl-for-openshift-pipeline-plugin-steps-plug-points is not to restructure along similar lines. I think in the end, we would have something that syntactically looks very similar to

sh 'oc start-build frontend

Perhaps a consideration later on as we progress through delivery in 3.3 and even get some customer feedback. But just not seeing it as a need currently.

bparees commented 8 years ago

def builder = OpenShiftBuilder bldCfg: 'frontend'

definitely buildConfig :) Where do other parameters go? would the full command be something like:

def builder = OpenShiftBuilder buildConfig: 'frontend' showLogs: 'true' verbose: 'true' namespace: 'foobar'

?

My initial sense if that for our flows we'll always want synchronous

agree. you can always have parallel steps if you want effectively async behavior, right? (rather than making the implementation asynchronous).

regarding the actual syntax... i like the job-dsl style of:

build 'bcName' 'namespacename' 'other args'

the "def builder = ....." syntax strikes me as strange, especially since as you note that actually executes it, even though it looks like you're just defining it. and what can i do w/ the builder object after defining it, if anything?

I could also get behind a fluent syntax like:

buildConfig('bcname').in('namespace').setVerbose('true').showLogs('true').triggerAndVerifyDeploy()
gabemontero commented 8 years ago

Bits flowing in parallel .... just discovered that fabric does in fact, in their kubernetes workflow plugin https://github.com/fabric8io/kubernetes-workflow (so many different plugins :-( ), does in fact implement the same workflow DSL extension points as my prototype. Now that I know more precisely what to look for, I was able to improve upon my earlier scans.

I'll start studying what they do precisely and update this issue with findings.

Otherwise, embedded responses below.

On Wed, May 25, 2016 at 3:08 PM, Ben Parees notifications@github.com wrote:

def builder = OpenShiftBuilder bldCfg: 'frontend'

definitely buildConfig :) Where do other parameters go? would the full command be something like:

def builder = OpenShiftBuilder buildConfig: 'frontend' showLogs: 'true' verbose: 'true' namespace: 'foobar'

?

Close. Need to separate with commas.

def builder = OpenShiftBuilder buildConfig: 'frontend', showLogs: 'true', verbose: 'true', namespace: 'foobar'

My initial sense if that for our flows we'll always want synchronous

agree. you can always have parallel steps if you want effectively async behavior, right? (rather than making the implementation asynchronous).

regarding the actual syntax... i like the job-dsl style of:

build 'bcName' 'namespacename' 'other args'

As I understand it (admittedly though my understanding is still evolving), you are missing the { and } which are needed (and which I'm not a big fan of). It would be something like either build { buildConfig 'bcName' namespace 'namespaceName' }

or

build { buildConfig('bcName') namespace('namespaceName') }

Also, with the newly discovered fabric kubernetes-workflow, they do NOT claim job-dsl-plugin as a dependency. So for what it is worth, they are not implementing those extension points.

the "def builder = ....." syntax strikes me as strange, especially since as you note that actually executes it, even though it looks like you're just defining it. and what can i do w/ the builder object after defining it, if anything?

I believe I could disable the automatic execution and still force the use of step builder if that was more desirable. I did experiment a little bit with not having def builder and just having OpenShiftBuilder ... but no luck as of yet. But I'm going to circle back to this (especially after looking at fabric's kubernetes-workflow some more).

I could also get behind a fluent syntax like:

buildConfig('bcname').in('namespace').setVerbose('true').showLogs('true').triggerAndVerifyDeploy()

Yeah I appreciate the look / feel as well .... to reiterate from earlier, IMO it is a priority / staging sort of decision for not doing right now.

— You are receiving this because you were assigned. Reply to this email directly or view it on GitHub https://github.com/openshift/jenkins-plugin/issues/36#issuecomment-221675671

bparees commented 8 years ago

you are missing the { and } which are needed (and which I'm not a big fan of).

ok fair enough, i clearly didn't fully understand the job-dsl syntax fully. I agree that's uglier and i'm not such a fan.

gabemontero commented 8 years ago

On Wed, May 25, 2016 at 3:41 PM, Ben Parees notifications@github.com wrote:

you are missing the { and } which are needed (and which I'm not a big fan of).

ok fair enough, i clearly didn't fully understand the job-dsl syntax fully. I agree that's uglier and i'm not such a fan.

ok cool

— You are receiving this because you were assigned. Reply to this email directly or view it on GitHub https://github.com/openshift/jenkins-plugin/issues/36#issuecomment-221684388

gabemontero commented 8 years ago

I figured out how to avoid the def builder = snippet (thanks to some cross referencing with the fabric stuff).

With the same prototype, I'm able to simply put in my groovy script:

OpenShiftBuilder(bldCfg: 'frontend')

They have similar step kubernetesApply(....) that takes parameters in the same way.

On the naming front, aside from the bldCfg -> buildConfig, per @spadgett 's origin sample, OpenShiftBuilder -> openShiftBuilder or openShiftBuild makes sense now.

I also discovered their use of org.jenkinsci.plugins.workflow.cps.GlobalVariable as the entryway into some fluent style syntax. Not committing to anything yet for this sprint, but initially it is easing my sizing concerns a bit.

gabemontero commented 8 years ago

I was able to mimic our classic "OpenShift Sample Job" in a pipeline job with the following pipeline groovy script, using the workflow DSL plugin points where you can execute a single step and the number of parameters can vary (where there is a minimum set, followed by optional parameters)

node { openShiftScale(deploymentConfig: 'frontend', replicaCount: '0') openShiftBuild(buildConfig: 'frontend') openShiftVerifyDeployment(deploymentConfig: 'frontend') openShiftScale(deploymentConfig: 'frontend', replicaCount: '1', verifyReplicaCount: 'true') openShiftVerifyService(serviceName: 'frontend') openShiftTag(sourceStream: 'origin-nodejs-sample', sourceTag: 'latest', destinationStream: 'origin-nodejs-sample', destinationTag: 'prod') openShiftVerifyDeployment(deploymentConfig: 'frontend-prod') }

Some points on the names I considered: 1) I believe say openShiftBuild could be named say oc.start-build or oc-start-build (need to still try them out) .... certainly it could be ocStartBuild or ocBuild ... similar alterations with the others of course 2) I went back and forth between openShift and openshift

Pending feedback, I'll start on the fluent DSL style, though I'll solicit feedback there as well. The fabric guys use a root of kubernetes. for all the subsequent Java invocations. It is not quite an apples to oranges comparison for us, but using that as a base, I could see us going with 1) openshift. ... most analogous to what they have 2) oc. ... what current users are used to

2) seemed the first choice

After the period, I'm thinking these methods:

The biggest trick is going to be all the default settings for project name, api URL, tokens from build ENV vars ... they won't be readily available via the fluent DSL approach (there isn't anything analogous with fabric). I can probably access the mounted dirs to get most of this, but I'd like to avoid that if possible.

bparees commented 8 years ago

I think i could definitely live w/ the first example you show above (the one w/ parameters), and i don't have very strong feelings about the names but i think i'm disinclined to include "oc" in the naming, since that implies a similarity that may not run all the way through.

sspeiche commented 8 years ago

The first example looks very usable. Though I could see something like this being quite handy as well:

oc.build('mybuild').start() or oc.build('mybuild').start().verbose().follow().do()

oc.deploy('mydeploy').start()

gabemontero commented 8 years ago

On Thu, May 26, 2016 at 5:26 PM, Ben Parees notifications@github.com wrote:

I think i could definitely live w/ the first example you show above (the one w/ parameters), and i don't have very strong feelings about the names but i think i'm disinclined to include "oc" in the naming, since that implies a similarity that may not run all the way through.

Cool, since I had made the same comment to @sspeiche about oc here at my cube earlier this afternoon :-)

— You are receiving this because you were assigned. Reply to this email directly or view it on GitHub https://github.com/openshift/jenkins-plugin/issues/36#issuecomment-221999713

gabemontero commented 8 years ago

Yeah, I think the naming of the oc. bit will cause the most consternation. Let's sleep on it.

On Thu, May 26, 2016 at 5:29 PM, Steve Speicher notifications@github.com wrote:

The first example looks very usable. Though I could see something like this being quite handy as well:

oc.build('mybuild').start() or oc.build('mybuild').start().verbose().follow().do()

oc.deploy('mydeploy').start()

— You are receiving this because you were assigned. Reply to this email directly or view it on GitHub https://github.com/openshift/jenkins-plugin/issues/36#issuecomment-222000381

gabemontero commented 8 years ago

I've finished my dive into the fabric implementation of the pipeline "GlobalVariable" api / extension point and fluent style API, and concluded that to do it well will require a good bit of work. This, along with the feedback already received, I'm punting on taking that on with this iteration's effort.

Of course we can track user consumption of this and can add cards/work in the future for this (similar to Job DSL plugin stance).