-
Notifications
You must be signed in to change notification settings - Fork 74
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
Respect Existing Docker Config Location #218
Comments
This may solve my issue as well: |
Hi @chucknelson and @adespain, Thank you for reporting this issue. I do think this would be worth while to expose a config variable that passes through the vagrantproxy plugin as you suggest and I believe that is supported by docker itself. That I do think is worth while, but the one thing I'm having a hard time trying to grasp is the need to change the standard config? I think it might help if I had some examples to support reasons to add this behavior. I don't think this feature would be a lot of work to implement. The only problem we would have is permissions since there could be sensitive info in the |
Also, the link provided that references
Is for the docker client not the docker daemon. Which is the reason I would like to see some examples for changing the default daemon config from |
Thanks @codylane. Sorry for any confusion, but I think the client config is what this issue and #219 are referencing. It looks like the client Maybe I'm still confusing the various config settings or something? |
Gotcha, no worries and honestly I think this confusion is all faults of my own. I haven't touched this code in a while so I think I might see what the problem is now. However, before I make a speculation of what I think the fix might be. I'm wondering if you can put together a small gist, or screen shot (copy and paste is fine) of ways I can try to reproduce this on my own. I think I might still be a little confused. If you could please provide the following:
I assume this is happening when you have modified or created your own
I'm hoping once I have this information I'll be able to reproduce this on my own without trying to make a bunch of guesses. I realize that what I've asked for might be very time consuming but I do appreciate your time and I know others will too if we can find a solution to this problem. Thanks again for reporting and I do look forward to hearing back from you with the information I've requested. |
I'm affected by this too, so I put together an example as requested. See https://gist.github.com/samdbmg/8b1ed08dd1d81882c12b0c7df50414dc for the Vagrantfile and config files (obviously I've redacted the username and password, although it's Software Versions samn@RD011386$ vagrant --version
Vagrant 2.2.7
samn@RD011386$ vagrant plugin list
vagrant-proxyconf (2.0.7, global)
vagrant-vbguest (0.23.0, global)
samn@RD011386$ VBoxManage --version
6.0.14r133895 Behaviour in Vagrant box vagrant@vagrant:~$ docker login
Login with your Docker ID to push and pull images from Docker Hub. If you don't have a Docker ID, head over to https://hub.docker.com to create one.
Username: ^C
vagrant@vagrant:~$ env | grep DOCKER
DOCKER_CONFIG=/etc/docker
vagrant@vagrant:~$ # That's not what I wanted, lets try and fix it
vagrant@vagrant:~$ unset DOCKER_CONFIG
vagrant@vagrant:~$ docker login
Authenticating with existing credentials...
WARNING! Your password will be stored unencrypted in /home/vagrant/.docker/config.json.
Configure a credential helper to remove this warning. See
https://docs.docker.com/engine/reference/commandline/login/#credentials-store
Login Succeeded
vagrant@vagrant:~$ # That's better My use case is using Vagrant in CI behind a proxy, where we need to log into the Docker registry. Jenkins creates a temporary |
Thanks @samdbmg - thorough example and the scenario of using the default Docker client config location (as opposed to having a set |
Thank you all for the very detailed information. This helps me better understand what I need to do to replicate it on my own, re-read the docs and come up with something. I really appreciate the time spent here and for these really helpful responses. Let's make this thing better together! Anyway, I plan to look at this over the weekend if I have some free time and I hope to respond before Monday with a solution. Thanks again! |
https://elegantinfrastructure.com/docker/ultimate-guide-to-docker-http-proxy-configuration/ Really nice explanation of how docker client vs docker daemon works. As promised I'll spend some more time getting more intimate with the docs to see what I need to fix. Hang tight folks. |
OK, so I have a feeling I added the That said, I don't think the client even cares about the proxy configuration and should instead make only REST API calls to the daemon, which as of the latest release, the daemon is configured to use the proxy. So that all out of the way I think this is what I need to do to fix the problem. Again, I appreciate your help here providing all the examples and helpful explanations of the situation. Test Scenario
|
Having poked around the documentation a bit more, I think the time when the proxy config matters to the client is when building/running a container that wants to access the Internet over a proxy; in my case that might be running To summarise my understanding:
Which makes this a tad harder to solve, as I think you have to write a I'm very happy to test a fix when you've got one and thank you, this plugin is a lifesaver in avoiding a huge tangled mess of scripting in annoying corporate proxy environments |
Thanks @samdbmg, I'll take the information you provided into account as well as I'm going through my testing and refactoring. Thank you so much for taking the time to provide validation that this isn't an easy or straight forward fix. FWIW - I've spent a few hours today trying to get my test environment working again due to upstream bugs preventing me from running my tests on actual VMs. I believe I have a work around in place that is working nicely for now. Once Vagrant releases 2.2.8 (crossing fingers) this bug goes away and I we can get back to normal. Here is the link to the upstream bug that I'm tracking: #209 that is causing me the headache since vagrant 2.2.5 was released. |
Ok, tests are passing as well as my integration tests. Test Scenarios:
All tests scenarios were performed on the docker host, where the docker client also resides and talks directly to docker via The feature basically just comments out the I'd love to have a few of you guys check out this commit and test this on your machines and make sure it works. If you need help figuring out how to bootstrap your environment just let me know. |
I've tested this, and I think it fixes one problem but creates a new gotcha. Docker can pull container images through the proxy because the systemd dropin is there, but because the proxies are no longer set in the client config, their environment variables aren't set inside the container so stuff like I've used the same Vagrantfile as before, this time on my Ubuntu 16.04 desktop which can only access the Internet through a proxy: vagrant@vagrant:~$ env | grep DOCKER_CONFIG
vagrant@vagrant:~$ docker login
Authenticating with existing credentials...
WARNING! Your password will be stored unencrypted in /home/vagrant/.docker/config.json.
Configure a credential helper to remove this warning. See
https://docs.docker.com/engine/reference/commandline/login/#credentials-store
Login Succeeded
vagrant@vagrant:~$ # Yay! Let's try running a container
vagrant@vagrant:~$ docker run -ti ubuntu:18.04
Unable to find image 'ubuntu:18.04' locally
18.04: Pulling from library/ubuntu
23884877105a: Pull complete
bc38caa0f5b9: Pull complete
2910811b6c42: Pull complete
36505266dcc6: Pull complete
Digest: sha256:3235326357dfb65f1781dbc4df3b834546d8bf914e82cce58e6e6b676e23ce8f
Status: Downloaded newer image for ubuntu:18.04
root@45aa068200be:/# apt update
Err:1 http://archive.ubuntu.com/ubuntu bionic InRelease
Could not connect to archive.ubuntu.com:80 (91.189.88.142), connection timed out Could not connect to archive.ubuntu.com:80 (91.189.88.152), connection timed out
Err:2 http://archive.ubuntu.com/ubuntu bionic-updates InRelease
Unable to connect to archive.ubuntu.com:http:
<snip>
W: Failed to fetch http://security.ubuntu.com/ubuntu/dists/bionic-security/InRelease Could not connect to security.ubuntu.com:80 (91.189.91.39), connection timed out Could not connect to security.ubuntu.com:80 (91.189.88.152), connection timed out Could not connect to security.ubuntu.com:80 (91.189.91.38), connection timed out Could not connect to security.ubuntu.com:80 (91.189.88.142), connection timed out
W: Some index files failed to download. They have been ignored, or old ones used instead.
root@45aa068200be:/# # Ugh. Is that proxies?
root@45aa068200be:/# env | grep proxy
root@45aa068200be:/# export http_proxy=http://www-cache.rd.bbc.co.uk:8080
root@45aa068200be:/# export https_proxy=http://www-cache.rd.bbc.co.uk:8080
root@45aa068200be:/# apt update
Get:1 http://archive.ubuntu.com/ubuntu bionic InRelease [242 kB]
Get:2 http://security.ubuntu.com/ubuntu bionic-security InRelease [88.7 kB]
<snip>
Get:17 http://archive.ubuntu.com/ubuntu bionic-backports/universe amd64 Packages [8158 B]
Get:18 http://archive.ubuntu.com/ubuntu bionic-backports/main amd64 Packages [8286 B]
Fetched 17.9 MB in 2s (9685 kB/s)
Reading package lists... Done
Building dependency tree
Reading state information... Done
All packages are up to date.
root@45aa068200be:/# There's an argument that if you supply your own config.json then that's your problem, although I think before you merged proxy settings into any existing file at /etc/docker/config.json instead of overwriting? Could the same approach be used, but instead of /etc/docker/config.json make the file configurable, with a default of /home/vagrant/.docker/config.json? |
@samdbmg - Dude you rock, thanks man! I appreciate your time helping me better understand some of these use cases that I didn't test for yet. This kind of detail helps me out so much! Solution 1Indeed, I honestly was thinking about this a couple days ago of just adding a new config variable here and it would be named I think that could be a good approach to start out with for sure. Potential Solution 2 (more complex)Perhaps we could also have a hierarchy of configuration locations that the
This could also be a configurable option inside the Vagrantfile with the defaults above. If the user supplies their own array of paths to look for then the defaults
will not be merged and the user will be in full control of their destiny. Of course they could also just append to the array as well. Let me know your thoughts and i'll get started hacking away. |
Along with @codylane, yes, thanks so much @samdbmg for all of this detail - sorry I just created the issue and then proceed to not help much 🙁 @codylane - Instead of worrying about a whole variety of possible config locations, we can just follow the way Docker works, right?
Whatever path is used, take whatever Is there something I'm not considering? |
No problem @chucknelson. Thanks for your response and the details. Just confirming this is what the new feature would look like and how it would be configured. FeatureExpose a new environment variable Example
|
+1 from me. @chucknelson? Thoughts? Also, thanks for your patients folks. I promise to get this in quickly. |
@codylane - Sure, +1 from me, let's try it. I'm still a little confused why we need a separate environment variable and/or setting instead of just relying on whatever the guest machine has set for |
Thanks folks, I'll get started on this today. @chucknelson - That is a good point, I don't want to increase complexity either and introduce performance limitations. I have a hunch that it may take longer to configure the plugin if we first have to detect if I'll certainly give it a shot though and see what is the easiest to do and not break something else or create a performance degradation. It might take me a few days to test out these idea(s). |
FYI - Vagrant 2.2.8 was released which finally gives me the ability to integration tests again. We were unable to reliably test vagrant versions 2.2.5 - 2.2.7 without patching parts of vagrant. Yeehaw we are back to normal again. I'll be writing this new feature against vagrant 2.2.8. |
Linking new bug report found on Vagrant 2.2.8. hashicorp/vagrant#11580 |
The vagrant bug has been patched in master, but I still need to work around it until it is officially in a release. That said, it's not holding me up so I've just begun testing this new feature again. I hope to have this done before the end of the week. |
Heck yeah, hashicorp just released Vagrant I'll provide a status tomorrow. Again, I appreciate your patience while this is sorted out. |
Howdy! I've been testing what would be the quickest MVP (with tests) to fix this small bug with the proposed new feature. Additionally this new feature will also try to auto detect if However..... with this great news that things are working we have some downsides: While testing, I confirmed that we have some missing test coverage for how variables are set in the Vagrantfile and the docker_proxy provisioner. While I have we have a working example, I'm not comfortable releasing this without additional tests to ensure I don't break the plugin in a different way. Please bare with me a few more days until I have time to sort this out. |
Just providing a quick status update as it's been a few days..... I'm still ironing out testing problems and ensuring we have proper test coverage as needed. I've had to end up refactoring a large portion of our test suite and I'm hoping all this work will create a more reliable and dependepadle release. What's left you might ask?
I'll create a new task board and link it to this thread so that's easier to see whats going on. FYI - This is my full time job until it's sorted out. Again, I appreciate your time and patience. |
Adding additional tasks to feature board:
2020-05-22 - Quick update. Continued efforts to refactor broken/missing tests is still in progress and getting cleaned up nicely. Finally, a way to do some shared resources in tests and still have clarity (Crossing fingers) when reading and writing new tests. I am also going through each of the tests and ensuring we are testing for the right things and right outcomes. Once the unit tests are completed, I'll move onto the integration tests. It's hard at this point to say how much is left but I'm making good progress. If I had to guess it will be another week or so. 2020-05-27 - UPDATE
Hang tight folks, good things to come for sure. 2020-06-03 - UPDATE
|
@codylane - Not exactly related to this issue, but related to the latest Vagrant versions Do you know if any of the changes made in Vagrant Example stack trace:
|
Hey @chucknelson, Ugh, I'm sorry to hear that you are experiencing some weird behaviors with the newer versions of vagrant and this plugin. I won't lie, I'm also having a hard time trying to determine what the heck has changed since Vagrant 2.2.4, and as far as I'm concerned that is the latest stable release that continues to work for me, at least in my environments at home. Before I point the finger though, I'd like to make sure there isn't something wonky that I created by accident. Give me a few days to investigate. Also for what it's worth vagrant 2.2.8 was released and then patched with at least 3 criticals from what I could see and released with 2.2.9 in less then a week. Mind installing vagrant 2.2.9? I have a feeling this problem will still remain but I'd love to know either way. Thanks again for reporting your findings. Hopefully in the next couple of weeks all this will behind us and we will get back to normal behaviors. I've made some pretty good progress today but still not worthy of posting on GH yet. |
Upon closer inspection of the log that you posted which is awesome by the way, the problem appears to be related to how the tempfile implementation works. I'll need to investigate how this is supposed to work on Windows. It looks as though you are using Windows? if so, can you please tell me which version? I'd like to setup a test environment on my own with the same version and see if I can find a solution. That said, this seems like a different issue. I'll create a new issue and link it to this one. Just for convenience. |
FYI - I'm putting this into a stalled state for as I have not had the time to work on this as much as I'd like and I wanted to provide a status update as to the delays. I promise this will get done, I just need to focus on my some more important priorities at the moment. If nothing else, I'll check in the current state of my branch into this repo: https://github.com/codylane/vagrant-proxyconf by the end of the week. |
As promised I've pushed the stalled code to my local github branch. |
Howdy and happy friday at least on my side of the world. I still apologize for the long delay on getting back to this. I woke up this morning and was on mission to fix broken things so I decided to make an attempt to fix this issue. I have got this staged in my personal repo and it's currently going through travis tests. If they pass, I'll open a pull request here and at least get this into the master branch. @chucknelson - If you have time, once this merged to tmatilai/vagrant-proxyconf I'd welcome some feedback and testing if you have the time? I'll let you know when it's ready for testing. |
When setting the docker proxy config, the plugin seems to force a docker config location of
/etc/docker
, even changing it to that if it's already configured to some other location.Not sure if there are any other impacts, but would it be possible to look for an existing setting per
DOCKER_CONFIG
before defaulting to/etc/docker
?The text was updated successfully, but these errors were encountered: