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

breaking!(buildDockerAndPublishImage): support multi-platform with Docker Bake for building Linux container images #730

Merged
merged 70 commits into from
Aug 31, 2023

Conversation

smerle33
Copy link
Contributor

@smerle33 smerle33 commented Aug 17, 2023

as per jenkins-infra/helpdesk#3619
provide the ability to build docker images for multi platforms as linux/arm64, linux/amd64 etc ...

this use the bake mode to directly build multiplatform.
It also convert "normal" one platform build for linux as docker bake build in buildx engine.

BREAKING CHANGES :

  • ⚠️ the makefile parameter IMAGE_PLATFORM change forBUILD_TARGETPLATFORM
  • the pipeline library parameter platform change for targetplatforms (the deprecated parameter is still consume for now)

@smerle33
Copy link
Contributor Author

sounds good :

@Library('pipeline-library@pull/730/head') _
parallelDockerUpdatecli([imageName: 'wiki', rebuildImageOnPeriodicJob: false, containerMemory: '1G', buildDockerConfig : [platform: 'linux/amd64,linux/arm64,linux/s390x']])
Capture d’écran 2023-08-23 à 17 32 50

@smerle33 smerle33 marked this pull request as ready for review August 23, 2023 15:49
@smerle33 smerle33 requested a review from a team August 23, 2023 15:49
@smerle33 smerle33 marked this pull request as draft August 25, 2023 15:36
smerle33 added a commit to smerle33/docker-inbound-agents that referenced this pull request Aug 29, 2023
@smerle33 smerle33 marked this pull request as ready for review August 29, 2023 15:22
@smerle33
Copy link
Contributor Author

last successful build for wiki with this PR : https://infra.ci.jenkins.io/job/docker-jobs/job/docker-confluence-data/job/PR-48/63/pipeline-graph/
and last successful build for docker-inbound-agent with the legacy path (make build not bake-build) as those are windows images : https://infra.ci.jenkins.io/job/docker-jobs/job/docker-inbound-agents/job/PR-101/1/pipeline-graph/

@dduportal dduportal self-assigned this Aug 30, 2023
Copy link
Contributor

@dduportal dduportal left a comment

Choose a reason for hiding this comment

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

First pass of review:

  • You have to update the documentation of the method (both README.md and the txt file)
  • You have to update the PR title and body
  • There are missing unit tests for some if / WARNING
  • A few code discussion (removal of untested or untestable code, a few suggestions on comments, code style and naming conventions)

It is a great job, the code looks really nice! 👏

The end to end tests are good and shows that the intent work!

resources/io/jenkins/infra/docker/Makefile Outdated Show resolved Hide resolved
resources/io/jenkins/infra/docker/Makefile Outdated Show resolved Hide resolved
resources/io/jenkins/infra/docker/Makefile Outdated Show resolved Hide resolved
resources/io/jenkins/infra/docker/Makefile Outdated Show resolved Hide resolved
resources/io/jenkins/infra/docker/Makefile Outdated Show resolved Hide resolved
vars/buildDockerAndPublishImage.groovy Outdated Show resolved Hide resolved
vars/buildDockerAndPublishImage.groovy Outdated Show resolved Hide resolved
vars/buildDockerAndPublishImage.groovy Outdated Show resolved Hide resolved
vars/buildDockerAndPublishImage.groovy Show resolved Hide resolved
vars/buildDockerAndPublishImage.groovy Show resolved Hide resolved
@smerle33 smerle33 changed the title feat(docker): Multiplatform build using docker bake breaking!(buildDockerAndPublishImage): support multi-platform with Docker Bake for building Linux container images Aug 30, 2023
Copy link
Contributor

@dduportal dduportal left a comment

Choose a reason for hiding this comment

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

One last thing to fix (multiple docker buildx create --use instructions), the rest is LGTM once it is fixed

Copy link
Contributor

@dduportal dduportal left a comment

Choose a reason for hiding this comment

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

That looks good to me 👏 Let's roll!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants