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.

Sign up for GitHub

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

Jump to bottom

Add support for Buildkit #675

Merged
merged 2 commits into from
Apr 5, 2021
Merged

Add support for Buildkit #675

merged 2 commits into from
Apr 5, 2021

Conversation

qu1queee
Copy link
Contributor

@qu1queee qu1queee commented Mar 17, 2021 •
edited
Loading

Changes

Fixes #407

This adds support for rootless buildkit.

Some of the key things from using buildkit are:

  • The cache on subsequent pushes to the same image in a registry, also known as export_cache and import_cache makes the building experience incredible fast.
  • It runs as rootless, without any privileges.
  • It have a better way to handle where the source code is located and where is the dockerfile located (e.g. compared to Kaniko), however we will need to extend our strategy params to support a new one. At the moment, some of the configs for running it are not configurable, but with the upcoming params support, this will change. I documented this limitations in the related document.
  • Latest Docker versions come with the option to opt-in for a backend Buildkit, therefore its very popular and we should provide support for it.

This adds an insecure strategy, so that we can push to the insecure local in-cluster HTTP registry in e2e tests, otherwise it will not work with a secure one.

Submitter Checklist

  • Includes tests if functionality changed/was added
  • Includes docs if changes are user-facing
  • Set a kind label on this PR
  • Release notes block has been filled in, or marked NONE

Release Notes

Provide support for rootless Buildkit as a new Build Strategy.

@qu1queee qu1queee added kind/feature Categorizes issue or PR as related to a new feature. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Mar 17, 2021
@openshift-ci-robot openshift-ci-robot added release-note Label for when a PR has specified a release note and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Mar 17, 2021
@sbose78
Copy link
Member

sbose78 commented Mar 17, 2021

Could you please update the readme? ( "Samples" section, not the "roadmap". We should probably remove the latter).

@qu1queee
Copy link
Contributor Author

@sbose78 yes, this PR still needs more work, mainly docs. Im gonna wait some more time for feedback, then I will update as you requested.

@qu1queee qu1queee added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 19, 2021
@qu1queee qu1queee changed the title Add support for Buildkit WIP: Add support for Buildkit Mar 22, 2021
@qu1queee qu1queee force-pushed the qu1queee/support_buildkit_strategy branch 4 times, most recently from e19d94c to 8674b99 Compare April 1, 2021 09:19
@qu1queee qu1queee changed the title WIP: Add support for Buildkit Add support for Buildkit Apr 1, 2021
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 1, 2021
@qu1queee qu1queee force-pushed the qu1queee/support_buildkit_strategy branch from 8674b99 to 1033dea Compare April 1, 2021 09:24
@qu1queee qu1queee mentioned this pull request Apr 1, 2021
Closed
SaschaSchwarze0
SaschaSchwarze0 requested changes Apr 1, 2021
View reviewed changes
Copy link
Member

@SaschaSchwarze0 SaschaSchwarze0 left a comment

Choose a reason for hiding this comment

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

Nice. Please add an e2e test case for completeness. Adding some suggestions.

samples/buildstrategy/buildkit/buildstrategy_buildkit_cr.yaml Outdated Show resolved Hide resolved
samples/buildstrategy/buildkit/buildstrategy_buildkit_cr.yaml Show resolved Hide resolved
samples/buildstrategy/buildkit/buildstrategy_buildkit_cr.yaml Show resolved Hide resolved
docs/buildstrategies.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
docs/tutorials/README.md Outdated Show resolved Hide resolved
docs/tutorials/README.md Outdated Show resolved Hide resolved
docs/tutorials/building_with_buildkit.md Outdated Show resolved Hide resolved
docs/tutorials/building_with_buildkit.md Outdated Show resolved Hide resolved
docs/buildstrategies.md Show resolved Hide resolved
@qu1queee qu1queee force-pushed the qu1queee/support_buildkit_strategy branch from 1033dea to a62c6e0 Compare April 1, 2021 14:27
@qu1queee qu1queee force-pushed the qu1queee/support_buildkit_strategy branch from a62c6e0 to 22995ef Compare April 1, 2021 14:41
@qu1queee
Add support for buildkit
d803354 
This is a daemonless flavor. It can run as rootless with some specific
configurations.
@qu1queee qu1queee force-pushed the qu1queee/support_buildkit_strategy branch 2 times, most recently from eeb9b06 to 2ac15ea Compare April 3, 2021 18:18
SaschaSchwarze0
SaschaSchwarze0 requested changes Apr 4, 2021
View reviewed changes
Copy link
Member

@SaschaSchwarze0 SaschaSchwarze0 left a comment

Choose a reason for hiding this comment

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

Looks good. Stumbled over one more typo.

docs/tutorials/README.md Outdated Show resolved Hide resolved
samples/buildstrategy/buildkit/buildstrategy_buildkit_cr.yaml Show resolved Hide resolved
@qu1queee
Enhance docs with Buildkit
bf03fa9 
Document usage of BuildKit and update all related docs
@qu1queee qu1queee force-pushed the qu1queee/support_buildkit_strategy branch from 2ac15ea to bf03fa9 Compare April 4, 2021 18:25
SaschaSchwarze0
SaschaSchwarze0 approved these changes Apr 5, 2021
View reviewed changes
Copy link
Member

@SaschaSchwarze0 SaschaSchwarze0 left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 5, 2021
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SaschaSchwarze0

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 5, 2021
@openshift-merge-robot openshift-merge-robot merged commit 81514b3 into shipwright-io:master Apr 5, 2021
@qu1queee qu1queee deleted the qu1queee/support_buildkit_strategy branch April 5, 2021 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SaschaSchwarze0 SaschaSchwarze0 SaschaSchwarze0 approved these changes

@xiujuan95 xiujuan95 Awaiting requested review from xiujuan95

Assignees

@SaschaSchwarze0 SaschaSchwarze0

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. release-note Label for when a PR has specified a release note
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for buildkit
5 participants
@qu1queee @sbose78 @openshift-ci-robot @SaschaSchwarze0 @openshift-merge-robot