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

buildkit: enable net modes and bridge #37620

Merged
merged 5 commits into from
Aug 20, 2018

Conversation

tonistiigi
Copy link
Member

@tonistiigi tonistiigi commented Aug 9, 2018

Changes the default network used by buildkit to libnetwork bridge and enables --network=host and --network=none. Frontends have the capability to set networking modes per-command.

depends on moby/buildkit#560

@fcrisciani @abhi

EDIT: Related to moby/buildkit#583 and #37676

"github.com/moby/buildkit/util/tracing"
"github.com/pkg/errors"
"golang.org/x/sync/errgroup"
grpcmetadata "google.golang.org/grpc/metadata"
)

func init() {
llbsolver.AllowNetworkHostUnstable = true
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a temporary variable that needs to be set globally because buildkit by default forbids host network entitlement on daemon level.

Copy link
Contributor

@abhi abhi left a comment

Choose a reason for hiding this comment

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

LGTM. I will give the patch a try for better understanding.

@codecov
Copy link

codecov bot commented Aug 10, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@9916827). Click here to learn what that means.
The diff coverage is 0%.

@@            Coverage Diff            @@
##             master   #37620   +/-   ##
=========================================
  Coverage          ?   35.16%           
=========================================
  Files             ?      611           
  Lines             ?    45817           
  Branches          ?        0           
=========================================
  Hits              ?    16112           
  Misses            ?    27485           
  Partials          ?     2220

@tonistiigi
Copy link
Member Author

Forgot to add --add-host support as well. Added a commit.

@thaJeztah
Copy link
Member

This test is failing on all CI, so may be related;

09:46:47 --- FAIL: TestContainerStartOnDaemonRestart (2.33s)
09:46:47 	daemon.go:289: [d887a474afedf] waiting for daemon to start
09:46:47 	daemon.go:321: [d887a474afedf] daemon started
09:46:47 	daemon.go:289: [d887a474afedf] waiting for daemon to start
09:46:47 	daemon.go:321: [d887a474afedf] daemon started
09:46:47 	daemon_linux_test.go:65: assertion failed: error is not nil: Error response from daemon: mkdir /var/run/docker/containerd/daemon/io.containerd.runtime.v1.linux/moby/5f3a5f580ed0f8999f27b9a7d9f33f251ec812ced9d0998744f104995c20464d: file exists: already exists: failed to start test container
09:46:47 	daemon.go:279: [d887a474afedf] exiting daemon

vendor.conf Outdated
@@ -26,7 +26,7 @@ github.com/imdario/mergo v0.3.6
golang.org/x/sync 1d60e4601c6fd243af51cc01ddf169918a5407ca

# buildkit
github.com/moby/buildkit e57eed420c7573ae44875be98fa877175b4677a1
github.com/moby/buildkit 95e0348f571a0e500a9b6911992c92f877b44ff1 git://github.com/tonistiigi/buildkit.git
Copy link
Member

Choose a reason for hiding this comment

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

moby/buildkit#560 was merged; so this can likely be "un-forked"

Copy link
Contributor

Choose a reason for hiding this comment

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

@thaJeztah yes, I'll update as soon I fixed the failure hopefully with @abhi's help

@thaJeztah thaJeztah added the status/failing-ci Indicates that the PR in its current state fails the test suite label Aug 19, 2018
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tibor Vass <tibor@docker.com>
tonistiigi and others added 4 commits August 20, 2018 18:55
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Signed-off-by: Tibor Vass <tibor@docker.com>
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@tiborvass tiborvass left a comment

Choose a reason for hiding this comment

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

LGTM

@tiborvass tiborvass merged commit cf72051 into moby:master Aug 20, 2018
@thaJeztah thaJeztah removed the status/failing-ci Indicates that the PR in its current state fails the test suite label Dec 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants