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

Declarative pipeline and ActiveData submission #293

Merged
merged 16 commits into from Mar 22, 2017

Conversation

@stephendonner
Copy link
Contributor

stephendonner commented Feb 25, 2017

[davehunt]: I removed the original description because this is now ready.

unstash 'workspace'
try {
writeCapabilities(capabilities)
withCredentials([[

This comment has been minimized.

Copy link
@davehunt

davehunt Feb 25, 2017

Member

This is something missing from the new pipeline. I've not done with with a cedentials file, but up on line 18 you could try:

VARIABLES = credentials('MOZILLIANS_VARIABLES')

Then on line 24 add it to the PYTEST_ADDOPTS:

      "--variables=capabilities.json " +
      "--variables=${VARIABLES}"

Hopefully that will work. 🤞

This comment has been minimized.

Copy link
@stephendonner

stephendonner Feb 26, 2017

Author Contributor

Doesn't seem to, but thanks for the suggestion:

org.codehaus.groovy.control.MultipleCompilationErrorsException: startup failed:
WorkflowScript: 25: ""--variables" is not a valid identifier and cannot be used for an environment variable. Identifiers must start with a letter or underscore and can contain only letters, numbers or underscores. @ line 25, column 7.
         "--variables=${VARIABLES}"
         ^

WorkflowScript: 17: Expected name=value pairs @ line 17, column 3.
     environment {
     ^

This comment has been minimized.

Copy link
@davehunt

davehunt Feb 27, 2017

Member

It must be that we can't reference another variable defined within environment. A couple of other suggestions, though it would be good to test these on a replay:

"--variables=${credentials('MOZILLIANS_VARIABLES')}"
"--variables=" + credentials('MOZILLIANS_VARIABLES')

If these don't work, we may need to ask on the Jenkins developer mailing list for advice.

This comment has been minimized.

Copy link
@davehunt

davehunt Feb 27, 2017

Member

I spoke to @abayer and apparently this will be addressed by jenkinsci/pipeline-model-definition-plugin#110. We may need to hold off upgrading this pipeline to declarative for now...

This comment has been minimized.

Copy link
@abayer

abayer Feb 27, 2017

Soonish, I promise!

This comment has been minimized.

Copy link
@stephendonner

stephendonner Mar 14, 2017

Author Contributor

@abayer what's the preferred, new way (if there are multiple) of setting this, in a post-1.1.1 Pipeline Model Definition plugin world? Thx!

This comment has been minimized.

Copy link
@abayer

abayer Mar 15, 2017

Probably something like MOZ_VARS = credentials('MOZILLIAN_VARIABLES') in the environment directive, and then add --variables=${MOZ_VARS} to the PYTEST_ADDOPTS declaration in environment - that should work. I think. =)

@stephendonner
Copy link
Contributor Author

stephendonner commented Mar 14, 2017

Staging Jenkins is now on 1.1.1 of Pipeline Model Definition Plugin, but still needs:

  1. https://issues.jenkins-ci.org/browse/JENKINS-42771
  2. for ansiColor() to pass in 'xterm' like so: ansiColor('xterm')
  3. For us to correctly set/pass in environment variables
stephendonner and others added 12 commits Feb 24, 2017
Copy link
Member

davehunt left a comment

Just a comment to add about known issues with declarative pipelines.

Jenkinsfile Outdated
}
stage('Test') {
environment {
PYTEST_ADDOPTS = "--color=yes --driver=SauceLabs --variables=capabilities.json --variables=${VARIABLES}"

This comment has been minimized.

Copy link
@davehunt

davehunt Mar 16, 2017

Member

Once you've opened the two issues, please add comments so we can move this into the main evnironment section and make it more readable by splitting it over multiple lines.

This comment has been minimized.

Copy link
@stephendonner

stephendonner Mar 16, 2017

Author Contributor

Done!

@davehunt davehunt changed the title Declarative pipeline and ActiveData submission, rough, broken, 1st cut Declarative pipeline and ActiveData submission Mar 16, 2017
Jenkinsfile Outdated
}
stage('Test') {
environment {
/** See https://issues.jenkins-ci.org/browse/JENKINS-42857 - we'd like to expand this out into multi-line concatenations */

This comment has been minimized.

Copy link
@davehunt

davehunt Mar 17, 2017

Member

Same comments as other pull requests regarding the issue number, and potential to hold out for the next version of the plugin. Sorry to comment the same in so many places, maybe we should aim to finish this one up before we move onto the others.

Jenkinsfile Outdated
stage('Test') {
environment {
/** See https://issues.jenkins-ci.org/browse/JENKINS-42771 - we'd like to expand this out into multi-line concatenations */
PYTEST_ADDOPTS = "--color=yes --driver=SauceLabs --variables=capabilities.json --variables=${VARIABLES}"

This comment has been minimized.

Copy link
@davehunt

davehunt Mar 22, 2017

Member

We're missing --tb=short

@stephendonner
Copy link
Contributor Author

stephendonner commented Mar 22, 2017

@davehunt final r?

Copy link

jrbenny35 left a comment

Python file and tox look good!

@stephendonner stephendonner dismissed davehunt’s stale review Mar 22, 2017

Addressed review comments, and had Benny take a look and r+

@stephendonner stephendonner merged commit 76150fb into mozilla:master Mar 22, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@stephendonner stephendonner deleted the stephendonner:declarative branch Mar 22, 2017
@stephendonner
Copy link
Contributor Author

stephendonner commented Mar 23, 2017

stephendonner added a commit that referenced this pull request Mar 23, 2017
This reverts commit 76150fb.
@davehunt
Copy link
Member

davehunt commented Mar 23, 2017

Doesn't this require Pipeline Definition Model plugin v1.1.1? I'm not sure we have this installed on our production Jenkins yet.

davehunt added a commit that referenced this pull request Mar 23, 2017
@stephendonner stephendonner restored the stephendonner:declarative branch Mar 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.