Skip to content

Remote docker host#1228

Merged
nlopezgi merged 90 commits intobazelbuild:masterfrom
bajacondor:remote-docker-host
Jan 14, 2020
Merged

Remote docker host#1228
nlopezgi merged 90 commits intobazelbuild:masterfrom
bajacondor:remote-docker-host

Conversation

@bajacondor
Copy link
Contributor

@bajacondor bajacondor commented Oct 18, 2019

This allows flags like -H=some.other.docker.host:2375 to be set with docker_path in docker toolchain_configure() so that rules_docker works with a remote docker host.

Related issues:
#1207

Regarding the CLA for d****@nordstrom.com:
I attest that I approve the CLA for both my personal email and the above mentioned nordstrom.com email. Both addresses are mine.

Thank you for your consideration

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@bajacondor
Copy link
Contributor Author

@googlebot I fixed it.

@bajacondor
Copy link
Contributor Author

Can someone tell me which email address is causing the cla:no flag? All my commits have the email address that has an individual cla attached. I'm not sure how to fix this.

Thank you.
David

@smukherj1
Copy link
Collaborator

smukherj1 commented Oct 18, 2019

Hi @bajacondor,

Thanks for the PR. Regarding the CLA, the bot found the following commit authors:

  1. ***@nordstrom.com (No CLA)
  2. ***@gmail.com (Has CLA)

(edited to redact part of the email address)

@bajacondor
Copy link
Contributor Author

@smukherj1,
That is so annoying. Here is my situation:
Both the gmail and the nordstrom addresses are mine. I have a personal cla for the gmail address. The nordstrom address is tied to another google account so I can't add it to my personal alternate emails. When I attempt to create a CLA for the nordstrom account, I get directed and can't sign in. Who can I speak to about this? I tried rewriting the history to change email addresses, and somehow the bot still knows about the n.com address!

@nlopezgi
Copy link
Contributor

@bajacondor we still have one test failing with message:

INFO: Build completed successfully, 1 total action
+ ./bazel-bin///testdata/workdir_with_tar_base
./testing/e2e/bazel_run_docker.sh: line 95: ./bazel-bin///testdata/workdir_with_tar_base: No such file or directory

corresponding to an error in this line

@bajacondor
Copy link
Contributor Author

Hi @nlopezgi, Thanks for all your help with this. I'm curious why it's on hold to merge?

@k8s-ci-robot k8s-ci-robot removed the lgtm label Jan 13, 2020
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@nlopezgi
Copy link
Contributor

/gcbrun

@nlopezgi
Copy link
Contributor

Hi @nlopezgi, Thanks for all your help with this. I'm curious why it's on hold to merge?

Because some tests have failed

@bajacondor
Copy link
Contributor Author

Because some tests have failed

Ah, I'm unable to see the cloudbuild test results. Is there anything I can do?

@nlopezgi
Copy link
Contributor

Because some tests have failed

Ah, I'm unable to see the cloudbuild test results. Is there anything I can do?

Did you fix the one I commented above?
testing/e2e/bazel_run_docker.sh: line 95: ./bazel-bin///testdata/workdir_with_tar_base

@bajacondor
Copy link
Contributor Author

Did you fix the one I commented above?

Ah, no. I misunderstood that function and the loop. I think I fixed it with this commit.

@nlopezgi
Copy link
Contributor

/gcbrun

@bajacondor
Copy link
Contributor Author

Looks like one more failing on Cloud Build. Is there a way to get access to these test results for my google user?

@nlopezgi nlopezgi added cla: yes and removed cla: no labels Jan 14, 2020
@googlebot
Copy link

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

ℹ️ Googlers: Go here for more info.

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bajacondor, nlopezgi

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

The pull request process is described here

Details 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

@nlopezgi
Copy link
Contributor

Thanks a lot for all the work on this PR!

@nlopezgi nlopezgi merged commit fa5f719 into bazelbuild:master Jan 14, 2020
emranbm added a commit to emranbm/rules_docker that referenced this pull request Jan 15, 2020
@smparkes
Copy link

So I've been exploring docker_flags because when run in CI/CD, we use a remote docker.

We need host, tls, and the various PKI files.

The values are provided through env variables but getting those into bazel is tricky.

Critically, pretty much all the ways of getting the values into bazel are going to bust the cache if they change, which they generally will for us on every run. This is a show stopper.

I can see a couple of ways forward and was wondering what people think.

They both revolve around putting the necessary info in $HOME/.docker. Unfortunately, these values aren't part of config.json. If you use --tls, docker will look here for the PKI files but it doesn't use that directory for host, tls or tlsverify.

So I'm thinking, outside of bazel we could copy the env variables into that directory and then tweak the rules_docker scripts to use those values if those files exist. It's painful and has problems with shared home directories if people were trying to talk to multiple remote dockers but that isn't immediately a blocker for us. It's moderately straightforward.

Alternatively, rather than tweaking the rules_docker scripts, we can create a wrapper around the docker client that use the values to set flags/env variables. The painful part there is having to set an absolute path in the toolchain that must be the same on every host we ever run this on.

Just wondering if I'm missing anything or what people thought in general.

Also, FWIW, I think this line in the document might be a little misleading/confusing:

When left unspecified (ie not set explicitly or set by the docker toolchain), docker will use the directory specified via the DOCKER_CONFIG environment variable. If DOCKER_CONFIG isn't set, docker falls back to $HOME/.docker.

Without an --action_env setting, I don't think DOCKER_CONFIG will be propagated from the shell invoking bazel to the docker client so won't have any impact (it'll still look in $HOME/.docker.)

@Jonpez2
Copy link

Jonpez2 commented Jun 8, 2021

Hello! I've been trying to use docker_flags with container_test as well - is there a particular reason why the flags aren't piped through from the toolchain to the docker call in container_test? That seems like the right thing to do, no?

(Obviously there's significant chance that I've missed something obvious in which case please forgive and educate me)

Thank you!

OK sorry I did miss something obvious: containe-structure-test appears to have no mechanism for passing args to the docker call. I'll leave this comment here to educate anyone else who is also confused.

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.

10 participants