Added support for chef-provisioning 1.0 #47
Conversation
Signed-off-by: Maksim Chizhov <maksim.chizhov@gmail.com>
Signed-off-by: Maksim Chizhov <maksim.chizhov@gmail.com>
Addresses - chef-boneyard#37 - chef-boneyard#24 Signed-off-by: Maksim Chizhov <maksim.chizhov@gmail.com>
| # FIXME: make a proper fix | ||
| if /^chefzero:\/\/localhost/ =~ url | ||
| url.gsub!(/^chefzero/, 'http') | ||
| end |
There was a problem hiding this comment.
How about:
uri = URI(url)
uri.scheme = 'http'instead?
ex:
irb(main):001:0> require 'URI'
=> true
irb(main):002:0> require 'socket'
=> true
irb(main):003:0> url = 'chefzero://localhost'
=> "chefzero://localhost"
irb(main):004:0> uri = URI(url)
=> #<URI::Generic chefzero://localhost>
irb(main):005:0> uri.scheme = 'http' unless %w(http https).include?(uri.scheme)
=> "http"
irb(main):006:0> host = Socket.getaddrinfo(uri.host, uri.scheme, nil, :STREAM)[0][3]
=> "::1"There was a problem hiding this comment.
This could be remote chef-server with https also. Though, I think fix must be made somewhere else, probably on chef-provisioning side. That's why I consider code above as temporal workaround.
There was a problem hiding this comment.
Makes sense now. Thank you for reviewing that.
There was a problem hiding this comment.
maybe it's even better to limit the explicit setting to chefzero:
uri.scheme = 'http' if uri.scheme == 'chefzero' Signed-off-by: Maksim Chizhov <maksim.chizhov@gmail.com>
|
I used this as an opportunity to write #51, and while the latter doesn't cover everything here, I feel pretty good about it. 👍 |
| @@ -179,11 +179,12 @@ def upload_file(local_path, path) | |||
| def make_url_available_to_remote(url) | |||
| # The host is already open to the container. Just find out its address and return it! | |||
| uri = URI(url) | |||
| uri.scheme = 'http' if 'chefzero' == uri.scheme && uri.host == 'localhost' | |||
| host = Socket.getaddrinfo(uri.host, uri.scheme, nil, :STREAM)[0][3] | |||
| Chef::Log.debug("Making URL available: #{host}") | |||
|
|
|||
| if host == '127.0.0.1' || host == '::1' | |||
There was a problem hiding this comment.
What about host == 'localhost'?
There was a problem hiding this comment.
It was so for a long time and no bugs were filed.
There was a problem hiding this comment.
@tyler-ball getaddrinfo()[0][3] is the IP address. http://ruby-doc.org/stdlib-1.9.3/libdoc/socket/rdoc/Socket.html#method-c-getaddrinfo
|
This looks good to me (other than a few minor comments). Because I'm so unfamiliar with Docker I am interested in adding good testing to make sure this works. But that won't be a blocker to getting this merged! |
| @@ -179,11 +179,12 @@ def upload_file(local_path, path) | |||
| def make_url_available_to_remote(url) | |||
| # The host is already open to the container. Just find out its address and return it! | |||
| uri = URI(url) | |||
| uri.scheme = 'http' if 'chefzero' == uri.scheme && uri.host == 'localhost' | |||
There was a problem hiding this comment.
I believe this is now fixed in chef-provisioning 1.2
There was a problem hiding this comment.
Didn't work for me with chef-provisioning 1.2 and this line removed. url always comes as chefzero://localhost:8889 if chef zero used as provisioner. Just in case gem list:
chef (12.3.0)
chef-provisioning (1.2.0)
chef-zero (4.2.1)
cheffish (1.2)
There was a problem hiding this comment.
Yes, we're pretty sure this is necessary.
There was a problem hiding this comment.
@mwrock the fix we added in chef-boneyard/chef-provisioning#337 that was released in chef-provisioning 1.2 applies to the target (guest, VM, etc.) node. It means that chef-client running on that node will be able to talk to the chef-zero server running on the workstation.
This change is different but related. It makes sure the chef-zero server running on the workstation has its port forwarded to the container.
Having said this, if the user wants to run chef-client version 12.3.x or greater on their container they will need to use chef-provisioning ~> 1.2, or use some of the workarounds detailed in chef-boneyard/chef-provisioning#337.
Actually, I would love to see some documentation about this somewhere. I've heard multiple people having issues with the changes introduced in chef-client 12.3. We should at least update https://docs.chef.io/release_notes.html to detail what changed from 12.2 to 12.3 when running chef-client -z.
|
Absent any unanswered concerns, I'll merge this in about an hour. |
Added support for chef-provisioning 1.0
|
@randomcamel will you close the 4 related issues listed at the top of this? |
Also addresses