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

Support PRs to use PR ID in the docker images pushed to our docker registry #20317

Closed
mdelapenya opened this issue Jul 29, 2020 · 4 comments · Fixed by #20323
Closed

Support PRs to use PR ID in the docker images pushed to our docker registry #20317

mdelapenya opened this issue Jul 29, 2020 · 4 comments · Fixed by #20323
Assignees
Labels
Team:Automation Label for the Observability productivity team

Comments

@mdelapenya
Copy link
Contributor

When packaging a PR, the generated docker image is pushed to our docker registry with the name of the snapshot. We'd like to generate the docker image including the PR ID:

- def newName = "${DOCKER_REGISTRY}/observability-ci/${name}:${libbetaVer}"
- def commitName = "${DOCKER_REGISTRY}/observability-ci/${name}:${env.GIT_BASE_COMMIT}"
+ def newName = "${DOCKER_REGISTRY}/observability-ci/${name}:${PR_ID}-${libbetaVer}"
+ def commitName = "${DOCKER_REGISTRY}/observability-ci/${name}:${PR_ID}-${env.GIT_BASE_COMMIT}"

cc/ @elastic/observablt-robots @ph @EricDavisX

@mdelapenya mdelapenya added the Team:Automation Label for the Observability productivity team label Jul 29, 2020
@mdelapenya mdelapenya self-assigned this Jul 29, 2020
@mdelapenya
Copy link
Contributor Author

I'm thinking about the proposed name, and I could see two different choices:

  1. def newName = "${DOCKER_REGISTRY}/observability-ci/${name}:${libbetaVer}"
  2. def newName = "${DOCKER_REGISTRY}/observability-ci/${name}/${PR_ID}:${libbetaVer}"

The first one adds the PR-ID to the tag, and the second one adds it to the namespace. I like the second one as it preserves the tag name structure, so it's easier to identify the tag.

Which one do you prefer?

@ph
Copy link
Contributor

ph commented Jul 29, 2020

I like option 2.

@mdelapenya
Copy link
Contributor Author

Adding @kuisathaverat's comments for context: #20323 (comment)

@mdelapenya
Copy link
Contributor Author

What he suggests is a third option:

  1. def newName = "${DOCKER_REGISTRY}/observability-ci/${name}:${PR_ID}

I like this option the most

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Automation Label for the Observability productivity team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants