[JENKINS-34912] vSphere from Pipeline integation

[witokondoria]

No description provided.

[ekostjuk]

Thanks dude!

[jswager]

Is this necessary? This will require every user to be Jenkins 2.6 - not yet released. And not everyone is upgrading to the very latest and greatest immediately. Can a lower version be safely used?

[witokondoria]

Seems like org.jenkins-ci.plugins:plugin since ver. 2 decouples jenkins.version from pom version (see jenkinsci/plugin-pom/blob/master/README.md). For parent 2.6, the jenkins version to be build to is 1.625.3

A lower versión could be used, indeed.

Neverthless, I'm currently using a snapshot with this PR on a Jenkins 2.4
and is working (so there seems no API changes has been made).

Will try on Monday to lower that parent, rebuild and retest.

[jswager]

Just noticed that the minimum version still targets 2.6. That works, but will force everyone to upgrade their Jenkins. And there's a security fix in the 2.x line that has broken several plugins, so the adoption rate isn't that good. Were you able to retest with a lower version of Jenkins than 2.6?

[ekostjuk]

Have you tried exposing guest env vars in groovy code? On 2.5:

vSphere buildStep: [$class: 'ExposeGuestInfo', vm: 'foo', envVariablePrefix: "foo"], serverName: 'VSPHERE'

println env.foo_IpAddress

// null

Log suggests that it 'Successfully exposed guest info for VM "foo"'

[witokondoria]

Freestyle jobs and pipeline ones dont access to env vars in the same way. thus witokondoria/vsphere-cloud-plugin/blob/5eeffd8e1d506bf4aaa53ddfff47427762598796/src/main/java/org/jenkinsci/plugins/workflow/vSphereStep.java#L126 comes into action. Unfortunately, has just been tested for PowerOn, Clone and Deploy actions. Can add ExposeGuestInfo, but ommitting the envVariablePrefix element.

WDYT?

Edit:
After a quick read, that method exposes several variables. Due that different way to expose env vars (Cant find the original issue), I would jusr keep the VSPHERE_IP env var for pipelines.

Edit2:
https://issues.jenkins-ci.org/browse/JENKINS-29144, states that SimpleBuildStep doesnt have and EnvVars parameter, thus cant operate on them

[ekostjuk]

Looks like just returning IP without prefix from a ExposeGuestInfo is the simplest way to get an IP non-destructively.
Was thinking of using "protected Map run()..." with EnvVar defined in vSphereStep returned via getter/class reflection (as you do it for IP). Hacky and scope creep... WDYT?

Edit: https://issues.jenkins-ci.org/browse/JENKINS-29144 is that about passing in existing EnvVar to steps? It could still output a new EnvVar (since its just a map) for each step.

[witokondoria]

Certainly is a hacky and scope creep getter, but for the meantime, looks like a solution. Will keep publishing the VSPHERE_IP for ExposeGuestInfo. @noquirks, could you test that method?

[ekostjuk]

How would you get the instance of step (to run a getter on it), if new is forbidden and build steps are de-serialized from $class when you call a step? For example, how would you achieve below:

def info = *instantiate*([$class: "org.jenkinsci.plugins.vsphere.builders.ExposeGuestInfo", vm:"foo", envVariablePrefix:"foo"])
vSphere buildStep: info, serverName: 'VMHOSTS'
echo info.getVm() // or info.getEnvVar() in this case

I thought of changing String run() to Map run() in vSphereExecution and extending from AbstractSynchronousNonBlockingStepExecution instead.

Looks like this MR will define an API contract for workflows for this plugin making future changes major/difficult, trying to cover all bases here, but might be overdesign in the first place.

[witokondoria]

Returning a Map instead of a String looks a littel bit confusing (IMHO) as tree of four possible methods returning (Clone, Deploy and PowerOn)values would just return a onekey- value Map.

About getting a step instance, I think is not a nice usage. With the last commit, an env var is published, following the envVariablePrefix_value pattern

[witokondoria]

@jswager what are your thoughts about this PR? could be merged?

[jswager]

Sorry - didn't see that last code commit. I'm fine with merging and will do so shortly...

[jswager]

Didn't notice it a first, but still using 2.6 as baseline Jenkins version. Can a lower version be used?

[witokondoria]

I wrote a previous comment:

Seems like org.jenkins-ci.plugins:plugin since ver. 2 decouples jenkins.version from pom version (see jenkinsci/plugin-pom/blob/master/README.md). For parent 2.6, the jenkins version to be build to is 1.625.3

[jswager]

Ah - I see. OK, then. Merging...

[oleg-nenashev]

@jswager AFAIK this PR also added support of vSphere 5.5 and above (as a part of the migration to yavijava). Maybe it deserves mentioning in the changelog

[witokondoria]

@oleg-nenashev true that: we are up and running on vSphere 6