-
Notifications
You must be signed in to change notification settings - Fork 70
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: Allow copa to use buildkit from dockerd #233
feat: Allow copa to use buildkit from dockerd #233
Conversation
0354f5d
to
74a89d7
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #233 +/- ##
==========================================
- Coverage 34.72% 28.51% -6.22%
==========================================
Files 12 16 +4
Lines 1146 1487 +341
==========================================
+ Hits 398 424 +26
- Misses 727 1037 +310
- Partials 21 26 +5
☔ View full report in Codecov by Sentry. |
@cpuguy83 should we remove the buildkit instance for some of the e2e tests or have a matrix of them (one using docker builtin and another one using standalone buildkit)? copacetic/.github/workflows/build.yml Line 142 in 3e22d49
|
f4e8404
to
f47fa29
Compare
I updated this with to fixes issues. |
@sozercan Sounds good. I'll have to take a look at how e2e is setup tomorrow. |
No review comments this is just super cool that's all |
0bdc8b7
to
dbb0c56
Compare
Pushed updates for adding a test matrix. |
4126ebb
to
8d1ad0f
Compare
3167bc9
to
da12dca
Compare
Finally have CI working the way I'd like it to. Moved the patch tests into go integration tests. codecov is not happy. |
@@ -0,0 +1,5 @@ | |||
#!/usr/bin/env sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to align on sh or bash?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's nothing bashy-in here, really the line is to make my editor load the right toolset.
Otherwise totally fine to throw either sh or bash in there.
- Adds all the built-in connection helpers from upstream buildkit (e.g. `docker-container://`, `ssh://`, `kubepod://` - Add connection helpers for: - dockerd's built-in buildkit (`docker://`) - buildx configured buildkit instances (`buildx://`) - When addr is unset try to find a suitable buildkit instance - Priority: docker -> buildx -> default buildkit addr For the buildx support, currently only buildx instances over docker-containers are supported. Support other buildx drivers is possible (e.g. the kubernetes driver), but this is a lot more effort that can come later. Signed-off-by: Brian Goff <[email protected]>
This is just a way to test all the connection helpers using the existing test harness. Ideally the patch tests are refactored into use go and we can more easily manage mutliple parallel environments from one runner. Signed-off-by: Brian Goff <[email protected]>
e307f53
to
1162108
Compare
Co-authored-by: Sertaç Özercan <[email protected]> Signed-off-by: Brian Goff <[email protected]>
1162108
to
57c66e8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! LGTM
Co-authored-by: Sertaç Özercan <[email protected]>
This adds support for connection drivers for connecting to buildkit.
unix://
addrs are for connecting directly to buildkitd as beforedocker://
will connect to docker's built-in buildkit instancedocker-container://
takes a container name and proxies the buildkit socket usingbuildctl
in the container (similar to buildx)buildx://
will read a specific buildx instance config and try to connect to that (defaults to the currently selected buildx instance) - currently only supports docker-container backed buildx instancesFixes #177