Skip to content

Jenkinsfile cleanup and improvements#39656

Merged
tiborvass merged 12 commits intomoby:masterfrom
thaJeztah:jenkinsfile_cleanup
Aug 3, 2019
Merged

Jenkinsfile cleanup and improvements#39656
tiborvass merged 12 commits intomoby:masterfrom
thaJeztah:jenkinsfile_cleanup

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

See individual commits for what changed

@thaJeztah
Copy link
Copy Markdown
Member Author

Booo! Looks like this version either doesn't support label, or it must be in a specific order?

org.codehaus.groovy.control.MultipleCompilationErrorsException: startup failed:
WorkflowScript: 58: Invalid parameter "label", did you mean "script"? @ line 58, column 32.
                               sh label: 'Remove dev container', script: '''

@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Aug 2, 2019

Label option was added in workflow-durable-task-step 2.28 (https://issues.jenkins-ci.org/browse/JENKINS-55410 / jenkinsci/workflow-durable-task-step-plugin#93), so perhaps the plugin installed is too old?

@andrewhsu
Copy link
Copy Markdown
Contributor

cc @zelahi for review

Jenkinsfile Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're wanting to label these shell steps you could just stages within stages

Copy link
Copy Markdown
Contributor

@zelahi zelahi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, the only thing that was a little interesting was having DOCKER_BUILDKIT=1 globally. I remember running issues on pp and s390x when we were running these tests, but if they were fixed it should be good

Jenkinsfile Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We ran into an issue where it was set locally but then the tests would consistently fail on s390x-ubuntu-1604. Was it still failing?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm.. didn't know that was the case; would be interesting to know why (we can still override it for s390x/power)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, not sure why docker build with buildkit is bonking out on 18.06...docker-ce 18.06.3 was the last s390x package version we released because the hardware maintenance has been difficult and almost no users. Perhaps we should start disabling the tests for s390x because we don't intend to ship docker-ce packages for s390x in the future.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i see you set environment { DOCKER_BUILDKIT = '0' } in s390x and ppc64le steps. i think that's fine for now.

Jenkinsfile Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be in a cleanup block in the Jenkinsfile? https://jenkins.io/doc/book/pipeline/syntax/#post

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewriting the PR now to use substages; good suggestion, I'll use a cleanup block for this

@thaJeztah thaJeztah force-pushed the jenkinsfile_cleanup branch from 4d7d374 to 709e45b Compare August 2, 2019 18:02
From the code style guidelines;
https://wiki.jenkins.io/display/JENKINS/Code+Style+Guidelines

> 1. Use spaces. Tabs are banned.
> 2. Java blocks are 4 spaces. JavaScript blocks as for Java. XML nesting is 2 spaces

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The main Dockerfile is multi-arch now.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Container and image names are already unique because they have
the git-sha or build-number, and a single machine won't be running
tests for multiple architectures.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the jenkinsfile_cleanup branch from 709e45b to b04c769 Compare August 2, 2019 18:12
@thaJeztah
Copy link
Copy Markdown
Member Author

Looks indeed like both Power and Z are failing with BuildKit;

[2019-08-02T18:12:52.300Z] + git rev-parse --short HEAD
[2019-08-02T18:12:52.300Z] + GITCOMMIT=b04c769
[2019-08-02T18:12:52.300Z] + docker build --force-rm --build-arg APT_MIRROR -t docker:b04c769 -f Dockerfile .
[2019-08-02T18:12:52.300Z] 
[2019-08-02T18:12:52.300Z] #2 local://dockerfile (Dockerfile)
[2019-08-02T18:12:52.300Z] #2       digest: sha256:a87b3c6a302283c3cd74ddc8c55aff23dd0aee216868d02c86ab77ed7279cc55
[2019-08-02T18:12:52.300Z] #2         name: "local://dockerfile (Dockerfile)"
[2019-08-02T18:12:52.300Z] #2      started: 2019-08-02 18:12:52.16056608 +0000 UTC
[2019-08-02T18:12:52.300Z] #2    completed: 2019-08-02 18:12:52.160721894 +0000 UTC
[2019-08-02T18:12:52.300Z] #2     duration: 155.814µs
[2019-08-02T18:12:52.300Z] #2      started: 2019-08-02 18:12:52.162606802 +0000 UTC
[2019-08-02T18:12:52.300Z] #2    completed: 2019-08-02 18:12:52.191543126 +0000 UTC
[2019-08-02T18:12:52.300Z] #2     duration: 28.936324ms
[2019-08-02T18:12:52.300Z] #2 transferring dockerfile: 10.29kB done
[2019-08-02T18:12:52.300Z] 
[2019-08-02T18:12:52.300Z] 
[2019-08-02T18:12:52.300Z] #1 local://context (.dockerignore)
[2019-08-02T18:12:52.300Z] #1       digest: sha256:21ff54b81d9705536327625e4b1d381ba396c00cc28e60237482858d09a97541
[2019-08-02T18:12:52.300Z] #1         name: "local://context (.dockerignore)"
[2019-08-02T18:12:52.300Z] #1      started: 2019-08-02 18:12:52.16056608 +0000 UTC
[2019-08-02T18:12:52.300Z] #1    completed: 2019-08-02 18:12:52.160740993 +0000 UTC
[2019-08-02T18:12:52.300Z] #1     duration: 174.913µs
[2019-08-02T18:12:52.300Z] #1      started: 2019-08-02 18:12:52.162981976 +0000 UTC
[2019-08-02T18:12:52.300Z] #1    completed: 2019-08-02 18:12:52.189489938 +0000 UTC
[2019-08-02T18:12:52.300Z] #1     duration: 26.507962ms
[2019-08-02T18:12:52.300Z] #1 transferring context: 87B done
[2019-08-02T18:12:52.300Z] 
[2019-08-02T18:12:52.300Z] failed to create LLB definition: invalid reference format
script returned exit code 1

[2019-08-02T18:12:37.203Z] + git rev-parse --short HEAD
[2019-08-02T18:12:37.203Z] + GITCOMMIT=b04c769
[2019-08-02T18:12:37.203Z] + docker build --force-rm --build-arg APT_MIRROR -t docker:b04c769 -f Dockerfile .
[2019-08-02T18:12:37.203Z] 
[2019-08-02T18:12:37.203Z] #1 local://dockerfile (Dockerfile)
[2019-08-02T18:12:37.203Z] #1       digest: sha256:daeca1c5cee986953da860f13508a4d5a09e682dfa3eb1ed4b760d6f67d307bf
[2019-08-02T18:12:37.203Z] #1         name: "local://dockerfile (Dockerfile)"
[2019-08-02T18:12:37.203Z] #1      started: 2019-08-02 18:12:36.837928994 +0000 UTC
[2019-08-02T18:12:37.203Z] #1    completed: 2019-08-02 18:12:36.837992798 +0000 UTC
[2019-08-02T18:12:37.203Z] #1     duration: 63.804µs
[2019-08-02T18:12:37.203Z] 
[2019-08-02T18:12:37.203Z] 
[2019-08-02T18:12:37.203Z] #2 local://context (.dockerignore)
[2019-08-02T18:12:37.203Z] #2       digest: sha256:014790827f20ad912957874ee677dba625d990516fa8f8f3239792e29e755aa1
[2019-08-02T18:12:37.203Z] #2         name: "local://context (.dockerignore)"
[2019-08-02T18:12:37.203Z] #2      started: 2019-08-02 18:12:36.838123141 +0000 UTC
[2019-08-02T18:12:37.203Z] #2    completed: 2019-08-02 18:12:36.838157924 +0000 UTC
[2019-08-02T18:12:37.203Z] #2     duration: 34.783µs
[2019-08-02T18:12:37.203Z] 
[2019-08-02T18:12:37.203Z] 
[2019-08-02T18:12:37.203Z] #2 local://context (.dockerignore)
[2019-08-02T18:12:37.203Z] #2      started: 2019-08-02 18:12:36.849222305 +0000 UTC
[2019-08-02T18:12:37.203Z] #2    completed: 2019-08-02 18:12:36.885078388 +0000 UTC
[2019-08-02T18:12:37.203Z] #2     duration: 35.856083ms
[2019-08-02T18:12:37.203Z] #2 transferring context: 87B done
[2019-08-02T18:12:37.203Z] 
[2019-08-02T18:12:37.203Z] 
[2019-08-02T18:12:37.203Z] #1 local://dockerfile (Dockerfile)
[2019-08-02T18:12:37.203Z] #1      started: 2019-08-02 18:12:36.849320324 +0000 UTC
[2019-08-02T18:12:37.203Z] #1    completed: 2019-08-02 18:12:36.889461504 +0000 UTC
[2019-08-02T18:12:37.203Z] #1     duration: 40.14118ms
[2019-08-02T18:12:37.203Z] #1 transferring dockerfile: 10.29kB done
[2019-08-02T18:12:37.203Z] 
[2019-08-02T18:12:37.203Z] failed to create LLB definition: invalid reference format
script returned exit code 1

Perhaps the Docker version on those machines is old?

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@seemethere
Copy link
Copy Markdown
Contributor

seemethere commented Aug 2, 2019

@thaJeztah I believe those went out of support before buildkit integration became a real thing.

Last version released for ppc64el and s390x were 18.06.3


'''

sh '''
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thaJeztah not important to change, but was wondering why this was added

Copy link
Copy Markdown
Contributor

@tiborvass tiborvass left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @thaJeztah !! This is great!

Copy link
Copy Markdown
Contributor

@andrewhsu andrewhsu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tiborvass tiborvass merged commit fbc3c06 into moby:master Aug 3, 2019
@thaJeztah thaJeztah deleted the jenkinsfile_cleanup branch August 3, 2019 01:33
@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Aug 3, 2019

Looks like it's not really possible to restart individual (parallel) stages, which is a bit of a pain. Still reading up on what's possible, but linking some issues that are related;

Also looks like the declarative pipelines currently don't support "nested" parallel stages;

  • JENKINS-54010 Support two levels of parallelity in stages
  • JENKINS-41334 Support parallel execution of stages in Declarative
    • fixed this for top-level stages only

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants