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 additional docker flags and remove legacy docker build driver #1999

Merged
merged 5 commits into from
Apr 5, 2022

Conversation

carolynvs
Copy link
Member

@carolynvs carolynvs commented Apr 1, 2022

What does this change

Support additional docker flags

This adds support for the following docker flags:

--build-arg: Pass build arguments that can be used in the template dockerfile
--ssh: Provide a ssh configuration to the container while building
--secret: Provide a secret to the container while building
--no-cache: Build the image and do not use cached layers.

It also fixes how we call the docker buildx plugin so that user configuration, such as a proxy, is used.

After upgrading to a new version of buildx, I was also able to pick up a fix for pretty printing the progress to stderr, while capturing the plaintext output to the logs.

The dockerfile syntax 1.4.0 is officially released, so I've also edited our templates to use the official version # syntax=docker/dockerfile-upstream:1.4.0

Remove experimental flag for build drivers
Default to buildkit only and remove the docker build driver. Keep the build driver config option, in case we ever get creative in the future

What issue does it fix

Closes #1769 (support new buildkit flags)
Closes #1954 (make buildkit the default driver and get rid of legacy docker)
Closes #1941 (load docker config)

Notes for the reviewer

🚨 This must be rebased before merging, it relies on #1998

I kept the configuration infra in place in case we ever add a new build driver in the future. But the flag --driver is now hidden for the build command since there's no reason to set it at the moment. I also kept the experimental config documentation and just let people know it's not needed anymore. I felt that people who were used to the flag would be confused to have its doc disappear without explanation.

Long term maybe we need a page/section of "promoted" experimental features.

Checklist

  • Did you write tests?
  • Did you write documentation?
  • Did you change porter.yaml or a storage document record? Update the corresponding schema file.
  • If this is your first pull request, please add your name to the bottom of our Contributors list. Thank you for making Porter better! 🙇‍♀️

Reviewer Checklist

  • Comment with /azp run test-porter-release if a magefile or build script was modified
  • Comment with /azp run porter-integration if it's a non-trivial PR

@carolynvs
Copy link
Member Author

/azp run porter-integration

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@carolynvs carolynvs force-pushed the buildkit-flags branch 2 times, most recently from f8317c1 to 910e3a1 Compare April 1, 2022 20:10
@carolynvs carolynvs changed the title Support docker buildkit build flags and remove legacy docker build driver Support additional docker flags and remove legacy docker build driver Apr 1, 2022
@carolynvs
Copy link
Member Author

/azp run porter-integration

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@carolynvs
Copy link
Member Author

/azp run porter-integration

@azure-pipelines
Copy link

Pull request contains merge conflicts.

@carolynvs
Copy link
Member Author

/azp run porter-integration

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

This adds support for the following docker flags:

--build-arg: Pass build arguments that can be used in the template
dockerfile
--ssh: Provide a ssh configuration to the container while building
--secret: Provide a secret to the container while building
--no-cache: Build the image and do not use cached layers.

It also fixes how we call the docker buildx plugin so that user
configuration, such as a proxy, is used.

After upgrading to a new version of buildx, I was also able to pick up a
fix for pretty printing the progress to stderr, while capturing the
plaintext output to the logs.

Closes getporter#1769
Closes getporter#1941

Signed-off-by: Carolyn Van Slyck <[email protected]>
Default to buildkit only and remove the docker build driver
Keep the build driver config option, in case we ever get creative in the future

Closes getporter#1954

Signed-off-by: Carolyn Van Slyck <[email protected]>
If a template that was previously written for legacy docker is used now
that we only support buildkit, use dockerfile syntax 1.4.0 and prepend
that to their template so that when we inject things like `COPY --link`
their bundle still builds.

Signed-off-by: Carolyn Van Slyck <[email protected]>
@carolynvs
Copy link
Member Author

/azp run porter-integration

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@carolynvs carolynvs marked this pull request as ready for review April 4, 2022 16:01
@carolynvs carolynvs requested a review from VinozzZ April 4, 2022 16:01
Signed-off-by: Carolyn Van Slyck <[email protected]>
Signed-off-by: Carolyn Van Slyck <[email protected]>
@carolynvs
Copy link
Member Author

/azp run porter-integration

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@vdice vdice left a comment

Choose a reason for hiding this comment

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

So much goodness in this one! The added example is great as well. LGTM.

@carolynvs
Copy link
Member Author

@vdice The example sort of snuck in 😂 It's going to be the source for an upcoming blog post telling people about the new buildkit features. I'll add a follow-up to make it a proper example linked from our examples page too though! https://release-v1.porter.sh/examples.

@carolynvs carolynvs merged commit 4e10c4d into getporter:release/v1 Apr 5, 2022
@github-actions github-actions bot mentioned this pull request Apr 5, 2022
@carolynvs carolynvs deleted the buildkit-flags branch April 5, 2022 18:57
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