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

feat(buildDockerAndPublishImage): allow to specify a target when using docker bake #822

Conversation

lemeurherve
Copy link
Member

@lemeurherve lemeurherve commented Jan 10, 2024

This PR adds a dockerBakeTarget parameter to the buildDockerAndPublishImage function to allow specifying a target when using docker bake.

This PR also fixes the fact that before this change when using docker bake, cst tests are executed only on the image with the latest tag instead of testing all* built targets.

Ref:

*: with a platform corresponding to the architecture of the machine executing the tests.

@lemeurherve lemeurherve force-pushed the helpdesk3887-feat-build-bake-target branch from e99e4aa to 6be0c7c Compare January 10, 2024 22:08
lemeurherve added a commit to jenkins-infra/docker-jenkins-weekly that referenced this pull request Jan 10, 2024
@lemeurherve
Copy link
Member Author

lemeurherve commented Jan 10, 2024

@lemeurherve lemeurherve marked this pull request as ready for review January 10, 2024 22:17
@lemeurherve lemeurherve requested a review from a team as a code owner January 10, 2024 22:18
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.

(edited after team discussion)

This PR also fixes the fact that before this change when using docker bake, cst tests are executed only on the image with the latest tag instead of testing all* built targets.

This is a distinct change which need to be discussed as there were reason for not doing it.

Can you remove it from this PR please as it is a blocker from my point of view.


After discussion:

  • I misunderstood: the changes on "tests" in this PR not only are required, but are also only scoped to choose the proper local image to build. The PR body explains it carefully, I misread the asterisk, nevermind my remark
  • Although we want to remove the naming constraint "image tag" <-> "target" (ref. Test jenkins-infra/pipeline-library#822 docker-jenkins-weekly#1467)

@dduportal dduportal dismissed their stale review January 11, 2024 08:53

self-dismiss

lemeurherve added a commit to jenkins-infra/docker-jenkins-weekly that referenced this pull request Jan 11, 2024
@lemeurherve
Copy link
Member Author

Addressed in jenkins-infra/docker-jenkins-weekly@814c2a0 by retrieving the tag of the selected target(s), with the / character replaced by # so it can be used as Makefile step name.

Tested successfully in jenkins-infra/docker-jenkins-weekly#1467 from where I removed the tags modifications to keep only the test of this PR:

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.

LGTM!

@lemeurherve lemeurherve merged commit 61da11f into jenkins-infra:master Jan 11, 2024
4 checks passed
@lemeurherve lemeurherve deleted the helpdesk3887-feat-build-bake-target branch January 11, 2024 12:52
smerle33 pushed a commit to smerle33/pipeline-library that referenced this pull request Jan 16, 2024
…g docker bake (jenkins-infra#822)

* feat(buildDockerAndPublishImage): allow to specify a target when using docker bake

* allow cst to test a specific tag of an image

* allow cst to test a specific tag of an image

* Update vars/buildDockerAndPublishImage.txt

Co-authored-by: Damien Duportal <damien.duportal@gmail.com>

---------

Co-authored-by: Damien Duportal <damien.duportal@gmail.com>
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.

None yet

2 participants