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

Extract Base Image for V4 #551

Merged
merged 24 commits into from
Nov 18, 2021
Merged

Extract Base Image for V4 #551

merged 24 commits into from
Nov 18, 2021

Conversation

CooperLink
Copy link
Collaborator

PR information

Continuation of #516 which is an effort to have all language images rely on a base host image. This effort will simplify updates to the docker images used in production and should reduce the need for large and difficult to parse PRs in this repo.

This PR specifically enables dotnet images and brings the changes verified in v3 over to v4. I also updated the release pipeline to upload a zip file of the dockerfiles generated during a release so that we can trace the contents of each image in prod.

  • The title of the PR is clear and informative.
  • There are a small number of commits, each of which has an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
  • If applicable, the PR references the bug/issue that it fixes in the description.
  • New Unit tests were added for the changes made and CI is passing.

@pragnagopa
Copy link
Member

V4 pipeline builds failed - @CooperLink - please take a look

Copy link
Collaborator

@TsuyoshiUshio TsuyoshiUshio left a comment

Choose a reason for hiding this comment

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

I leave the comments, if you don't agree with me, I'd happy to approve. It is great work! Thank you Cooper!

host/3.0/dotnet-build.yml Show resolved Hide resolved
@@ -30,6 +32,12 @@ steps:
continueOnError: false
env:
pswd: $(dockerPassword)

- bash: |
./host/generate-composite.sh -4 node
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about adding PublishArtifact task for pushing the generated Dockerfile for debugging?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a great idea. I am just not sure how we would publish that artifact or where it should go. The build pipelines should tell us if any images failed building. But, it would be nice to have slightly higher traceability if there was some way to surface that artifact on the pipeline run of build that happens.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is great Tsuyoshi. I think that I can and should use this to produce artifacts during build pipelines for each language. I have captured your comment in an issue on the repo here.
#560

Feels like something that should be addressed in a follow up pr but I agree that it is useful

Copy link
Collaborator

@TsuyoshiUshio TsuyoshiUshio left a comment

Choose a reason for hiding this comment

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

LGTM

host/generate-composite.sh Outdated Show resolved Hide resolved
@CooperLink CooperLink merged commit 485c0ef into dev Nov 18, 2021
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.

5 participants