From ec2e0380ee813cc70d9d1f3bbedd3bb92f5132b3 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 25 Feb 2019 15:25:22 -0800 Subject: [PATCH 01/64] Add basic docker driver methods for `docker network` subcommand --- plugins/providers/docker/driver.rb | 43 ++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/plugins/providers/docker/driver.rb b/plugins/providers/docker/driver.rb index 88903039c..bc5867dd6 100644 --- a/plugins/providers/docker/driver.rb +++ b/plugins/providers/docker/driver.rb @@ -177,6 +177,49 @@ module VagrantPlugins end end + # @param[String] network - name of network to connect conatiner to + # @param[String] cid - container id + # @param[Array] opts - An array of flags used for listing networks + def connect_network(network, cid, opts=nil) + output = execute('docker', 'network', 'connect', network, cid, opts) + end + + # @param[String] network - name of network to create + # @param[Array] opts - An array of flags used for listing networks + def create_network(network, opts=nil) + output = execute('docker', 'network', 'create', network, opts) + end + + # @param[String] network - name of network to disconnect container from + # @param[String] cid - container id + # @param[Array] opts - An array of flags used for listing networks + def disconnect_network(network, cid, opts=nil) + output = execute('docker', 'network', 'disconnect', network, cid, opts) + end + + # @param[Array] networks - list of networks to inspect + # @param[Array] opts - An array of flags used for listing networks + def inspect_network(networks, opts=nil) + output = execute('docker', 'network', 'inspect', networks, opts) + end + + # @param[Array] opts - An array of flags used for listing networks + def list_network(opts=nil) + output = execute('docker', 'network', 'ls', opts) + end + + # @param[Array] opts - An array of flags used for listing networks + def prune_network(opts) + output = execute('docker', 'network', 'prune', '--force', opts) + end + + # @param[String] network - name of network to remove + # @param[Array] opts - An array of flags used for listing networks + def rm_network(networks, opts) + output = execute('docker', 'network', 'rm', network, opts) + end + + # @param[Array] opts - An array of flags used for listing networks def execute(*cmd, **opts, &block) @executor.execute(*cmd, **opts, &block) end From 627251a307a2c93bb4cc58988ccd7011e2574776 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 25 Feb 2019 15:27:04 -0800 Subject: [PATCH 02/64] Add basic docker network action --- plugins/providers/docker/action/network.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 plugins/providers/docker/action/network.rb diff --git a/plugins/providers/docker/action/network.rb b/plugins/providers/docker/action/network.rb new file mode 100644 index 000000000..cede34e8a --- /dev/null +++ b/plugins/providers/docker/action/network.rb @@ -0,0 +1,18 @@ +module VagrantPlugins + module DockerProvider + module Action + class Network + def initialize(app, env) + @app = app + end + + def call(env) + # If we aren't using a host VM, then don't worry about it + return @app.call(env) if !env[:machine].provider.host_vm? + + @app.call(env) + end + end + end + end +end From 67ea15126d15ad4c539dd5c7c6cc5040081ccb89 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 25 Feb 2019 15:29:50 -0800 Subject: [PATCH 03/64] Make opts var optional for docker driver methods --- plugins/providers/docker/driver.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/providers/docker/driver.rb b/plugins/providers/docker/driver.rb index bc5867dd6..f8f44fe48 100644 --- a/plugins/providers/docker/driver.rb +++ b/plugins/providers/docker/driver.rb @@ -209,13 +209,13 @@ module VagrantPlugins end # @param[Array] opts - An array of flags used for listing networks - def prune_network(opts) + def prune_network(opts=nil) output = execute('docker', 'network', 'prune', '--force', opts) end # @param[String] network - name of network to remove # @param[Array] opts - An array of flags used for listing networks - def rm_network(networks, opts) + def rm_network(networks, opts=nil) output = execute('docker', 'network', 'rm', network, opts) end From bed653eeb44b495c432c5e07c4f6f638661447dc Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 25 Feb 2019 15:30:04 -0800 Subject: [PATCH 04/64] Autoload and use Network action on up --- plugins/providers/docker/action.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/providers/docker/action.rb b/plugins/providers/docker/action.rb index f423f6e65..0b1692b72 100644 --- a/plugins/providers/docker/action.rb +++ b/plugins/providers/docker/action.rb @@ -28,6 +28,7 @@ module VagrantPlugins b.use ConfigValidate b.use HostMachine + b.use Network # Yeah, this is supposed to be here twice (once more above). This # catches the case when the container was supposed to be created, @@ -311,6 +312,7 @@ module VagrantPlugins autoload :Pull, action_root.join("pull") autoload :PrepareSSH, action_root.join("prepare_ssh") autoload :Stop, action_root.join("stop") + autoload :Network, action_root.join("network") autoload :PrepareNFSValidIds, action_root.join("prepare_nfs_valid_ids") autoload :PrepareNFSSettings, action_root.join("prepare_nfs_settings") autoload :Start, action_root.join("start") From a336aa687c166db6fcfeea3c3f4cb093af637411 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 25 Feb 2019 16:11:32 -0800 Subject: [PATCH 05/64] Add logger and start to iterate over networks in config --- plugins/providers/docker/action/network.rb | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/plugins/providers/docker/action/network.rb b/plugins/providers/docker/action/network.rb index cede34e8a..2d2391e2d 100644 --- a/plugins/providers/docker/action/network.rb +++ b/plugins/providers/docker/action/network.rb @@ -1,14 +1,29 @@ +require 'log4r' + module VagrantPlugins module DockerProvider module Action class Network def initialize(app, env) @app = app + @logger = Log4r::Logger.new('vagrant::plugins::docker::network') end def call(env) - # If we aren't using a host VM, then don't worry about it - return @app.call(env) if !env[:machine].provider.host_vm? + # If we are using a host VM, then don't worry about it + if env[:machine].provider.host_vm? + @logger.debug("Not setting up networks because docker host_vm is in use") + return @app.call(env) + end + + env[:machine].config.vm.networks.each do |type, options| + # We only handle private and public networks + next if type != :private_network && type != :public_network + + # TODO: Configure and set up network for each container that has a network + # machine.id == container id + # docker network name == env[:machine].name + "_vagrant_" + unique-sha + end @app.call(env) end From 349cc5ddac1c377ba5f93dfadde45f80992c4000 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 26 Feb 2019 11:46:00 -0800 Subject: [PATCH 06/64] Add placeholder network destroy action for docker --- .../docker/action/destroy_network.rb | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 plugins/providers/docker/action/destroy_network.rb diff --git a/plugins/providers/docker/action/destroy_network.rb b/plugins/providers/docker/action/destroy_network.rb new file mode 100644 index 000000000..e45a558b3 --- /dev/null +++ b/plugins/providers/docker/action/destroy_network.rb @@ -0,0 +1,33 @@ +require 'log4r' + +module VagrantPlugins + module DockerProvider + module Action + class DestroyNetwork + def initialize(app, env) + @app = app + @logger = Log4r::Logger.new('vagrant::plugins::docker::network') + end + + def call(env) + # If we are using a host VM, then don't worry about it + machine = env[:machine] + if machine.provider.host_vm? + @logger.debug("Not setting up networks because docker host_vm is in use") + return @app.call(env) + end + + machine.config.vm.networks.each do |type, options| + # We only handle private and public networks + next if type != :private_network && type != :public_network + + # If network is defined in machines config as isolated single container network, then delete + # If it's custom and or default, check if other containers are using it, and if not, delete + end + + @app.call(env) + end + end + end + end +end From 63ba0f9964076efb892f209caa53197e84cc2f77 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 26 Feb 2019 11:47:17 -0800 Subject: [PATCH 07/64] Add DestroyNetwork action to docker provider --- plugins/providers/docker/action.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/providers/docker/action.rb b/plugins/providers/docker/action.rb index 0b1692b72..9dbe64499 100644 --- a/plugins/providers/docker/action.rb +++ b/plugins/providers/docker/action.rb @@ -162,6 +162,7 @@ module VagrantPlugins b4.use action_halt b4.use HostMachineSyncFoldersDisable b4.use Destroy + b4.use DestroyNetwork b4.use DestroyBuildImage else b4.use Message, @@ -296,6 +297,7 @@ module VagrantPlugins autoload :Create, action_root.join("create") autoload :Destroy, action_root.join("destroy") autoload :DestroyBuildImage, action_root.join("destroy_build_image") + autoload :DestroyNetwork, action_root.join("destroy_network") autoload :ForwardedPorts, action_root.join("forwarded_ports") autoload :HasSSH, action_root.join("has_ssh") autoload :HostMachine, action_root.join("host_machine") From 8013d8d5ef0638722984126f98eab46007d02767 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 26 Feb 2019 11:47:33 -0800 Subject: [PATCH 08/64] Move around Network setup action for docker provider --- plugins/providers/docker/action.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/providers/docker/action.rb b/plugins/providers/docker/action.rb index 9dbe64499..68bb78011 100644 --- a/plugins/providers/docker/action.rb +++ b/plugins/providers/docker/action.rb @@ -28,7 +28,6 @@ module VagrantPlugins b.use ConfigValidate b.use HostMachine - b.use Network # Yeah, this is supposed to be here twice (once more above). This # catches the case when the container was supposed to be created, @@ -267,6 +266,7 @@ module VagrantPlugins end end + b2.use Network b2.use Start b2.use WaitForRunning From e860c7709d15148a569326d816ea2e49daa1f528 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 26 Feb 2019 11:47:48 -0800 Subject: [PATCH 09/64] Begin to setup and configure docker networks for containers --- plugins/providers/docker/action/network.rb | 19 ++++++--- plugins/providers/docker/driver.rb | 46 ++++++++++++++-------- 2 files changed, 44 insertions(+), 21 deletions(-) diff --git a/plugins/providers/docker/action/network.rb b/plugins/providers/docker/action/network.rb index 2d2391e2d..d4cee09c5 100644 --- a/plugins/providers/docker/action/network.rb +++ b/plugins/providers/docker/action/network.rb @@ -11,18 +11,27 @@ module VagrantPlugins def call(env) # If we are using a host VM, then don't worry about it - if env[:machine].provider.host_vm? + machine = env[:machine] + if machine.provider.host_vm? @logger.debug("Not setting up networks because docker host_vm is in use") return @app.call(env) end - env[:machine].config.vm.networks.each do |type, options| + machine.config.vm.networks.each do |type, options| # We only handle private and public networks next if type != :private_network && type != :public_network - # TODO: Configure and set up network for each container that has a network - # machine.id == container id - # docker network name == env[:machine].name + "_vagrant_" + unique-sha + # Look at docker provider config for network options: + # - If not defined, create simple "folder environment" multi-network + # that all containers with this default option are attached to + # - If provider config option is defined, create the requested network + # and attach that container to it + network_name = "#{env[:root_path].basename.to_s}_network_#{machine.name}" + container_id = machine.id + # TODO: Need to check if network already exists and not error + machine.provider.driver.create_network(network_name) + options = ["--ip", options[:ip]] + machine.provider.driver.connect_network(network_name, container_id, options) end @app.call(env) diff --git a/plugins/providers/docker/driver.rb b/plugins/providers/docker/driver.rb index f8f44fe48..9de7d5cd3 100644 --- a/plugins/providers/docker/driver.rb +++ b/plugins/providers/docker/driver.rb @@ -180,43 +180,57 @@ module VagrantPlugins # @param[String] network - name of network to connect conatiner to # @param[String] cid - container id # @param[Array] opts - An array of flags used for listing networks - def connect_network(network, cid, opts=nil) - output = execute('docker', 'network', 'connect', network, cid, opts) + def connect_network(network, cid, *opts) + command = ['docker', 'network', 'connect', network, cid].concat(*opts) + output = execute(*command) + output end # @param[String] network - name of network to create - # @param[Array] opts - An array of flags used for listing networks - def create_network(network, opts=nil) - output = execute('docker', 'network', 'create', network, opts) + # @param[Array] opts - An array of flags used for listing networks + def create_network(network, *opts) + command = ['docker', 'network', 'create', network].concat(*opts) + output = execute(*command) + output end # @param[String] network - name of network to disconnect container from # @param[String] cid - container id - # @param[Array] opts - An array of flags used for listing networks - def disconnect_network(network, cid, opts=nil) - output = execute('docker', 'network', 'disconnect', network, cid, opts) + # @param[Array] opts - An array of flags used for listing networks + def disconnect_network(network, cid, *opts) + command = ['docker', 'network', 'disconnect', network, cid].concat(*opts) + output = execute(*command) + output end # @param[Array] networks - list of networks to inspect # @param[Array] opts - An array of flags used for listing networks - def inspect_network(networks, opts=nil) - output = execute('docker', 'network', 'inspect', networks, opts) + def inspect_network(networks, *opts) + command = ['docker', 'network', 'inspect', networks].concat(*opts) + output = execute(*command) + output end # @param[Array] opts - An array of flags used for listing networks - def list_network(opts=nil) - output = execute('docker', 'network', 'ls', opts) + def list_network(*opts) + command = ['docker', 'network', 'ls'].concat(*opts) + output = execute(*command) + output end # @param[Array] opts - An array of flags used for listing networks - def prune_network(opts=nil) - output = execute('docker', 'network', 'prune', '--force', opts) + def prune_network(*opts) + command = ['docker', 'network', 'prune', '--force'].concat(*opts) + output = execute(*command) + output end # @param[String] network - name of network to remove # @param[Array] opts - An array of flags used for listing networks - def rm_network(networks, opts=nil) - output = execute('docker', 'network', 'rm', network, opts) + def rm_network(networks, *opts) + command = ['docker', 'network', 'rm', network].concat(*opts) + output = execute(*command) + output end # @param[Array] opts - An array of flags used for listing networks From 4dc5f7c330facd5dc36e2deabd46d082819fddef Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 26 Feb 2019 15:00:04 -0800 Subject: [PATCH 10/64] Add basic ability to configure some networks for containers --- .../docker/action/destroy_network.rb | 8 ++++++ plugins/providers/docker/action/network.rb | 27 ++++++++++++++++--- plugins/providers/docker/driver.rb | 25 ++++++++++++++--- 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/plugins/providers/docker/action/destroy_network.rb b/plugins/providers/docker/action/destroy_network.rb index e45a558b3..b056751ff 100644 --- a/plugins/providers/docker/action/destroy_network.rb +++ b/plugins/providers/docker/action/destroy_network.rb @@ -23,6 +23,14 @@ module VagrantPlugins # If network is defined in machines config as isolated single container network, then delete # If it's custom and or default, check if other containers are using it, and if not, delete + network_name = "#{env[:root_path].basename.to_s}_network_#{machine.name}" + if machine.provider.driver.existing_network?(network_name) && + !machine.provider.driver.network_used?(network_name) + env[:ui].info("Removing network #{network_name}") + machine.provider.driver.rm_network(network_name) + else + @logger.debug("Network #{network_name} not found") + end end @app.call(env) diff --git a/plugins/providers/docker/action/network.rb b/plugins/providers/docker/action/network.rb index d4cee09c5..af98eed4a 100644 --- a/plugins/providers/docker/action/network.rb +++ b/plugins/providers/docker/action/network.rb @@ -17,6 +17,8 @@ module VagrantPlugins return @app.call(env) end + env[:ui].info("Configuring and enabling network interfaces...") + machine.config.vm.networks.each do |type, options| # We only handle private and public networks next if type != :private_network && type != :public_network @@ -26,12 +28,31 @@ module VagrantPlugins # that all containers with this default option are attached to # - If provider config option is defined, create the requested network # and attach that container to it + connect_cli_opts = [] + create_cli_opts = [] + + # make this a function that generates the proper flags + if options[:type] != "dhcp" + if options[:subnet] + create_cli_opts.concat(["--subnet", options[:subnet]]) + end + + if options[:ip] + connect_cli_opts.concat(["--ip", options[:ip]]) + end + end + network_name = "#{env[:root_path].basename.to_s}_network_#{machine.name}" container_id = machine.id # TODO: Need to check if network already exists and not error - machine.provider.driver.create_network(network_name) - options = ["--ip", options[:ip]] - machine.provider.driver.connect_network(network_name, container_id, options) + if !machine.provider.driver.existing_network?(network_name) + @logger.debug("Creating network #{network_name}") + machine.provider.driver.create_network(network_name, create_cli_opts) + else + @logger.debug("Network #{network_name} already created") + end + @logger.debug("Connecting network #{network_name} to container guest #{machine.name}") + machine.provider.driver.connect_network(network_name, container_id, connect_cli_opts) end @app.call(env) diff --git a/plugins/providers/docker/driver.rb b/plugins/providers/docker/driver.rb index 9de7d5cd3..3896ed11d 100644 --- a/plugins/providers/docker/driver.rb +++ b/plugins/providers/docker/driver.rb @@ -205,8 +205,8 @@ module VagrantPlugins # @param[Array] networks - list of networks to inspect # @param[Array] opts - An array of flags used for listing networks - def inspect_network(networks, *opts) - command = ['docker', 'network', 'inspect', networks].concat(*opts) + def inspect_network(network, *opts) + command = ['docker', 'network', 'inspect', network].concat(*opts) output = execute(*command) output end @@ -227,12 +227,31 @@ module VagrantPlugins # @param[String] network - name of network to remove # @param[Array] opts - An array of flags used for listing networks - def rm_network(networks, *opts) + def rm_network(network, *opts) command = ['docker', 'network', 'rm', network].concat(*opts) output = execute(*command) output end + # Docker network helpers + + # @param[String] network - name of network to look for + def existing_network?(network) + result = list_network(["--format='{{json .Name}}'"]) + result.include?(network) + end + + # @param[String] network - name of network to look for + def network_used?(network) + result = inspect_network(network) + begin + result = JSON.parse(result) + return result.first["Containers"].size > 0 + rescue JSON::ParserError => e + # Could not parse result of network inspection + end + end + # @param[Array] opts - An array of flags used for listing networks def execute(*cmd, **opts, &block) @executor.execute(*cmd, **opts, &block) From dc5f8c66f217b23906c30b2c44995d32f88350d5 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 26 Feb 2019 15:54:29 -0800 Subject: [PATCH 11/64] Add todo comment for future fixup --- plugins/providers/docker/driver.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/plugins/providers/docker/driver.rb b/plugins/providers/docker/driver.rb index 3896ed11d..31d2d9953 100644 --- a/plugins/providers/docker/driver.rb +++ b/plugins/providers/docker/driver.rb @@ -233,11 +233,14 @@ module VagrantPlugins output end + # ###################### # Docker network helpers + # ###################### # @param[String] network - name of network to look for def existing_network?(network) result = list_network(["--format='{{json .Name}}'"]) + #TODO: we should be more explicit here if we can result.include?(network) end From ba2a1224e08fb4b1a9c566335d1eb35b3c1f8910 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 27 Feb 2019 14:54:30 -0800 Subject: [PATCH 12/64] Update driver to include network tests --- .../plugins/providers/docker/driver_test.rb | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/test/unit/plugins/providers/docker/driver_test.rb b/test/unit/plugins/providers/docker/driver_test.rb index fb271af54..d4e66b72e 100644 --- a/test/unit/plugins/providers/docker/driver_test.rb +++ b/test/unit/plugins/providers/docker/driver_test.rb @@ -251,4 +251,48 @@ describe VagrantPlugins::DockerProvider::Driver do expect(subject.docker_bridge_ip).to eq('123.456.789.012') end end + + describe '#docker_connect_network' do + let(:opts) { ["--ip", "172.20.128.2"] } + it 'connects a network to a container' do + expect(subject).to receive(:execute).with("docker", "network", "connect", "vagrant_network", cid, "--ip", "172.20.128.2") + subject.connect_network("vagrant_network", cid, opts) + end + end + + describe '#docker_create_network' do + let(:opts) { ["--subnet", "172.20.0.0/16"] } + it 'creates a network' do + expect(subject).to receive(:execute).with("docker", "network", "create", "vagrant_network", "--subnet", "172.20.0.0/16") + subject.create_network("vagrant_network", opts) + end + end + + describe '#docker_disconnet_network' do + it 'disconnects a network from a container' do + expect(subject).to receive(:execute).with("docker", "network", "disconnect", "vagrant_network", cid, "--force") + subject.disconnect_network("vagrant_network", cid) + end + end + + describe '#docker_inspect_network' do + it 'gets info about a network' do + expect(subject).to receive(:execute).with("docker", "network", "inspect", "vagrant_network") + subject.inspect_network("vagrant_network") + end + end + + describe '#docker_list_network' do + it 'lists docker networks' do + expect(subject).to receive(:execute).with("docker", "network", "ls") + subject.list_network() + end + end + + describe '#docker_rm_network' do + it 'deletes a docker network' do + expect(subject).to receive(:execute).with("docker", "network", "rm", "vagrant_network") + subject.rm_network("vagrant_network") + end + end end From b5a092397f8f4e7c05182b4c7c2fbe57e590c8c6 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 27 Feb 2019 14:55:01 -0800 Subject: [PATCH 13/64] Add new function for handling vagrant options to docker network cli flags --- .../docker/action/destroy_network.rb | 1 + plugins/providers/docker/action/network.rb | 75 +++++++++++++------ plugins/providers/docker/driver.rb | 55 ++++++++------ 3 files changed, 87 insertions(+), 44 deletions(-) diff --git a/plugins/providers/docker/action/destroy_network.rb b/plugins/providers/docker/action/destroy_network.rb index b056751ff..2cbe560a5 100644 --- a/plugins/providers/docker/action/destroy_network.rb +++ b/plugins/providers/docker/action/destroy_network.rb @@ -24,6 +24,7 @@ module VagrantPlugins # If network is defined in machines config as isolated single container network, then delete # If it's custom and or default, check if other containers are using it, and if not, delete network_name = "#{env[:root_path].basename.to_s}_network_#{machine.name}" + if machine.provider.driver.existing_network?(network_name) && !machine.provider.driver.network_used?(network_name) env[:ui].info("Removing network #{network_name}") diff --git a/plugins/providers/docker/action/network.rb b/plugins/providers/docker/action/network.rb index af98eed4a..ed6abc7f5 100644 --- a/plugins/providers/docker/action/network.rb +++ b/plugins/providers/docker/action/network.rb @@ -9,6 +9,55 @@ module VagrantPlugins @logger = Log4r::Logger.new('vagrant::plugins::docker::network') end + # TODO: This is not ideal, but not enitrely sure a good way + # around this without either having separate config options (which is worse for the user) + # or forcing them to care about options for each separate docker network command (which is also + # not great for the user). Basically a user wants to say: + # + # config.vm.network :private_network, + # + # We could limit the options to be name spaced simply `docker__option` like + # + # config.vm.network :private_network, docker__ip: "ip here" + # + # But then we will need this method to split out each option for creating and connecting networks + # + # Alternatively we could do + # + # - `docker__create__option` + # - `docker__connect__option` + # - ...etc... + # + # config.vm.network :private_network, docker__connect__ip: "ip here", + # docker__create__subnet: "subnet" + # + # But this will force users to care about what options are for which commands, + # but maybe they will have to care about it anyway, and it isn't that big of + # a deal for the extra namespacing? + # + # Extra namespacing puts more effort on the user side, but would allow us + # to be 'smarter' about how we set options in code by seeing essnetially + # what command the flag/option is intended for, rather than us trying to keep + # track of the commands/flags manually in this function. + # + # @param[Hash] options - options from the network config + # @returns[Hash] cli_opts - an array of strings used for the network commnad + def parse_cli_arguments(options) + cli_opts = {create: [], connect: []} + + # make this a function that generates the proper flags + if options[:type] != "dhcp" + if options[:docker__subnet] + cli_opts[:create].concat(["--subnet", options[:docker__subnet]]) + end + + if options[:docker__ip] + cli_opts[:connect].concat(["--ip", options[:docker__ip]]) + end + end + return cli_opts + end + def call(env) # If we are using a host VM, then don't worry about it machine = env[:machine] @@ -23,36 +72,20 @@ module VagrantPlugins # We only handle private and public networks next if type != :private_network && type != :public_network - # Look at docker provider config for network options: - # - If not defined, create simple "folder environment" multi-network - # that all containers with this default option are attached to - # - If provider config option is defined, create the requested network - # and attach that container to it - connect_cli_opts = [] - create_cli_opts = [] - - # make this a function that generates the proper flags - if options[:type] != "dhcp" - if options[:subnet] - create_cli_opts.concat(["--subnet", options[:subnet]]) - end - - if options[:ip] - connect_cli_opts.concat(["--ip", options[:ip]]) - end - end + cli_opts = parse_cli_arguments(options) network_name = "#{env[:root_path].basename.to_s}_network_#{machine.name}" container_id = machine.id - # TODO: Need to check if network already exists and not error + if !machine.provider.driver.existing_network?(network_name) @logger.debug("Creating network #{network_name}") - machine.provider.driver.create_network(network_name, create_cli_opts) + machine.provider.driver.create_network(network_name, cli_opts[:create]) else @logger.debug("Network #{network_name} already created") end + @logger.debug("Connecting network #{network_name} to container guest #{machine.name}") - machine.provider.driver.connect_network(network_name, container_id, connect_cli_opts) + machine.provider.driver.connect_network(network_name, container_id, cli_opts[:connect]) end @app.call(env) diff --git a/plugins/providers/docker/driver.rb b/plugins/providers/docker/driver.rb index 31d2d9953..cbc17c96c 100644 --- a/plugins/providers/docker/driver.rb +++ b/plugins/providers/docker/driver.rb @@ -177,74 +177,86 @@ module VagrantPlugins end end - # @param[String] network - name of network to connect conatiner to - # @param[String] cid - container id - # @param[Array] opts - An array of flags used for listing networks + # @param [String] network - name of network to connect conatiner to + # @param [String] cid - container id + # @param [Array] opts - An array of flags used for listing networks def connect_network(network, cid, *opts) command = ['docker', 'network', 'connect', network, cid].concat(*opts) output = execute(*command) output end - # @param[String] network - name of network to create - # @param[Array] opts - An array of flags used for listing networks + # @param [String] network - name of network to create + # @param [Array] opts - An array of flags used for listing networks def create_network(network, *opts) command = ['docker', 'network', 'create', network].concat(*opts) output = execute(*command) output end - # @param[String] network - name of network to disconnect container from - # @param[String] cid - container id - # @param[Array] opts - An array of flags used for listing networks + # @param [String] network - name of network to disconnect container from + # @param [String] cid - container id def disconnect_network(network, cid, *opts) - command = ['docker', 'network', 'disconnect', network, cid].concat(*opts) + command = ['docker', 'network', 'disconnect', network, cid, "--force"] output = execute(*command) output end - # @param[Array] networks - list of networks to inspect - # @param[Array] opts - An array of flags used for listing networks + # @param [Array] networks - list of networks to inspect + # @param [Array] opts - An array of flags used for listing networks def inspect_network(network, *opts) command = ['docker', 'network', 'inspect', network].concat(*opts) output = execute(*command) output end - # @param[Array] opts - An array of flags used for listing networks + # @param [Array] opts - An array of flags used for listing networks def list_network(*opts) command = ['docker', 'network', 'ls'].concat(*opts) output = execute(*command) output end - # @param[Array] opts - An array of flags used for listing networks + # Will delete _all_ defined but unused networks in the docker engine. Even + # networks not created by Vagrant. + # + # @param [Array] opts - An array of flags used for listing networks def prune_network(*opts) command = ['docker', 'network', 'prune', '--force'].concat(*opts) output = execute(*command) output end - # @param[String] network - name of network to remove - # @param[Array] opts - An array of flags used for listing networks - def rm_network(network, *opts) - command = ['docker', 'network', 'rm', network].concat(*opts) + # TODO: Note...cli can optionally take a list of networks to delete. + # We might need this later, but for now our helper takes 1 network at a time + # + # @param [String] network - name of network to remove + def rm_network(network) + command = ['docker', 'network', 'rm', network] output = execute(*command) output end + # @param [Array] opts - An array of flags used for listing networks + def execute(*cmd, **opts, &block) + @executor.execute(*cmd, **opts, &block) + end + # ###################### # Docker network helpers # ###################### - # @param[String] network - name of network to look for + # @param [String] network - name of network to look for def existing_network?(network) result = list_network(["--format='{{json .Name}}'"]) #TODO: we should be more explicit here if we can result.include?(network) end - # @param[String] network - name of network to look for + # Looks to see if a docker network has already been defined + # + # @param [String] network - name of network to look for + # @return [Bool] def network_used?(network) result = inspect_network(network) begin @@ -252,13 +264,10 @@ module VagrantPlugins return result.first["Containers"].size > 0 rescue JSON::ParserError => e # Could not parse result of network inspection + # Handle this some how, maybe log error but not raise? end end - # @param[Array] opts - An array of flags used for listing networks - def execute(*cmd, **opts, &block) - @executor.execute(*cmd, **opts, &block) - end end end end From 4a0f1a0a33f13e017095b2c42b53fbf372e80d7f Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 28 Feb 2019 14:53:36 -0800 Subject: [PATCH 14/64] Update docker network action to auto-generate cli flags This commit updates the docker network behavior to auto-generate the docker network command flags from Vagrants network config option. --- plugins/providers/docker/action/network.rb | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/plugins/providers/docker/action/network.rb b/plugins/providers/docker/action/network.rb index ed6abc7f5..500087078 100644 --- a/plugins/providers/docker/action/network.rb +++ b/plugins/providers/docker/action/network.rb @@ -45,16 +45,16 @@ module VagrantPlugins def parse_cli_arguments(options) cli_opts = {create: [], connect: []} - # make this a function that generates the proper flags + # Splits the networking options to generate the proper CLI flags for docker if options[:type] != "dhcp" - if options[:docker__subnet] - cli_opts[:create].concat(["--subnet", options[:docker__subnet]]) - end - - if options[:docker__ip] - cli_opts[:connect].concat(["--ip", options[:docker__ip]]) + options.each do |opt, value| + vals = opt.to_s.split("__") + if vals[0] == "docker" && vals.size == 3 + cli_opts[vals[1].to_sym].concat(["--#{vals[2]}", value]) + end end end + return cli_opts end From c20de9044d3f98947bbb5513c65e2143a7abb543 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 28 Feb 2019 15:24:48 -0800 Subject: [PATCH 15/64] Log information if unsupported docker network option is provided --- plugins/providers/docker/action/network.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/providers/docker/action/network.rb b/plugins/providers/docker/action/network.rb index 500087078..38b797ed4 100644 --- a/plugins/providers/docker/action/network.rb +++ b/plugins/providers/docker/action/network.rb @@ -51,6 +51,8 @@ module VagrantPlugins vals = opt.to_s.split("__") if vals[0] == "docker" && vals.size == 3 cli_opts[vals[1].to_sym].concat(["--#{vals[2]}", value]) + else + @logger.debug("Unsupported network option encountered: #{opt}. Ignoring...") end end end From 000457a012476de64b9aad092e49f508935f7196 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 1 Mar 2019 16:07:53 -0800 Subject: [PATCH 16/64] Update how docker network provider creates networks This commit updates the docker network provider to only create networks by subnet rather than per-container. --- .../docker/action/destroy_network.rb | 8 ++- plugins/providers/docker/action/network.rb | 68 +++++++------------ plugins/providers/docker/driver.rb | 6 +- 3 files changed, 32 insertions(+), 50 deletions(-) diff --git a/plugins/providers/docker/action/destroy_network.rb b/plugins/providers/docker/action/destroy_network.rb index 2cbe560a5..215947770 100644 --- a/plugins/providers/docker/action/destroy_network.rb +++ b/plugins/providers/docker/action/destroy_network.rb @@ -21,9 +21,11 @@ module VagrantPlugins # We only handle private and public networks next if type != :private_network && type != :public_network - # If network is defined in machines config as isolated single container network, then delete - # If it's custom and or default, check if other containers are using it, and if not, delete - network_name = "#{env[:root_path].basename.to_s}_network_#{machine.name}" + if options[:subnet] + network_name = "vagrant_network_#{options[:subnet]}" + else + network_name = "vagrant_network" + end if machine.provider.driver.existing_network?(network_name) && !machine.provider.driver.network_used?(network_name) diff --git a/plugins/providers/docker/action/network.rb b/plugins/providers/docker/action/network.rb index 38b797ed4..b2cfcc1a6 100644 --- a/plugins/providers/docker/action/network.rb +++ b/plugins/providers/docker/action/network.rb @@ -9,51 +9,20 @@ module VagrantPlugins @logger = Log4r::Logger.new('vagrant::plugins::docker::network') end - # TODO: This is not ideal, but not enitrely sure a good way - # around this without either having separate config options (which is worse for the user) - # or forcing them to care about options for each separate docker network command (which is also - # not great for the user). Basically a user wants to say: - # - # config.vm.network :private_network, - # - # We could limit the options to be name spaced simply `docker__option` like - # - # config.vm.network :private_network, docker__ip: "ip here" - # - # But then we will need this method to split out each option for creating and connecting networks - # - # Alternatively we could do - # - # - `docker__create__option` - # - `docker__connect__option` - # - ...etc... - # - # config.vm.network :private_network, docker__connect__ip: "ip here", - # docker__create__subnet: "subnet" - # - # But this will force users to care about what options are for which commands, - # but maybe they will have to care about it anyway, and it isn't that big of - # a deal for the extra namespacing? - # - # Extra namespacing puts more effort on the user side, but would allow us - # to be 'smarter' about how we set options in code by seeing essnetially - # what command the flag/option is intended for, rather than us trying to keep - # track of the commands/flags manually in this function. - # # @param[Hash] options - options from the network config - # @returns[Hash] cli_opts - an array of strings used for the network commnad + # @returns[Array] cli_opts - an array of strings used for the network commnad def parse_cli_arguments(options) - cli_opts = {create: [], connect: []} + cli_opts = [] # Splits the networking options to generate the proper CLI flags for docker - if options[:type] != "dhcp" - options.each do |opt, value| - vals = opt.to_s.split("__") - if vals[0] == "docker" && vals.size == 3 - cli_opts[vals[1].to_sym].concat(["--#{vals[2]}", value]) - else - @logger.debug("Unsupported network option encountered: #{opt}. Ignoring...") - end + options.each do |opt, value| + opt = opt.to_s + if opt == "ip" || (opt == "type" && value = "dhcp") || + opt == "protocol" || opt == "id" + # `docker network create` doesn't care about these options + next + else + cli_opts.concat(["--#{opt}", value]) end end @@ -76,18 +45,29 @@ module VagrantPlugins cli_opts = parse_cli_arguments(options) - network_name = "#{env[:root_path].basename.to_s}_network_#{machine.name}" + if options[:subnet] + network_name = "vagrant_network_#{options[:subnet]}" + elsif options[:type] == "dhcp" + network_name = "vagrant_network" + else + # TODO: Make this an error class + raise "Must specify a `subnet` or use `dhcp`" + end container_id = machine.id if !machine.provider.driver.existing_network?(network_name) @logger.debug("Creating network #{network_name}") - machine.provider.driver.create_network(network_name, cli_opts[:create]) + machine.provider.driver.create_network(network_name, cli_opts) else @logger.debug("Network #{network_name} already created") end @logger.debug("Connecting network #{network_name} to container guest #{machine.name}") - machine.provider.driver.connect_network(network_name, container_id, cli_opts[:connect]) + connect_opts = [] + if options[:ip] + connect_opts = ["--ip", options[:ip]] + end + machine.provider.driver.connect_network(network_name, container_id, connect_opts) end @app.call(env) diff --git a/plugins/providers/docker/driver.rb b/plugins/providers/docker/driver.rb index cbc17c96c..264d6663e 100644 --- a/plugins/providers/docker/driver.rb +++ b/plugins/providers/docker/driver.rb @@ -246,15 +246,15 @@ module VagrantPlugins # Docker network helpers # ###################### + # Looks to see if a docker network has already been defined + # # @param [String] network - name of network to look for def existing_network?(network) result = list_network(["--format='{{json .Name}}'"]) #TODO: we should be more explicit here if we can - result.include?(network) + result.match?(/\"#{network}\"/) end - # Looks to see if a docker network has already been defined - # # @param [String] network - name of network to look for # @return [Bool] def network_used?(network) From 29696b0f731813557b2b445be1bfe5586dbe0260 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 1 Mar 2019 16:20:16 -0800 Subject: [PATCH 17/64] Allow for ipv6 networks in docker --- plugins/providers/docker/action/network.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugins/providers/docker/action/network.rb b/plugins/providers/docker/action/network.rb index b2cfcc1a6..a6b1e743b 100644 --- a/plugins/providers/docker/action/network.rb +++ b/plugins/providers/docker/action/network.rb @@ -66,6 +66,8 @@ module VagrantPlugins connect_opts = [] if options[:ip] connect_opts = ["--ip", options[:ip]] + elsif options[:ip6] + connect_opts = ["--ip6", options[:ip6]] end machine.provider.driver.connect_network(network_name, container_id, connect_opts) end From 5adffb608d14ee073728866961c429ca13f663fc Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 1 Mar 2019 16:23:09 -0800 Subject: [PATCH 18/64] Only allow private_network for docker network provider --- plugins/providers/docker/action/destroy_network.rb | 4 ++-- plugins/providers/docker/action/network.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/providers/docker/action/destroy_network.rb b/plugins/providers/docker/action/destroy_network.rb index 215947770..4a0b2943d 100644 --- a/plugins/providers/docker/action/destroy_network.rb +++ b/plugins/providers/docker/action/destroy_network.rb @@ -18,8 +18,8 @@ module VagrantPlugins end machine.config.vm.networks.each do |type, options| - # We only handle private and public networks - next if type != :private_network && type != :public_network + # We only handle private networks + next if type != :private_network if options[:subnet] network_name = "vagrant_network_#{options[:subnet]}" diff --git a/plugins/providers/docker/action/network.rb b/plugins/providers/docker/action/network.rb index a6b1e743b..557a9cf68 100644 --- a/plugins/providers/docker/action/network.rb +++ b/plugins/providers/docker/action/network.rb @@ -40,8 +40,8 @@ module VagrantPlugins env[:ui].info("Configuring and enabling network interfaces...") machine.config.vm.networks.each do |type, options| - # We only handle private and public networks - next if type != :private_network && type != :public_network + # We only handle private networks + next if type != :private_network cli_opts = parse_cli_arguments(options) From 4080f9e64d362e79af8b1846f286ef9b7a1dedcb Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 4 Mar 2019 10:25:10 -0800 Subject: [PATCH 19/64] Log warning if docker network inspect fails to return json --- plugins/providers/docker/driver.rb | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/plugins/providers/docker/driver.rb b/plugins/providers/docker/driver.rb index 264d6663e..4e73641f4 100644 --- a/plugins/providers/docker/driver.rb +++ b/plugins/providers/docker/driver.rb @@ -255,16 +255,19 @@ module VagrantPlugins result.match?(/\"#{network}\"/) end + # Returns true or false if network is in use or not. + # Nil if Vagrant fails to receive proper JSON from `docker network inspect` + # # @param [String] network - name of network to look for - # @return [Bool] + # @return [Bool,nil] def network_used?(network) result = inspect_network(network) begin result = JSON.parse(result) return result.first["Containers"].size > 0 rescue JSON::ParserError => e - # Could not parse result of network inspection - # Handle this some how, maybe log error but not raise? + @logger.warn("Could not properly parse response from `docker network inspect #{network}`") + return nil end end From 6c7c74be5a1ff91a251f83817015fda6f27c82fd Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 4 Mar 2019 10:28:05 -0800 Subject: [PATCH 20/64] Fix if statement from `=` to `==` --- plugins/providers/docker/action/network.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/providers/docker/action/network.rb b/plugins/providers/docker/action/network.rb index 557a9cf68..76124b37f 100644 --- a/plugins/providers/docker/action/network.rb +++ b/plugins/providers/docker/action/network.rb @@ -17,7 +17,7 @@ module VagrantPlugins # Splits the networking options to generate the proper CLI flags for docker options.each do |opt, value| opt = opt.to_s - if opt == "ip" || (opt == "type" && value = "dhcp") || + if opt == "ip" || (opt == "type" && value == "dhcp") || opt == "protocol" || opt == "id" # `docker network create` doesn't care about these options next From 81fa7036beb7f848cb30816c19ab27d7f0c50468 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 4 Mar 2019 10:28:49 -0800 Subject: [PATCH 21/64] Rename cli argument method --- plugins/providers/docker/action/network.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/providers/docker/action/network.rb b/plugins/providers/docker/action/network.rb index 76124b37f..2708a00b5 100644 --- a/plugins/providers/docker/action/network.rb +++ b/plugins/providers/docker/action/network.rb @@ -11,7 +11,7 @@ module VagrantPlugins # @param[Hash] options - options from the network config # @returns[Array] cli_opts - an array of strings used for the network commnad - def parse_cli_arguments(options) + def generate_connect_cli_arguments(options) cli_opts = [] # Splits the networking options to generate the proper CLI flags for docker @@ -43,7 +43,7 @@ module VagrantPlugins # We only handle private networks next if type != :private_network - cli_opts = parse_cli_arguments(options) + cli_opts = generate_connect_cli_arguments(options) if options[:subnet] network_name = "vagrant_network_#{options[:subnet]}" From 0b28580105ef429bf8dff5efa521613c03ae4383 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 4 Mar 2019 10:33:24 -0800 Subject: [PATCH 22/64] Move out cli argument generation to own function --- plugins/providers/docker/action/network.rb | 25 +++++++++++++++------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/plugins/providers/docker/action/network.rb b/plugins/providers/docker/action/network.rb index 2708a00b5..0627a156f 100644 --- a/plugins/providers/docker/action/network.rb +++ b/plugins/providers/docker/action/network.rb @@ -11,7 +11,7 @@ module VagrantPlugins # @param[Hash] options - options from the network config # @returns[Array] cli_opts - an array of strings used for the network commnad - def generate_connect_cli_arguments(options) + def generate_create_cli_arguments(options) cli_opts = [] # Splits the networking options to generate the proper CLI flags for docker @@ -29,6 +29,20 @@ module VagrantPlugins return cli_opts end + # @param[Hash] options - options from the network config + # @returns[Array] cli_opts - an array of strings used for the network commnad + def generate_connect_cli_arguments(options) + cli_opts = [] + + if options[:ip] + cli_opts = ["--ip", options[:ip]] + elsif options[:ip6] + cli_opts = ["--ip6", options[:ip6]] + end + + return cli_opts + end + def call(env) # If we are using a host VM, then don't worry about it machine = env[:machine] @@ -43,7 +57,7 @@ module VagrantPlugins # We only handle private networks next if type != :private_network - cli_opts = generate_connect_cli_arguments(options) + cli_opts = generate_create_cli_arguments(options) if options[:subnet] network_name = "vagrant_network_#{options[:subnet]}" @@ -63,12 +77,7 @@ module VagrantPlugins end @logger.debug("Connecting network #{network_name} to container guest #{machine.name}") - connect_opts = [] - if options[:ip] - connect_opts = ["--ip", options[:ip]] - elsif options[:ip6] - connect_opts = ["--ip6", options[:ip6]] - end + connect_opts = generate_connect_cli_arguments(options) machine.provider.driver.connect_network(network_name, container_id, connect_opts) end From a1b48ed1bb148eaae8414af52ac2a9f259d3d593 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 4 Mar 2019 13:17:14 -0800 Subject: [PATCH 23/64] Raise proper class if invalid options given for docker network --- plugins/providers/docker/action/network.rb | 3 +-- plugins/providers/docker/errors.rb | 4 ++++ templates/locales/providers_docker.yml | 3 +++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/plugins/providers/docker/action/network.rb b/plugins/providers/docker/action/network.rb index 0627a156f..ece840310 100644 --- a/plugins/providers/docker/action/network.rb +++ b/plugins/providers/docker/action/network.rb @@ -64,8 +64,7 @@ module VagrantPlugins elsif options[:type] == "dhcp" network_name = "vagrant_network" else - # TODO: Make this an error class - raise "Must specify a `subnet` or use `dhcp`" + raise Errors::NetworkInvalidOption, container: machine.name end container_id = machine.id diff --git a/plugins/providers/docker/errors.rb b/plugins/providers/docker/errors.rb index 36e1b9859..ffd085427 100644 --- a/plugins/providers/docker/errors.rb +++ b/plugins/providers/docker/errors.rb @@ -45,6 +45,10 @@ module VagrantPlugins error_key(:docker_provider_nfs_without_privileged) end + class NetworkInvalidOption < DockerError + error_key(:network_invalid_option) + end + class PackageNotSupported < DockerError error_key(:package_not_supported) end diff --git a/templates/locales/providers_docker.yml b/templates/locales/providers_docker.yml index ba90473c6..d6f805407 100644 --- a/templates/locales/providers_docker.yml +++ b/templates/locales/providers_docker.yml @@ -197,6 +197,9 @@ en: is functional and properly configured. Host VM ID: %{id} + network_invalid_option: |- + Invalid option given for docker network for guest "%{container}". Must specify either + a `subnet` or use `type: "dhcp"`. package_not_supported: |- The "package" command is not supported with the Docker provider. If you'd like to commit or push your Docker container, please SSH From efb9fd7b65f8614667c329fd4cecdfd89a5ad701 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 4 Mar 2019 13:33:08 -0800 Subject: [PATCH 24/64] Move strings into locales file --- plugins/providers/docker/action/destroy_network.rb | 2 +- plugins/providers/docker/action/network.rb | 2 +- templates/locales/providers_docker.yml | 4 ++++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/plugins/providers/docker/action/destroy_network.rb b/plugins/providers/docker/action/destroy_network.rb index 4a0b2943d..0a04a1cf6 100644 --- a/plugins/providers/docker/action/destroy_network.rb +++ b/plugins/providers/docker/action/destroy_network.rb @@ -29,7 +29,7 @@ module VagrantPlugins if machine.provider.driver.existing_network?(network_name) && !machine.provider.driver.network_used?(network_name) - env[:ui].info("Removing network #{network_name}") + env[:ui].info(I18n.t("docker_provider.network_destroy", network_name: network_name)) machine.provider.driver.rm_network(network_name) else @logger.debug("Network #{network_name} not found") diff --git a/plugins/providers/docker/action/network.rb b/plugins/providers/docker/action/network.rb index ece840310..68e946ac4 100644 --- a/plugins/providers/docker/action/network.rb +++ b/plugins/providers/docker/action/network.rb @@ -51,7 +51,7 @@ module VagrantPlugins return @app.call(env) end - env[:ui].info("Configuring and enabling network interfaces...") + env[:ui].info(I18n.t("docker_provider.network_configure")) machine.config.vm.networks.each do |type, options| # We only handle private networks diff --git a/templates/locales/providers_docker.yml b/templates/locales/providers_docker.yml index d6f805407..0ac94c16a 100644 --- a/templates/locales/providers_docker.yml +++ b/templates/locales/providers_docker.yml @@ -45,6 +45,10 @@ en: This container requires a host VM, and the state of that VM is unknown. Run `vagrant up` to verify that the container and its host VM is running, then try again. + network_configure: |- + Configuring and enabling network interfaces... + network_destroy: |- + Removing network %{network_name} ... not_created_skip: |- Container not created. Skipping. not_docker_provider: |- From 953a380371ad03d4a7ae51ea8de76b9e05af6068 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 4 Mar 2019 13:44:21 -0800 Subject: [PATCH 25/64] Fix how Vagrant assigns cli arguments for the create command This commit inlines the flag assignments so that arguments are properly assigned to flags rather than arguments to the subcommand. --- plugins/providers/docker/action/network.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/providers/docker/action/network.rb b/plugins/providers/docker/action/network.rb index 68e946ac4..de5afca57 100644 --- a/plugins/providers/docker/action/network.rb +++ b/plugins/providers/docker/action/network.rb @@ -22,7 +22,7 @@ module VagrantPlugins # `docker network create` doesn't care about these options next else - cli_opts.concat(["--#{opt}", value]) + cli_opts.concat(["--#{opt}=#{value.to_s}"]) end end From cccbedf4cea0ec8e8ee2b601ec6d7ca40abd2b71 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 4 Mar 2019 14:19:40 -0800 Subject: [PATCH 26/64] Update how docker network handles processing options to cli arguments Add an "ignored option" array rather than a big if-statement expression --- plugins/providers/docker/action/network.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/plugins/providers/docker/action/network.rb b/plugins/providers/docker/action/network.rb index de5afca57..d95a14ab1 100644 --- a/plugins/providers/docker/action/network.rb +++ b/plugins/providers/docker/action/network.rb @@ -13,12 +13,12 @@ module VagrantPlugins # @returns[Array] cli_opts - an array of strings used for the network commnad def generate_create_cli_arguments(options) cli_opts = [] + ignored_options = ["ip", "protocol", "id", "alias"] # Splits the networking options to generate the proper CLI flags for docker options.each do |opt, value| opt = opt.to_s - if opt == "ip" || (opt == "type" && value == "dhcp") || - opt == "protocol" || opt == "id" + if (opt == "type" && value == "dhcp") || ignored_options.include?(opt) # `docker network create` doesn't care about these options next else @@ -40,6 +40,10 @@ module VagrantPlugins cli_opts = ["--ip6", options[:ip6]] end + if options[:alias] + cli_opts.concat(["--alias=#{options[:alias]}"]) + end + return cli_opts end From 2be0bc2d81ae49ff542bcdcd47e3bd82bc2376bd Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 5 Mar 2019 09:49:15 -0800 Subject: [PATCH 27/64] Add unit tests for docker network actions --- .../docker/action/destroy_network_test.rb | 99 +++++++++++++ .../providers/docker/action/network_test.rb | 132 ++++++++++++++++++ 2 files changed, 231 insertions(+) create mode 100644 test/unit/plugins/providers/docker/action/destroy_network_test.rb create mode 100644 test/unit/plugins/providers/docker/action/network_test.rb diff --git a/test/unit/plugins/providers/docker/action/destroy_network_test.rb b/test/unit/plugins/providers/docker/action/destroy_network_test.rb new file mode 100644 index 000000000..54e3624a5 --- /dev/null +++ b/test/unit/plugins/providers/docker/action/destroy_network_test.rb @@ -0,0 +1,99 @@ +require_relative "../../../../base" +require_relative "../../../../../../plugins/providers/docker/action/destroy_network" + +describe VagrantPlugins::DockerProvider::Action::DestroyNetwork do + include_context "unit" + + let(:sandbox) { isolated_environment } + + let(:iso_env) do + # We have to create a Vagrantfile so there is a root path + sandbox.vagrantfile("") + sandbox.create_vagrant_env + end + + let(:machine) do + iso_env.machine(iso_env.machine_names[0], :docker).tap do |m| + allow(m.provider).to receive(:driver).and_return(driver) + allow(m.config.vm).to receive(:networks).and_return(networks) + end + end + + let(:env) {{ machine: machine, ui: machine.ui, root_path: Pathname.new(".") }} + let(:app) { lambda { |*args| }} + let(:driver) { double("driver", create: "abcd1234") } + + let(:networks) { [[:private_network, + {:ip=>"172.20.128.2", + :subnet=>"172.20.0.0/16", + :driver=>"bridge", + :internal=>"true", + :alias=>"mynetwork", + :protocol=>"tcp", + :id=>"80e017d5-388f-4a2f-a3de-f8dce8156a58"}], + [:private_network, + {:type=>"dhcp", + :ipv6=>"true", + :subnet=>"2a02:6b8:b010:9020:1::/80", + :protocol=>"tcp", + :id=>"b8f23054-38d5-45c3-99ea-d33fc5d1b9f2"}], + [:forwarded_port, + {:guest=>22, :host=>2200, :host_ip=>"127.0.0.1", :id=>"ssh", :auto_correct=>true, :protocol=>"tcp"}]] + } + + subject { described_class.new(app, env) } + + after do + sandbox.close + end + + describe "#call" do + it "calls the next action in the chain" do + allow(driver).to receive(:host_vm?).and_return(false) + allow(driver).to receive(:existing_network?).and_return(true) + allow(driver).to receive(:network_used?).and_return(true) + + called = false + app = ->(*args) { called = true } + + action = described_class.new(app, env) + action.call(env) + + expect(called).to eq(true) + end + + it "calls the proper driver method to destroy the network" do + allow(driver).to receive(:host_vm?).and_return(false) + allow(driver).to receive(:existing_network?).with("vagrant_network_172.20.0.0/16"). + and_return(true) + allow(driver).to receive(:network_used?).with("vagrant_network_172.20.0.0/16"). + and_return(false) + allow(driver).to receive(:existing_network?).with("vagrant_network_2a02:6b8:b010:9020:1::/80"). + and_return(true) + allow(driver).to receive(:network_used?).with("vagrant_network_2a02:6b8:b010:9020:1::/80"). + and_return(false) + + expect(driver).to receive(:rm_network).with("vagrant_network_172.20.0.0/16") + expect(driver).to receive(:rm_network).with("vagrant_network_2a02:6b8:b010:9020:1::/80") + + subject.call(env) + end + + it "doesn't destroy the network if another container is still using it" do + allow(driver).to receive(:host_vm?).and_return(false) + allow(driver).to receive(:existing_network?).with("vagrant_network_172.20.0.0/16"). + and_return(true) + allow(driver).to receive(:network_used?).with("vagrant_network_172.20.0.0/16"). + and_return(true) + allow(driver).to receive(:existing_network?).with("vagrant_network_2a02:6b8:b010:9020:1::/80"). + and_return(true) + allow(driver).to receive(:network_used?).with("vagrant_network_2a02:6b8:b010:9020:1::/80"). + and_return(true) + + expect(driver).not_to receive(:rm_network).with("vagrant_network_172.20.0.0/16") + expect(driver).not_to receive(:rm_network).with("vagrant_network_2a02:6b8:b010:9020:1::/80") + + subject.call(env) + end + end +end diff --git a/test/unit/plugins/providers/docker/action/network_test.rb b/test/unit/plugins/providers/docker/action/network_test.rb new file mode 100644 index 000000000..2af865f27 --- /dev/null +++ b/test/unit/plugins/providers/docker/action/network_test.rb @@ -0,0 +1,132 @@ +require_relative "../../../../base" +require_relative "../../../../../../plugins/providers/docker/action/network" + +describe VagrantPlugins::DockerProvider::Action::Network do + include_context "unit" + include_context "virtualbox" + + let(:sandbox) { isolated_environment } + + let(:iso_env) do + # We have to create a Vagrantfile so there is a root path + sandbox.vagrantfile("") + sandbox.create_vagrant_env + end + + let(:machine) do + iso_env.machine(iso_env.machine_names[0], :docker).tap do |m| + allow(m.provider).to receive(:driver).and_return(driver) + allow(m.config.vm).to receive(:networks).and_return(networks) + end + end + + let(:env) {{ machine: machine, ui: machine.ui, root_path: Pathname.new(".") }} + let(:app) { lambda { |*args| }} + let(:driver) { double("driver", create: "abcd1234") } + + let(:networks) { [[:private_network, + {:ip=>"172.20.128.2", + :subnet=>"172.20.0.0/16", + :driver=>"bridge", + :internal=>"true", + :alias=>"mynetwork", + :protocol=>"tcp", + :id=>"80e017d5-388f-4a2f-a3de-f8dce8156a58"}], + [:private_network, + {:type=>"dhcp", + :ipv6=>"true", + :subnet=>"2a02:6b8:b010:9020:1::/80", + :protocol=>"tcp", + :id=>"b8f23054-38d5-45c3-99ea-d33fc5d1b9f2"}], + [:forwarded_port, + {:guest=>22, :host=>2200, :host_ip=>"127.0.0.1", :id=>"ssh", :auto_correct=>true, :protocol=>"tcp"}]] + } + + let(:invalid_network) { + [[:private_network, + {:ipv6=>"true", + :protocol=>"tcp", + :id=>"b8f23054-38d5-45c3-99ea-d33fc5d1b9f2"}]] + } + + subject { described_class.new(app, env) } + + after do + sandbox.close + end + + describe "#call" do + it "calls the next action in the chain" do + allow(driver).to receive(:host_vm?).and_return(false) + allow(driver).to receive(:existing_network?).and_return(false) + allow(driver).to receive(:create_network).and_return(true) + allow(driver).to receive(:connect_network).and_return(true) + + called = false + app = ->(*args) { called = true } + + action = described_class.new(app, env) + action.call(env) + + expect(called).to eq(true) + end + + it "calls the proper driver methods to setup a network" do + allow(driver).to receive(:host_vm?).and_return(false) + allow(driver).to receive(:existing_network?).and_return(false) + allow(driver).to receive(:create_network).and_return(true) + allow(driver).to receive(:connect_network).and_return(true) + + expect(subject).to receive(:generate_create_cli_arguments). + with(networks[0][1]).and_return(["--subnet=172.20.0.0/16", "--driver=bridge", "--internal=true"]) + expect(subject).to receive(:generate_create_cli_arguments). + with(networks[1][1]).and_return(["--ipv6=true", "--subnet=2a02:6b8:b010:9020:1::/80"]) + expect(subject).to receive(:generate_connect_cli_arguments). + with(networks[0][1]).and_return(["--ipv6=true", "--subnet=2a02:6b8:b010:9020:1::/80"]) + expect(subject).to receive(:generate_connect_cli_arguments). + with(networks[1][1]).and_return([]) + + subject.call(env) + end + + it "raises an error if an inproper network configuration is given" do + allow(machine.config.vm).to receive(:networks).and_return(invalid_network) + allow(driver).to receive(:host_vm?).and_return(false) + allow(driver).to receive(:existing_network?).and_return(false) + + expect{ subject.call(env) }.to raise_error(VagrantPlugins::DockerProvider::Errors::NetworkInvalidOption) + end + end + + describe "#generate_connect_cli_arguments" do + let(:network_options) { + {:ip=>"172.20.128.2", + :subnet=>"172.20.0.0/16", + :driver=>"bridge", + :internal=>"true", + :alias=>"mynetwork", + :protocol=>"tcp", + :id=>"80e017d5-388f-4a2f-a3de-f8dce8156a58"} } + + it "returns an array of cli arguments" do + cli_args = subject.generate_connect_cli_arguments(network_options) + expect(cli_args).to eq(["--ip", "172.20.128.2", "--alias=mynetwork"]) + end + end + + describe "#generate_create_cli_arguments" do + let(:network_options) { + {:ip=>"172.20.128.2", + :subnet=>"172.20.0.0/16", + :driver=>"bridge", + :internal=>"true", + :alias=>"mynetwork", + :protocol=>"tcp", + :id=>"80e017d5-388f-4a2f-a3de-f8dce8156a58"} } + + it "returns an array of cli arguments" do + cli_args = subject.generate_create_cli_arguments(network_options) + expect(cli_args).to eq(["--subnet=172.20.0.0/16", "--driver=bridge", "--internal=true"]) + end + end +end From b78dada2c716e3a32a85f7470979f9770b1cdb7a Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 5 Mar 2019 10:57:05 -0800 Subject: [PATCH 28/64] Fix docker driver handling cli flags Make opts argument set to nil instead of splat to make Travis Ruby 2.3 happy --- plugins/providers/docker/driver.rb | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/plugins/providers/docker/driver.rb b/plugins/providers/docker/driver.rb index 4e73641f4..770490ed2 100644 --- a/plugins/providers/docker/driver.rb +++ b/plugins/providers/docker/driver.rb @@ -180,23 +180,23 @@ module VagrantPlugins # @param [String] network - name of network to connect conatiner to # @param [String] cid - container id # @param [Array] opts - An array of flags used for listing networks - def connect_network(network, cid, *opts) - command = ['docker', 'network', 'connect', network, cid].concat(*opts) + def connect_network(network, cid, opts=nil) + command = ['docker', 'network', 'connect', network, cid].push(*opts) output = execute(*command) output end # @param [String] network - name of network to create # @param [Array] opts - An array of flags used for listing networks - def create_network(network, *opts) - command = ['docker', 'network', 'create', network].concat(*opts) + def create_network(network, opts=nil) + command = ['docker', 'network', 'create', network].push(*opts) output = execute(*command) output end # @param [String] network - name of network to disconnect container from # @param [String] cid - container id - def disconnect_network(network, cid, *opts) + def disconnect_network(network, cid) command = ['docker', 'network', 'disconnect', network, cid, "--force"] output = execute(*command) output @@ -204,15 +204,15 @@ module VagrantPlugins # @param [Array] networks - list of networks to inspect # @param [Array] opts - An array of flags used for listing networks - def inspect_network(network, *opts) - command = ['docker', 'network', 'inspect', network].concat(*opts) + def inspect_network(network, opts=nil) + command = ['docker', 'network', 'inspect', network].push(*opts) output = execute(*command) output end # @param [Array] opts - An array of flags used for listing networks - def list_network(*opts) - command = ['docker', 'network', 'ls'].concat(*opts) + def list_network(opts=nil) + command = ['docker', 'network', 'ls'].push(*opts) output = execute(*command) output end @@ -221,8 +221,8 @@ module VagrantPlugins # networks not created by Vagrant. # # @param [Array] opts - An array of flags used for listing networks - def prune_network(*opts) - command = ['docker', 'network', 'prune', '--force'].concat(*opts) + def prune_network(opts=nil) + command = ['docker', 'network', 'prune', '--force'].push(*opts) output = execute(*command) output end From 93f833d2b347ab732216147fd8ae7638eae11e20 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 5 Mar 2019 11:14:50 -0800 Subject: [PATCH 29/64] Add docs page about networking with the docker provider --- website/source/docs/docker/basics.html.md | 2 - website/source/docs/docker/networking.html.md | 193 ++++++++++++++++++ website/source/layouts/docs.erb | 1 + 3 files changed, 194 insertions(+), 2 deletions(-) create mode 100644 website/source/docs/docker/networking.html.md diff --git a/website/source/docs/docker/basics.html.md b/website/source/docs/docker/basics.html.md index e0930b1c2..a38234c5e 100644 --- a/website/source/docs/docker/basics.html.md +++ b/website/source/docs/docker/basics.html.md @@ -73,8 +73,6 @@ This helps keep your Vagrantfile similar to how it has always looked. The Docker provider does not support specifying options for `owner` or `group` on folders synced with a docker container. -Private and public networks are not currently supported. - ### Volume Consistency Docker's [volume consistency](https://docs.docker.com/v17.09/engine/admin/volumes/bind-mounts/) setting can be specified using the `docker_consistency` option when defining a synced folder. This can diff --git a/website/source/docs/docker/networking.html.md b/website/source/docs/docker/networking.html.md new file mode 100644 index 000000000..59452536b --- /dev/null +++ b/website/source/docs/docker/networking.html.md @@ -0,0 +1,193 @@ +--- +layout: "docs" +page_title: "Networking - Docker Provider" +sidebar_current: "providers-docker-networking" +description: |- + The Vagrant Docker provider supports using the private network using the + `docker network` commands. +--- + +# Networking + +Vagrant uses the `docker network` command under the hood to create and manage +networks for containers. Vagrant will do its best to create and manage networks +for any containers configured inside the Vagrantfile. Each docker network is grouped +by the subnet used for a requested ip address. + +For each newly unique network, Vagrant will run the `docker network create` subcommand +with the provided options from the network config inside your Vagrantfile. If multiple +networks share the same subnet, it will reuse that existing network. Once these +networks have been created, Vagrant will attach these networks to the requested +containers using the `docker network connect` for each network. + +Most of the options given to `:private_network` align with the command line flags +for the [docker network create](https://docs.docker.com/engine/reference/commandline/network_create/) +command. However, if you want the container to have a specific IP instead of using +DHCP, you also will have to specify a subnet due to how docker networks behave. + +It should also be noted that if you want a specific IPv6 address, your `:private_network` +option should use `ip6` rather than `ip`. If you just want to use DHCP, you can +simply say `type: "dhcp"` insetad. More examples are shared below which demonstrate +creating a few common network interfaces. + +When destroying containers through Vagrant, Vagrant will clean up the network if +there are no more containers using the network. + +## Docker Network Example + +The following Vagrantfile will generate these networks for a container: + +1. A IPv4 IP address assigned by DHCP +2. A IPv4 IP address 172.20.128.2 on a network with subnet 172.20.0.0/16 +3. A IPv6 IP address assigned by DHCP on subnet 2a02:6b8:b010:9020:1::/80 + +```ruby +Vagrant.configure("2") do |config| + config.vm.define "docker" do |docker| + docker.vm.network :private_network, type: "dhcp" + docker.vm.network :private_network, + ip: "172.20.128.2", subnet: "172.20.0.0/16" + docker.vm.network :private_network, type: "dhcp", ipv6: "true", subnet: "2a02:6b8:b010:9020:1::/80" + docker.vm.provider "docker" do |d| + d.build_dir = "docker_build_dir" + end + end +end +``` + +You can test that your container has the proper configured networks by looking +at the result of running `ip addr`, for example: + +``` +brian@localghost:vagrant-sandbox % docker ps ±[●][master] +CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES +370f4e5d2217 196a06ef12f5 "tail -f /dev/null" 5 seconds ago Up 3 seconds 80/tcp, 443/tcp vagrant-sandbox_docker-1_1551810440 +brian@localghost:vagrant-sandbox % docker exec 370f4e5d2217 ip addr ±[●][master] +1: lo: mtu 65536 qdisc noqueue state UNKNOWN group default qlen 1000 + link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00 + inet 127.0.0.1/8 scope host lo + valid_lft forever preferred_lft forever + inet6 ::1/128 scope host + valid_lft forever preferred_lft forever +24: eth0@if25: mtu 1500 qdisc noqueue state UP group default + link/ether 02:42:ac:11:00:03 brd ff:ff:ff:ff:ff:ff link-netnsid 0 + inet 172.17.0.3/16 brd 172.17.255.255 scope global eth0 + valid_lft forever preferred_lft forever +27: eth1@if28: mtu 1500 qdisc noqueue state UP group default + link/ether 02:42:ac:13:00:02 brd ff:ff:ff:ff:ff:ff link-netnsid 0 + inet 172.19.0.2/16 brd 172.19.255.255 scope global eth1 + valid_lft forever preferred_lft forever +30: eth2@if31: mtu 1500 qdisc noqueue state UP group default + link/ether 02:42:ac:14:80:02 brd ff:ff:ff:ff:ff:ff link-netnsid 0 + inet 172.20.128.2/16 brd 172.20.255.255 scope global eth2 + valid_lft forever preferred_lft forever +33: eth3@if34: mtu 1500 qdisc noqueue state UP group default + link/ether 02:42:ac:15:00:02 brd ff:ff:ff:ff:ff:ff link-netnsid 0 + inet 172.21.0.2/16 brd 172.21.255.255 scope global eth3 + valid_lft forever preferred_lft forever + inet6 2a02:6b8:b010:9020:1::2/80 scope global nodad + valid_lft forever preferred_lft forever + inet6 fe80::42:acff:fe15:2/64 scope link + valid_lft forever preferred_lft forever +``` + +## Useful Debugging Tips + +If you provide Vagrant with a faulty config option when setting up a network, Vagrant +will pass that option along to the `docker network` commands it uses. That command +line tool should give you some insight if there is something wrong with the option +you configured: + +```ruby +docker.vm.network :private_network, + ip: "172.20.128.2", subnet: "172.20.0.0/16", + unsupported: "option" +``` + +``` +A Docker command executed by Vagrant didn't complete successfully! +The command run along with the output from the command is shown +below. + +Command: ["docker", "network", "create", "vagrant_network_172.20.0.0/16", "--subnet=172.20.0.0/16", "--unsupported=option", {:notify=>[:stdout, :stderr]}] + +Stderr: unknown flag: --unsupported +See 'docker network create --help'. + + +Stdout: +``` + +The `docker network` command provides some helpful insights to what might be going +on with the networks Vagrant creates. For example, if you want to know what networks +you currently have running on your machine, you can run the `docker network ls` command: + +``` +brian@localghost:vagrant-sandbox % docker network ls ±[●][master] +NETWORK ID NAME DRIVER SCOPE +a2bfc26bd876 bridge bridge local +2a2845e77550 host host local +f36682aeba68 none null local +00d4986c7dc2 vagrant_network bridge local +d02420ff4c39 vagrant_network_2a02:6b8:b010:9020:1::/80 bridge local +799ae9dbaf98 vagrant_network_172.20.0.0/16 bridge local +``` + +You can also inspect any network for more information: + +``` +brian@localghost:vagrant-sandbox % docker network inspect vagrant_network ±[●][master] +[ + { + "Name": "vagrant_network", + "Id": "00d4986c7dc2ed7bf1961989ae1cfe98504c711f9de2f547e5dfffe2bb819fc2", + "Created": "2019-03-05T10:27:21.558824922-08:00", + "Scope": "local", + "Driver": "bridge", + "EnableIPv6": false, + "IPAM": { + "Driver": "default", + "Options": {}, + "Config": [ + { + "Subnet": "172.19.0.0/16", + "Gateway": "172.19.0.1" + } + ] + }, + "Internal": false, + "Attachable": false, + "Ingress": false, + "ConfigFrom": { + "Network": "" + }, + "ConfigOnly": false, + "Containers": { + "370f4e5d2217e698b16376583fbf051dd34018e5fd18958b604017def92fea63": { + "Name": "vagrant-sandbox_docker-1_1551810440", + "EndpointID": "166b7ca8960a9f20a150bb75a68d07e27e674781ed9f916e9aa58c8bc2539a61", + "MacAddress": "02:42:ac:13:00:02", + "IPv4Address": "172.19.0.2/16", + "IPv6Address": "" + } + }, + "Options": {}, + "Labels": {} + } +] +``` + +## Caveats + +For now, Vagrant only looks at the subnet when figuring out if it should create +a new network for a guest container. If you bring up a container with a network, +and then change or add some new options (but leave the subnet the same), it will +not apply those changes or create a new network. + +## More Information + +For more information on how docker manages its networks, please refer to their +documentation: + +- https://docs.docker.com/network/ +- https://docs.docker.com/engine/reference/commandline/network/ diff --git a/website/source/layouts/docs.erb b/website/source/layouts/docs.erb index 462b5c509..437a6609a 100644 --- a/website/source/layouts/docs.erb +++ b/website/source/layouts/docs.erb @@ -166,6 +166,7 @@ >Commands >Boxes >Configuration + >Networking > From cedf5aff17c27f868c4a901c19f9d2ea1899868b Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 5 Mar 2019 11:23:23 -0800 Subject: [PATCH 30/64] Add note about link legacy flag in connect --- website/source/docs/docker/networking.html.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/website/source/docs/docker/networking.html.md b/website/source/docs/docker/networking.html.md index 59452536b..b2eddfdfc 100644 --- a/website/source/docs/docker/networking.html.md +++ b/website/source/docs/docker/networking.html.md @@ -184,6 +184,10 @@ a new network for a guest container. If you bring up a container with a network, and then change or add some new options (but leave the subnet the same), it will not apply those changes or create a new network. +Because the `--link` flag for the `docker network connect` command is considered +legacy, Vagrant does not support that option when creating containers and connecting +networks. + ## More Information For more information on how docker manages its networks, please refer to their From 2c25cf8d019fd12f16b9888cce1a9702075b4c0c Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 5 Mar 2019 13:46:05 -0800 Subject: [PATCH 31/64] Add note about network creation in docker provider --- website/source/docs/docker/networking.html.md | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/website/source/docs/docker/networking.html.md b/website/source/docs/docker/networking.html.md index b2eddfdfc..0f3b4b295 100644 --- a/website/source/docs/docker/networking.html.md +++ b/website/source/docs/docker/networking.html.md @@ -16,9 +16,23 @@ by the subnet used for a requested ip address. For each newly unique network, Vagrant will run the `docker network create` subcommand with the provided options from the network config inside your Vagrantfile. If multiple -networks share the same subnet, it will reuse that existing network. Once these -networks have been created, Vagrant will attach these networks to the requested -containers using the `docker network connect` for each network. +networks share the same subnet, Vagrant will reuse that existing network for multiple +containers. Once these networks have been created, Vagrant will attach these +networks to the requested containers using the `docker network connect` for each +network. + +Vagrant names the networks inside docker as `vagrant_network` or `vagrant_network_` +where `` is the subnet for the network if defined by the user. An +example of these networks is shown later in this page. If no subnet is requested +for the network, Vagrant will connect the `vagrant_network` to the container. + +When destroying containers through Vagrant, Vagrant will clean up the network if +there are no more containers using the network. + +## Docker Network Options + +Only the network option `:private_network` is currently supported with the docker +provider in Vagrant. Most of the options given to `:private_network` align with the command line flags for the [docker network create](https://docs.docker.com/engine/reference/commandline/network_create/) @@ -30,9 +44,6 @@ option should use `ip6` rather than `ip`. If you just want to use DHCP, you can simply say `type: "dhcp"` insetad. More examples are shared below which demonstrate creating a few common network interfaces. -When destroying containers through Vagrant, Vagrant will clean up the network if -there are no more containers using the network. - ## Docker Network Example The following Vagrantfile will generate these networks for a container: From 5ed5868067152077661f00eb94fb86ea2b29872a Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 12 Mar 2019 10:36:57 -0700 Subject: [PATCH 32/64] Inspect networks before creating new ones This commit updates the behavior of how the docker provider creates new docker networks. It looks at each existing network to see if the requested subnet has already been configured in the docker engine. If so, Vagrant will use that network rather than creating a new one. This includes networks not created by Vagrant. Vagrant will not clean up these networks if created outside of Vagrant. --- .../docker/action/destroy_network.rb | 1 + plugins/providers/docker/action/network.rb | 25 ++++++++++++---- plugins/providers/docker/driver.rb | 29 +++++++++++++++++-- templates/locales/providers_docker.yml | 3 ++ .../providers/docker/action/network_test.rb | 18 ++++++++++++ 5 files changed, 67 insertions(+), 9 deletions(-) diff --git a/plugins/providers/docker/action/destroy_network.rb b/plugins/providers/docker/action/destroy_network.rb index 0a04a1cf6..b6feac35e 100644 --- a/plugins/providers/docker/action/destroy_network.rb +++ b/plugins/providers/docker/action/destroy_network.rb @@ -27,6 +27,7 @@ module VagrantPlugins network_name = "vagrant_network" end + # Only cleans up networks defined by Vagrant if machine.provider.driver.existing_network?(network_name) && !machine.provider.driver.network_used?(network_name) env[:ui].info(I18n.t("docker_provider.network_destroy", network_name: network_name)) diff --git a/plugins/providers/docker/action/network.rb b/plugins/providers/docker/action/network.rb index d95a14ab1..f82f8642b 100644 --- a/plugins/providers/docker/action/network.rb +++ b/plugins/providers/docker/action/network.rb @@ -7,6 +7,7 @@ module VagrantPlugins def initialize(app, env) @app = app @logger = Log4r::Logger.new('vagrant::plugins::docker::network') + @@lock = Mutex.new end # @param[Hash] options - options from the network config @@ -64,7 +65,15 @@ module VagrantPlugins cli_opts = generate_create_cli_arguments(options) if options[:subnet] - network_name = "vagrant_network_#{options[:subnet]}" + existing_network = machine.provider.driver.subnet_defined?(options[:subnet]) + if !existing_network + network_name = "vagrant_network_#{options[:subnet]}" + else + env[:ui].warn(I18n.t("docker_provider.subnet_exists", + network_name: existing_network, + subnet: options[:subnet])) + network_name = existing_network + end elsif options[:type] == "dhcp" network_name = "vagrant_network" else @@ -72,11 +81,15 @@ module VagrantPlugins end container_id = machine.id - if !machine.provider.driver.existing_network?(network_name) - @logger.debug("Creating network #{network_name}") - machine.provider.driver.create_network(network_name, cli_opts) - else - @logger.debug("Network #{network_name} already created") + @@lock.synchronize do + machine.env.lock("docker-network-create", retry: true) do + if !machine.provider.driver.existing_network?(network_name) + @logger.debug("Creating network #{network_name}") + machine.provider.driver.create_network(network_name, cli_opts) + else + @logger.debug("Network #{network_name} already created") + end + end end @logger.debug("Connecting network #{network_name} to container guest #{machine.name}") diff --git a/plugins/providers/docker/driver.rb b/plugins/providers/docker/driver.rb index 770490ed2..2fb959e16 100644 --- a/plugins/providers/docker/driver.rb +++ b/plugins/providers/docker/driver.rb @@ -205,7 +205,8 @@ module VagrantPlugins # @param [Array] networks - list of networks to inspect # @param [Array] opts - An array of flags used for listing networks def inspect_network(network, opts=nil) - command = ['docker', 'network', 'inspect', network].push(*opts) + command = ['docker', 'network', 'inspect'] + Array(network) + command = command.push(*opts) output = execute(*command) output end @@ -246,13 +247,35 @@ module VagrantPlugins # Docker network helpers # ###################### + # @param [String] subnet_string - Subnet to look for + def subnet_defined?(subnet_string) + all_networks = list_network(["--format={{.Name}}"]) + all_networks = all_networks.split("\n") + + results = inspect_network(all_networks) + begin + networks_info = JSON.parse(results) + networks_info.each do |network| + config = network["IPAM"]["Config"] + if (config.size > 0 && + config.first["Subnet"] == subnet_string) + @logger.debug("Found existing network #{network["Name"]} already configured with #{subnet_string}") + return network["Name"] + end + end + rescue JSON::ParserError => e + @logger.warn("Could not properly parse response from `docker network inspect #{all_networks.join(" ")}`") + end + return nil + end + # Looks to see if a docker network has already been defined # # @param [String] network - name of network to look for def existing_network?(network) - result = list_network(["--format='{{json .Name}}'"]) + result = list_network(["--format={{.Name}}"]) #TODO: we should be more explicit here if we can - result.match?(/\"#{network}\"/) + result.match?(/#{network}/) end # Returns true or false if network is in use or not. diff --git a/templates/locales/providers_docker.yml b/templates/locales/providers_docker.yml index 0ac94c16a..678307f7c 100644 --- a/templates/locales/providers_docker.yml +++ b/templates/locales/providers_docker.yml @@ -70,6 +70,9 @@ en: ssh_through_host_vm: |- SSH will be proxied through the Docker virtual machine since we're not running Docker natively. This is just a notice, and not an error. + subnet_exists: |- + A network called '%{network_name}' using subnet '%{subnet}' is already in use. + Using '%{network_name}' instead of creating a new network... synced_folders_changed: |- Vagrant has noticed that the synced folder definitions have changed. With Docker, these synced folder changes won't take effect until you diff --git a/test/unit/plugins/providers/docker/action/network_test.rb b/test/unit/plugins/providers/docker/action/network_test.rb index 2af865f27..c3d09e726 100644 --- a/test/unit/plugins/providers/docker/action/network_test.rb +++ b/test/unit/plugins/providers/docker/action/network_test.rb @@ -61,6 +61,7 @@ describe VagrantPlugins::DockerProvider::Action::Network do allow(driver).to receive(:existing_network?).and_return(false) allow(driver).to receive(:create_network).and_return(true) allow(driver).to receive(:connect_network).and_return(true) + allow(driver).to receive(:subnet_defined?).and_return(nil) called = false app = ->(*args) { called = true } @@ -76,6 +77,8 @@ describe VagrantPlugins::DockerProvider::Action::Network do allow(driver).to receive(:existing_network?).and_return(false) allow(driver).to receive(:create_network).and_return(true) allow(driver).to receive(:connect_network).and_return(true) + allow(driver).to receive(:subnet_defined?).and_return(nil) + expect(subject).to receive(:generate_create_cli_arguments). with(networks[0][1]).and_return(["--subnet=172.20.0.0/16", "--driver=bridge", "--internal=true"]) @@ -86,6 +89,21 @@ describe VagrantPlugins::DockerProvider::Action::Network do expect(subject).to receive(:generate_connect_cli_arguments). with(networks[1][1]).and_return([]) + expect(driver).to receive(:create_network).twice + expect(driver).to receive(:connect_network).twice + + subject.call(env) + end + + it "uses an existing network if a matching subnet is found" do + allow(driver).to receive(:host_vm?).and_return(false) + allow(driver).to receive(:existing_network?).and_return(true) + allow(driver).to receive(:create_network).and_return(true) + allow(driver).to receive(:connect_network).and_return(true) + allow(driver).to receive(:subnet_defined?).and_return("my_cool_subnet_network") + + expect(driver).not_to receive(:create_network) + subject.call(env) end From 6664936c0bd3cf2f112623980b8a69c75c57f4b4 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 12 Mar 2019 10:40:58 -0700 Subject: [PATCH 33/64] Make ignored vagrant network options a constant --- plugins/providers/docker/action/network.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/providers/docker/action/network.rb b/plugins/providers/docker/action/network.rb index f82f8642b..16752f67e 100644 --- a/plugins/providers/docker/action/network.rb +++ b/plugins/providers/docker/action/network.rb @@ -14,7 +14,7 @@ module VagrantPlugins # @returns[Array] cli_opts - an array of strings used for the network commnad def generate_create_cli_arguments(options) cli_opts = [] - ignored_options = ["ip", "protocol", "id", "alias"] + ignored_options = ["ip", "protocol", "id", "alias"].map(&:freeze).freeze # Splits the networking options to generate the proper CLI flags for docker options.each do |opt, value| From c251e090b326ac4398859c56f5cd3f2dd13ffe99 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Fri, 15 Mar 2019 08:29:10 -0700 Subject: [PATCH 34/64] Remove Mutex for synchronization. Environment#lock satisfies requirement. --- plugins/providers/docker/action/network.rb | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/plugins/providers/docker/action/network.rb b/plugins/providers/docker/action/network.rb index 16752f67e..80cf0fb82 100644 --- a/plugins/providers/docker/action/network.rb +++ b/plugins/providers/docker/action/network.rb @@ -7,7 +7,6 @@ module VagrantPlugins def initialize(app, env) @app = app @logger = Log4r::Logger.new('vagrant::plugins::docker::network') - @@lock = Mutex.new end # @param[Hash] options - options from the network config @@ -81,14 +80,12 @@ module VagrantPlugins end container_id = machine.id - @@lock.synchronize do - machine.env.lock("docker-network-create", retry: true) do - if !machine.provider.driver.existing_network?(network_name) - @logger.debug("Creating network #{network_name}") - machine.provider.driver.create_network(network_name, cli_opts) - else - @logger.debug("Network #{network_name} already created") - end + machine.env.lock("docker-network-create", retry: true) do + if !machine.provider.driver.existing_network?(network_name) + @logger.debug("Creating network #{network_name}") + machine.provider.driver.create_network(network_name, cli_opts) + else + @logger.debug("Network #{network_name} already created") end end From 1224622387f6f22cc4d53df498ab2236d0fde21a Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Fri, 15 Mar 2019 09:23:27 -0700 Subject: [PATCH 35/64] Remove container inspection caching and Exception rescue Container inspection is caching data on first lookup. This will result in incorrect data being returned on subsequent lookups if a different `cid` value is provided. Also removed rescue of the `Exception` class as this generally should never happen; rescue of StandardError will be enough. --- plugins/providers/docker/driver.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/plugins/providers/docker/driver.rb b/plugins/providers/docker/driver.rb index 2fb959e16..dbe43abf1 100644 --- a/plugins/providers/docker/driver.rb +++ b/plugins/providers/docker/driver.rb @@ -152,15 +152,13 @@ module VagrantPlugins def rmi(id) execute('docker', 'rmi', id) return true - rescue Exception => e + rescue => e return false if e.to_s.include?("is using it") raise if !e.to_s.include?("No such image") end def inspect_container(cid) - # DISCUSS: Is there a chance that this json will change after the container - # has been brought up? - @data ||= JSON.parse(execute('docker', 'inspect', cid)).first + JSON.parse(execute('docker', 'inspect', cid)).first end def all_containers From a645ce3c25ca08072d17d932194847f76cad8772 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Tue, 19 Mar 2019 11:29:16 -0700 Subject: [PATCH 36/64] Docker provider networking support updates Use `mask` option for defining subnet on network configuration. Allow options to be passed through using hash scoping and docker_network and docker_connect prefixes. Enable public networks. Allow configuration to define pre-existing networks by name. --- plugins/providers/docker/action.rb | 12 +- .../docker/action/connect_networks.rb | 75 ++++ .../docker/action/destroy_network.rb | 28 +- plugins/providers/docker/action/network.rb | 102 ------ .../docker/action/prepare_networks.rb | 335 ++++++++++++++++++ plugins/providers/docker/driver.rb | 99 ++++-- plugins/providers/docker/errors.rb | 8 + templates/locales/en.yml | 2 + templates/locales/providers_docker.yml | 53 ++- 9 files changed, 555 insertions(+), 159 deletions(-) create mode 100644 plugins/providers/docker/action/connect_networks.rb delete mode 100644 plugins/providers/docker/action/network.rb create mode 100644 plugins/providers/docker/action/prepare_networks.rb diff --git a/plugins/providers/docker/action.rb b/plugins/providers/docker/action.rb index 68bb78011..b512da387 100644 --- a/plugins/providers/docker/action.rb +++ b/plugins/providers/docker/action.rb @@ -244,6 +244,7 @@ module VagrantPlugins b2.use PrepareNFSValidIds b2.use SyncedFolderCleanup b2.use PrepareNFSSettings + b2.use PrepareNetworks b2.use Login b2.use Build @@ -266,7 +267,7 @@ module VagrantPlugins end end - b2.use Network + b2.use ConnectNetworks b2.use Start b2.use WaitForRunning @@ -294,6 +295,7 @@ module VagrantPlugins action_root = Pathname.new(File.expand_path("../action", __FILE__)) autoload :Build, action_root.join("build") autoload :CompareSyncedFolders, action_root.join("compare_synced_folders") + autoload :ConnectNetworks, action_root.join("connect_networks") autoload :Create, action_root.join("create") autoload :Destroy, action_root.join("destroy") autoload :DestroyBuildImage, action_root.join("destroy_build_image") @@ -311,13 +313,13 @@ module VagrantPlugins autoload :IsBuild, action_root.join("is_build") autoload :IsHostMachineCreated, action_root.join("is_host_machine_created") autoload :Login, action_root.join("login") - autoload :Pull, action_root.join("pull") - autoload :PrepareSSH, action_root.join("prepare_ssh") - autoload :Stop, action_root.join("stop") - autoload :Network, action_root.join("network") + autoload :PrepareNetworks, action_root.join("prepare_networks") autoload :PrepareNFSValidIds, action_root.join("prepare_nfs_valid_ids") autoload :PrepareNFSSettings, action_root.join("prepare_nfs_settings") + autoload :PrepareSSH, action_root.join("prepare_ssh") + autoload :Pull, action_root.join("pull") autoload :Start, action_root.join("start") + autoload :Stop, action_root.join("stop") autoload :WaitForRunning, action_root.join("wait_for_running") end end diff --git a/plugins/providers/docker/action/connect_networks.rb b/plugins/providers/docker/action/connect_networks.rb new file mode 100644 index 000000000..9f29e2031 --- /dev/null +++ b/plugins/providers/docker/action/connect_networks.rb @@ -0,0 +1,75 @@ +require 'ipaddr' +require 'log4r' + +module VagrantPlugins + module DockerProvider + module Action + class ConnectNetworks + + include Vagrant::Util::ScopedHashOverride + + def initialize(app, env) + @app = app + @logger = Log4r::Logger.new('vagrant::plugins::docker::connectnetworks') + end + + # Generate CLI arguments for creating the docker network. + # + # @param [Hash] options Options from the network config + # @returns[Array Network create arguments + def generate_connect_cli_arguments(options) + options.map do |key, value| + # If value is false, option is not set + next if value == false + # If value is true, consider feature flag with no value + opt = value == true ? [] : [value] + opt.unshift("--#{key.to_s.tr("_", "-")}") + end.flatten.compact + end + + # Execute the action + def call(env) + # If we are using a host VM, then don't worry about it + machine = env[:machine] + if machine.provider.host_vm? + @logger.debug("Not setting up networks because docker host_vm is in use") + return @app.call(env) + end + + env[:ui].info(I18n.t("docker_provider.network_connect")) + + connections = env[:docker_connects] || {} + + machine.config.vm.networks.each_with_index do |args, idx| + type, options = args + next if type != :private_network && type != :public_network + + network_options = scoped_hash_override(options, :docker_connect) + network_options.delete_if{|k,_| options.key?(k)} + network_name = connections[idx] + + if !network_name + raise Errors::NetworkNameMissing, + index: idx, + container: machine.name + end + + @logger.debug("Connecting network #{network_name} to container guest #{machine.name}") + if options[:ip] + if IPAddr.new(options[:ip]).ipv4? + network_options[:ip] = options[:ip] + else + network_options[:ip6] = options[:ip] + end + end + network_options[:alias] = options[:alias] if options[:alias] + connect_opts = generate_connect_cli_arguments(network_options) + machine.provider.driver.connect_network(network_name, machine.id, connect_opts) + end + + @app.call(env) + end + end + end + end +end diff --git a/plugins/providers/docker/action/destroy_network.rb b/plugins/providers/docker/action/destroy_network.rb index b6feac35e..90e57fc28 100644 --- a/plugins/providers/docker/action/destroy_network.rb +++ b/plugins/providers/docker/action/destroy_network.rb @@ -18,22 +18,22 @@ module VagrantPlugins end machine.config.vm.networks.each do |type, options| - # We only handle private networks - next if type != :private_network + next if type != :private_network && type != :public_network - if options[:subnet] - network_name = "vagrant_network_#{options[:subnet]}" - else - network_name = "vagrant_network" - end + machine.env.lock("docker-network-destroy", retry: true) do + vagrant_networks = machine.provider.driver.list_network_names.find_all do |n| + n.start_with?("vagrant_network") + end - # Only cleans up networks defined by Vagrant - if machine.provider.driver.existing_network?(network_name) && - !machine.provider.driver.network_used?(network_name) - env[:ui].info(I18n.t("docker_provider.network_destroy", network_name: network_name)) - machine.provider.driver.rm_network(network_name) - else - @logger.debug("Network #{network_name} not found") + vagrant_networks.each do |network_name| + if machine.provider.driver.existing_named_network?(network_name) && + !machine.provider.driver.network_used?(network_name) + env[:ui].info(I18n.t("docker_provider.network_destroy", network_name: network_name)) + machine.provider.driver.rm_network(network_name) + else + @logger.debug("Network #{network_name} not found or in use") + end + end end end diff --git a/plugins/providers/docker/action/network.rb b/plugins/providers/docker/action/network.rb deleted file mode 100644 index 80cf0fb82..000000000 --- a/plugins/providers/docker/action/network.rb +++ /dev/null @@ -1,102 +0,0 @@ -require 'log4r' - -module VagrantPlugins - module DockerProvider - module Action - class Network - def initialize(app, env) - @app = app - @logger = Log4r::Logger.new('vagrant::plugins::docker::network') - end - - # @param[Hash] options - options from the network config - # @returns[Array] cli_opts - an array of strings used for the network commnad - def generate_create_cli_arguments(options) - cli_opts = [] - ignored_options = ["ip", "protocol", "id", "alias"].map(&:freeze).freeze - - # Splits the networking options to generate the proper CLI flags for docker - options.each do |opt, value| - opt = opt.to_s - if (opt == "type" && value == "dhcp") || ignored_options.include?(opt) - # `docker network create` doesn't care about these options - next - else - cli_opts.concat(["--#{opt}=#{value.to_s}"]) - end - end - - return cli_opts - end - - # @param[Hash] options - options from the network config - # @returns[Array] cli_opts - an array of strings used for the network commnad - def generate_connect_cli_arguments(options) - cli_opts = [] - - if options[:ip] - cli_opts = ["--ip", options[:ip]] - elsif options[:ip6] - cli_opts = ["--ip6", options[:ip6]] - end - - if options[:alias] - cli_opts.concat(["--alias=#{options[:alias]}"]) - end - - return cli_opts - end - - def call(env) - # If we are using a host VM, then don't worry about it - machine = env[:machine] - if machine.provider.host_vm? - @logger.debug("Not setting up networks because docker host_vm is in use") - return @app.call(env) - end - - env[:ui].info(I18n.t("docker_provider.network_configure")) - - machine.config.vm.networks.each do |type, options| - # We only handle private networks - next if type != :private_network - - cli_opts = generate_create_cli_arguments(options) - - if options[:subnet] - existing_network = machine.provider.driver.subnet_defined?(options[:subnet]) - if !existing_network - network_name = "vagrant_network_#{options[:subnet]}" - else - env[:ui].warn(I18n.t("docker_provider.subnet_exists", - network_name: existing_network, - subnet: options[:subnet])) - network_name = existing_network - end - elsif options[:type] == "dhcp" - network_name = "vagrant_network" - else - raise Errors::NetworkInvalidOption, container: machine.name - end - container_id = machine.id - - machine.env.lock("docker-network-create", retry: true) do - if !machine.provider.driver.existing_network?(network_name) - @logger.debug("Creating network #{network_name}") - machine.provider.driver.create_network(network_name, cli_opts) - else - @logger.debug("Network #{network_name} already created") - end - end - - @logger.debug("Connecting network #{network_name} to container guest #{machine.name}") - connect_opts = generate_connect_cli_arguments(options) - machine.provider.driver.connect_network(network_name, container_id, connect_opts) - end - - @app.call(env) - end - end - end - end -end diff --git a/plugins/providers/docker/action/prepare_networks.rb b/plugins/providers/docker/action/prepare_networks.rb new file mode 100644 index 000000000..842c69154 --- /dev/null +++ b/plugins/providers/docker/action/prepare_networks.rb @@ -0,0 +1,335 @@ +require 'ipaddr' +require 'log4r' + +module VagrantPlugins + module DockerProvider + module Action + class PrepareNetworks + + include Vagrant::Util::ScopedHashOverride + + def initialize(app, env) + @app = app + @logger = Log4r::Logger.new('vagrant::plugins::docker::preparenetworks') + end + + # Generate CLI arguments for creating the docker network. + # + # @param [Hash] options Options from the network config + # @returns[Array Network create arguments + def generate_create_cli_arguments(options) + options.map do |key, value| + # If value is false, option is not set + next if value == false + # If value is true, consider feature flag with no value + opt = value == true ? [] : [value] + opt.unshift("--#{key.to_s.tr("_", "-")}") + end.flatten.compact + end + + # @return [Array] interface list + def list_interfaces + Socket.getifaddrs.find_all do |i| + i.addr.ip? && !i.addr.ipv4_loopback? && + !i.addr.ipv6_loopback? && !i.addr.ipv6_linklocal? + end + end + + # Validates that a network name exists. If it does not + # exist, an exception is raised. + # + # @param [String] name Name of existing network + # @return [Boolean] + def validate_network_name!(name) + if !env[:machine].provider.driver.existing_named_network?(network_name) + raise Errors::NetworkNameUndefined, + network_name: name + end + true + end + + # Validates that the provided options are compatible with a + # pre-existing network. Raises exceptions on invalid configurations + # + # @param [String] network_name Name of the network + # @param [Hash] root_options Root networking options + # @param [Hash] network_options Docker scoped networking options + # @param [Driver] driver Docker driver + # @return [Boolean] + def validate_network_configuration!(network_name, root_options, network_options, driver) + if root_options[:ip] && + driver.network_containing_address(root_options[:ip]) != network_name + raise Errors::NetworkAddressInvalid, + address: root_options[:ip], + network_name: network_name + end + if network_options[:subnet] && + driver.network_containing_address(network_options[:subnet]) != network_name + raise Errors::NetworkSubnetInvalid, + subnet: network_options[:subnet], + network_name: network_name + end + true + end + + # Generate configuration for private network + # + # @param [Hash] root_options Root networking options + # @param [Hash] net_options Docker scoped networking options + # @param [Hash] env Local call env + # @return [String, Hash] Network name and updated network_options + def process_private_network(root_options, network_options, env) + if root_options[:name] && validate_network_name!(root_options[:name]) + network_name = root_options[:name] + end + + if root_options[:type].to_s == "dhcp" + network_name = "vagrant_network" if !network_name + return [network_name, network_options] + end + + if !root_options[:ip] + raise Errors::NetworkIPAddressRequired + end + + # Validate the IP address + addr = IPAddr.new(root_options[:ip]) + + # If address is ipv6, enable ipv6 support + network_options[:ipv6] = addr.ipv6? + + # If no mask is provided, attempt to locate any existing + # network which contains the assigned IP address + if !root_options[:mask] && !network_name + network_name = env[:machine].provider.driver. + network_containing_address(root_options[:ip]) + # When no existing network is found, we are creating + # a new network. Since no mask was provided, default + # to /24 for ipv4 and /64 for ipv6 + if !network_name + root_options[:mask] = addr.ipv4? ? 24 : 64 + end + end + + # With no network name, process options to find or determine + # name for new network + if !network_name + subnet = IPAddr.new("#{root_options[:ip]}/#{root_options[:mask]}") + network = "#{subnet}/#{root_options[:mask]}" + network_options[:subnet] = network + existing_network = env[:machine].provider.driver. + network_defined?(network) + if !existing_network + network_name = "vagrant_network_#{network}" + else + if !existing_network.to_s.start_with?("vagrant_network") + env[:ui].warn(I18n.t("docker_provider.subnet_exists", + network_name: existing_network, + subnet: network)) + end + network_name = existing_network + end + end + + [network_name, network_options] + end + + # Generate configuration for public network + # + # @param [Hash] root_options Root networking options + # @param [Hash] net_options Docker scoped networking options + # @param [Hash] env Local call env + # @return [String, Hash] Network name and updated network_options + def process_public_network(root_options, net_options, env) + if root_options[:name] && validate_network_name!(root_options[:name]) + network_name = root_options[:name] + end + if !network_name + valid_interfaces = list_interfaces + if valid_interfaces.empty? + raise Errors::NetworkNoInterfaces + elsif valid_interfaces.size == 1 + bridge_interface = valid_interfaces.first + elsif i = valid_interfaces.detect{|i| Array(root_options[:bridge]).include?(i.name) } + bridge_interface = i + end + if !bridge_interface + env[:ui].info(I18n.t("vagrant.actions.vm.bridged_networking.available"), + prefix: false) + valid_interfaces.each_with_index do |int, i| + env[:ui].info("#{i + 1}) #{int.name}", prefix: false) + end + env[:ui].info(I18n.t( + "vagrant.actions.vm.bridged_networking.choice_help") + "\n", + prefix: false + ) + end + while !bridge_interface + choice = env[:ui].ask( + I18n.t("vagrant.actions.vm.bridged_networking.select_interface") + " ", + prefix: false) + bridge_interface = valid_interfaces[choice.to_i - 1] + end + base_opts = Vagrant::Util::HashWithIndifferentAccess.new + base_opts[:opt] = "parent=#{bridge_interface.name}" + subnet = IPAddr.new(bridge_interface.addr.ip_address << + "/" << bridge_interface.netmask.ip_unpack.first) + base_opts[:subnet] = "#{subnet}/#{subnet.prefix}" + subnet_addr = IPAddr.new(base_opts[:subnet]) + base_opts[:driver] = "macvlan" + base_opts[:gateway] = subnet_addr.succ.to_s + base_opts[:ipv6] = subnet_addr.ipv6? + network_options = base_opts.merge(net_options) + + # Check if network already exists for this subnet + network_name = env[:machine].provider.driver. + network_containing_address(network_options[:gateway]) + if !network_name + network_name = "vagrant_network_public_#{bridge_interface.name}" + end + + # If the network doesn't already exist, gather available address range + # within subnet which docker can provide addressing + if !env[:machine].provider.driver.existing_named_network?(network_name) + if !net_options[:gateway] + network_options[:gateway] = request_public_gateway( + network_options, bridge_interface.name, env) + end + network_options[:ip_range] = request_public_iprange( + network_options, bridge_interface.name, env) + end + end + [network_name, network_options] + end + + # Request the gateway address for the public network + # + # @param [Hash] network_options Docker scoped networking options + # @param [String] interface The bridge interface used + # @param [Hash] env Local call env + # @return [String] Gateway address + def request_public_gateway(network_options, interface, env) + subnet = IPAddr.new(network_options[:subnet]) + gateway = nil + while !gateway + gateway = env[:ui].ask(I18n.t( + "docker_provider.network_bridge_gateway_request", + interface: interface, + default_gateway: network_options[:gateway]) + " ", + prefix: false + ).strip + if gateway.empty? + gateway = network_options[:gateway] + end + begin + gateway = IPAddr.new(gateway) + if !subnet.include?(gateway) + env[:ui].warn(I18n.t("docker_provider.network_bridge_gateway_outofbounds", + gateway: gateway, + subnet: network_options[:subnet]) + "\n", prefix: false) + end + rescue IPAddr::InvalidAddressError + env[:ui].warn(I18n.t("docker_provider.network_bridge_gateway_invalid", + gateway: gateway) + "\n", prefix: false) + gateway = nil + end + end + gateway.to_s + end + + # Request the IP range allowed for use by docker when creating a new + # public network + # + # @param [Hash] network_options Docker scoped networking options + # @param [String] interface The bridge interface used + # @param [Hash] env Local call env + # @return [String] Address range + def request_public_iprange(network_options, interface, env) + return network_options[:ip_range] if network_options[:ip_range] + subnet = IPAddr.new(network_options[:subnet]) + env[:ui].info(I18n.t( + "docker_provider.network_bridge_iprange_info") + "\n", + prefix: false + ) + range = nil + while !range + range = env[:ui].ask(I18n.t( + "docker_provider.network_bridge_iprange_request", + interface: interface, + default_range: network_options[:subnet]) + " ", + prefix: false + ).strip + if range.empty? + range = network_options[:subnet] + end + begin + range = IPAddr.new(range) + if !subnet.include?(range) + env[:ui].warn(I18n.t( + "docker_provider.network_bridge_iprange_outofbounds", + subnet: network_options[:subnet], + range: "#{range}/#{range.prefix}" + ) + "\n", prefix: false) + range = nil + end + rescue IPAddr::InvalidAddressError + env[:ui].warn(I18n.t( + "docker_provider.network_bridge_iprange_invalid", + range: range) + "\n", prefix: false) + range = nil + end + end + "#{range}/#{range.prefix}" + end + + # Execute the action + def call(env) + # If we are using a host VM, then don't worry about it + machine = env[:machine] + if machine.provider.host_vm? + @logger.debug("Not setting up networks because docker host_vm is in use") + return @app.call(env) + end + + connections = {} + machine.env.lock("docker-network-create", retry: true) do + env[:ui].info(I18n.t("docker_provider.network_create")) + machine.config.vm.networks.each_with_index do |net_info, net_idx| + type, options = net_info + network_options = scoped_hash_override(options, :docker_network) + network_options.delete_if{|k,_| options.key?(k)} + + case type + when :public_network + network_name, network_options = process_public_network( + options, network_options, env) + when :private_network + network_name, network_options = process_private_network( + options, network_options, env) + else + next # unsupported type so ignore + end + + if !network_name + raise Errors::NetworkInvalidOption, container: machine.name + end + + if !machine.provider.driver.existing_named_network?(network_name) + @logger.debug("Creating network #{network_name}") + cli_opts = generate_create_cli_arguments(network_options) + machine.provider.driver.create_network(network_name, cli_opts) + else + @logger.debug("Network #{network_name} already created") + validate_network_configuration!(network_name, options, network_options, machine.provider.driver) + end + connections[net_idx] = network_name + end + end + + env[:docker_connects] = connections + @app.call(env) + end + end + end + end +end diff --git a/plugins/providers/docker/driver.rb b/plugins/providers/docker/driver.rb index dbe43abf1..7af69efda 100644 --- a/plugins/providers/docker/driver.rb +++ b/plugins/providers/docker/driver.rb @@ -157,14 +157,20 @@ module VagrantPlugins raise if !e.to_s.include?("No such image") end + # Inspect the provided container + # + # @param [String] cid ID or name of container + # @return [Hash] def inspect_container(cid) JSON.parse(execute('docker', 'inspect', cid)).first end + # @return [Array] list of all container IDs def all_containers execute('docker', 'ps', '-a', '-q', '--no-trunc').to_s.split end + # @return [String] IP address of the docker bridge def docker_bridge_ip output = execute('/sbin/ip', '-4', 'addr', 'show', 'scope', 'global', 'docker0') if output =~ /^\s+inet ([0-9.]+)\/[0-9]+\s+/ @@ -206,12 +212,18 @@ module VagrantPlugins command = ['docker', 'network', 'inspect'] + Array(network) command = command.push(*opts) output = execute(*command) - output + begin + JSON.load(output) + rescue JSON::ParseError + @logger.warn("Failed to parse network inspection of network: #{network}") + @logger.debug("Failed network output content: `#{output.inspect}`") + nil + end end - # @param [Array] opts - An array of flags used for listing networks - def list_network(opts=nil) - command = ['docker', 'network', 'ls'].push(*opts) + # @param [String] opts - Flags used for listing networks + def list_network(*opts) + command = ['docker', 'network', 'ls', *opts] output = execute(*command) output end @@ -226,12 +238,11 @@ module VagrantPlugins output end - # TODO: Note...cli can optionally take a list of networks to delete. - # We might need this later, but for now our helper takes 1 network at a time + # Delete network(s) # # @param [String] network - name of network to remove - def rm_network(network) - command = ['docker', 'network', 'rm', network] + def rm_network(*network) + command = ['docker', 'network', 'rm', *network] output = execute(*command) output end @@ -246,34 +257,55 @@ module VagrantPlugins # ###################### # @param [String] subnet_string - Subnet to look for - def subnet_defined?(subnet_string) - all_networks = list_network(["--format={{.Name}}"]) - all_networks = all_networks.split("\n") + def network_defined?(subnet_string) + all_networks = list_network_names - results = inspect_network(all_networks) - begin - networks_info = JSON.parse(results) - networks_info.each do |network| - config = network["IPAM"]["Config"] - if (config.size > 0 && - config.first["Subnet"] == subnet_string) - @logger.debug("Found existing network #{network["Name"]} already configured with #{subnet_string}") - return network["Name"] - end + network_info = inspect_network(all_networks) + network_info.each do |network| + config = network["IPAM"]["Config"] + if (config.size > 0 && + config.first["Subnet"] == subnet_string) + @logger.debug("Found existing network #{network["Name"]} already configured with #{subnet_string}") + return network["Name"] end - rescue JSON::ParserError => e - @logger.warn("Could not properly parse response from `docker network inspect #{all_networks.join(" ")}`") end return nil end - # Looks to see if a docker network has already been defined + # Locate network which contains given address # - # @param [String] network - name of network to look for - def existing_network?(network) - result = list_network(["--format={{.Name}}"]) - #TODO: we should be more explicit here if we can - result.match?(/#{network}/) + # @param [String] address IP address + # @return [String] network name + def network_containing_address(address) + names = list_network_names + networks = inspect_network(names) + return if !networks + networks.each do |net| + next if !net["IPAM"] + config = net["IPAM"]["Config"] + next if !config || config.size < 1 + config.each do |opts| + subnet = IPAddr.new(opts["Subnet"]) + if subnet.include?(address) + return net["Name"] + end + end + end + nil + end + + # Looks to see if a docker network has already been defined + # with the given name + # + # @param [String] network_name - name of network to look for + def existing_named_network?(network_name) + result = list_network_names + result.any?{|net_name| net_name == network_name} + end + + # @return [Array] list of all docker networks + def list_network_names + list_network("--format={{.Name}}").split("\n").map(&:strip) end # Returns true or false if network is in use or not. @@ -283,13 +315,8 @@ module VagrantPlugins # @return [Bool,nil] def network_used?(network) result = inspect_network(network) - begin - result = JSON.parse(result) - return result.first["Containers"].size > 0 - rescue JSON::ParserError => e - @logger.warn("Could not properly parse response from `docker network inspect #{network}`") - return nil - end + return nil if !result + return result.first["Containers"].size > 0 end end diff --git a/plugins/providers/docker/errors.rb b/plugins/providers/docker/errors.rb index ffd085427..dc4e2e549 100644 --- a/plugins/providers/docker/errors.rb +++ b/plugins/providers/docker/errors.rb @@ -49,6 +49,14 @@ module VagrantPlugins error_key(:network_invalid_option) end + class NetworkNameMissing < DockerError + error_key(:network_name_missing) + end + + class NetworkNameUndefined < DockerError + error_key(:network_name_undefined) + end + class PackageNotSupported < DockerError error_key(:package_not_supported) end diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 1e4188c5b..5927d6b21 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -2148,6 +2148,8 @@ en: choice_help: |- When choosing an interface, it is usually the one that is being used to connect to the internet. + select_interface: |- + Which interface should the network bridge to? specific_not_found: |- Specific bridge '%{bridge}' not found. You may be asked to specify which network to bridge to. diff --git a/templates/locales/providers_docker.yml b/templates/locales/providers_docker.yml index 678307f7c..6d5c13635 100644 --- a/templates/locales/providers_docker.yml +++ b/templates/locales/providers_docker.yml @@ -45,8 +45,33 @@ en: This container requires a host VM, and the state of that VM is unknown. Run `vagrant up` to verify that the container and its host VM is running, then try again. - network_configure: |- - Configuring and enabling network interfaces... + network_bridge_gateway_invalid: |- + The provided gateway IP address is invalid (%{gateway}). Please + provide a valid IP address. + network_bridge_gateway_outofbounds: |- + The provided gateway IP (%{gateway}) is not within the defined + subnet (%{subnet}). Please provide an IP address within the + defined subnet. + network_bridge_gateway_request: |- + Gateway IP address for %{interface} interface [%{default_gateway}]: + network_bridge_iprange_info: |- + When an explicit address is not provided to a container attached + to this bridged network, docker will supply an address to the + container. This is independent of the local DHCP service that + may be available on the network. + network_bridge_iprange_invalid: |- + The provided IP address range is invalid (%{range}). Please + provide a valid range. + network_bridge_iprange_outofbounds: |- + The provided IP address range (%{range}) is not within the + defined subnet (%{subnet}). Please provide an address range + within the defined subnet. + network_bridge_iprange_request: |- + Available address range for assignment on %{interface} interface [%{default_range}]: + network_create: |- + Creating and configuring docker networks... + network_connect: |- + Enabling network interfaces... network_destroy: |- Removing network %{network_name} ... not_created_skip: |- @@ -204,9 +229,33 @@ en: is functional and properly configured. Host VM ID: %{id} + network_address_invalid: |- + The configured network address is not valid within the configured + subnet of the defined network. Please update the network settings + and try again. + + Configured address: %{address} + Network name: %{network_name} network_invalid_option: |- Invalid option given for docker network for guest "%{container}". Must specify either a `subnet` or use `type: "dhcp"`. + network_name_missing: |- + The Docker provider is unable to connect the container to the + defined network due to a missing network name. Please validate + your configuration and try again. + + Container: %{container} + Network Number: %{index} + network_name_undefined: |- + The Docker provider was unable to configure networking using the + provided network name `%{network_name}`. Please ensure the network + name is correct and exists, then try again. + network_subnet_invalid: |- + The configured network subnet is not valid for the defined network. + Please update the network settings and try again. + + Configured subnet: %{subnet} + Network name: %{network_name} package_not_supported: |- The "package" command is not supported with the Docker provider. If you'd like to commit or push your Docker container, please SSH From a1c7eec441b511ec1106ec31a0696aa248127f01 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Tue, 19 Mar 2019 13:46:14 -0700 Subject: [PATCH 37/64] Include synchronization as the environment lock is per process only --- .../docker/action/destroy_network.rb | 31 +++++----- .../docker/action/prepare_networks.rb | 58 ++++++++++--------- 2 files changed, 49 insertions(+), 40 deletions(-) diff --git a/plugins/providers/docker/action/destroy_network.rb b/plugins/providers/docker/action/destroy_network.rb index 90e57fc28..d2aa613ec 100644 --- a/plugins/providers/docker/action/destroy_network.rb +++ b/plugins/providers/docker/action/destroy_network.rb @@ -4,6 +4,9 @@ module VagrantPlugins module DockerProvider module Action class DestroyNetwork + + @@lock = Mutex.new + def initialize(app, env) @app = app @logger = Log4r::Logger.new('vagrant::plugins::docker::network') @@ -17,21 +20,23 @@ module VagrantPlugins return @app.call(env) end - machine.config.vm.networks.each do |type, options| - next if type != :private_network && type != :public_network - + @@lock.synchronize do machine.env.lock("docker-network-destroy", retry: true) do - vagrant_networks = machine.provider.driver.list_network_names.find_all do |n| - n.start_with?("vagrant_network") - end + machine.config.vm.networks.each do |type, options| + next if type != :private_network && type != :public_network - vagrant_networks.each do |network_name| - if machine.provider.driver.existing_named_network?(network_name) && - !machine.provider.driver.network_used?(network_name) - env[:ui].info(I18n.t("docker_provider.network_destroy", network_name: network_name)) - machine.provider.driver.rm_network(network_name) - else - @logger.debug("Network #{network_name} not found or in use") + vagrant_networks = machine.provider.driver.list_network_names.find_all do |n| + n.start_with?("vagrant_network") + end + + vagrant_networks.each do |network_name| + if machine.provider.driver.existing_named_network?(network_name) && + !machine.provider.driver.network_used?(network_name) + env[:ui].info(I18n.t("docker_provider.network_destroy", network_name: network_name)) + machine.provider.driver.rm_network(network_name) + else + @logger.debug("Network #{network_name} not found or in use") + end end end end diff --git a/plugins/providers/docker/action/prepare_networks.rb b/plugins/providers/docker/action/prepare_networks.rb index 842c69154..2815eccd3 100644 --- a/plugins/providers/docker/action/prepare_networks.rb +++ b/plugins/providers/docker/action/prepare_networks.rb @@ -8,6 +8,8 @@ module VagrantPlugins include Vagrant::Util::ScopedHashOverride + @@lock = Mutex.new + def initialize(app, env) @app = app @logger = Log4r::Logger.new('vagrant::plugins::docker::preparenetworks') @@ -292,37 +294,39 @@ module VagrantPlugins end connections = {} - machine.env.lock("docker-network-create", retry: true) do - env[:ui].info(I18n.t("docker_provider.network_create")) - machine.config.vm.networks.each_with_index do |net_info, net_idx| - type, options = net_info - network_options = scoped_hash_override(options, :docker_network) - network_options.delete_if{|k,_| options.key?(k)} + @@lock.synchronize do + machine.env.lock("docker-network-create", retry: true) do + env[:ui].info(I18n.t("docker_provider.network_create")) + machine.config.vm.networks.each_with_index do |net_info, net_idx| + type, options = net_info + network_options = scoped_hash_override(options, :docker_network) + network_options.delete_if{|k,_| options.key?(k)} - case type - when :public_network - network_name, network_options = process_public_network( - options, network_options, env) - when :private_network - network_name, network_options = process_private_network( - options, network_options, env) - else - next # unsupported type so ignore - end + case type + when :public_network + network_name, network_options = process_public_network( + options, network_options, env) + when :private_network + network_name, network_options = process_private_network( + options, network_options, env) + else + next # unsupported type so ignore + end - if !network_name - raise Errors::NetworkInvalidOption, container: machine.name - end + if !network_name + raise Errors::NetworkInvalidOption, container: machine.name + end - if !machine.provider.driver.existing_named_network?(network_name) - @logger.debug("Creating network #{network_name}") - cli_opts = generate_create_cli_arguments(network_options) - machine.provider.driver.create_network(network_name, cli_opts) - else - @logger.debug("Network #{network_name} already created") - validate_network_configuration!(network_name, options, network_options, machine.provider.driver) + if !machine.provider.driver.existing_named_network?(network_name) + @logger.debug("Creating network #{network_name}") + cli_opts = generate_create_cli_arguments(network_options) + machine.provider.driver.create_network(network_name, cli_opts) + else + @logger.debug("Network #{network_name} already created") + validate_network_configuration!(network_name, options, network_options, machine.provider.driver) + end + connections[net_idx] = network_name end - connections[net_idx] = network_name end end From afb6c205819443e42ec0c3e15fa0b389257e3b97 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Tue, 19 Mar 2019 14:03:03 -0700 Subject: [PATCH 38/64] Fix option mask to be expected netmask --- plugins/providers/docker/action/prepare_networks.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/providers/docker/action/prepare_networks.rb b/plugins/providers/docker/action/prepare_networks.rb index 2815eccd3..68145f1ef 100644 --- a/plugins/providers/docker/action/prepare_networks.rb +++ b/plugins/providers/docker/action/prepare_networks.rb @@ -102,22 +102,22 @@ module VagrantPlugins # If no mask is provided, attempt to locate any existing # network which contains the assigned IP address - if !root_options[:mask] && !network_name + if !root_options[:netmask] && !network_name network_name = env[:machine].provider.driver. network_containing_address(root_options[:ip]) # When no existing network is found, we are creating # a new network. Since no mask was provided, default # to /24 for ipv4 and /64 for ipv6 if !network_name - root_options[:mask] = addr.ipv4? ? 24 : 64 + root_options[:netmask] = addr.ipv4? ? 24 : 64 end end # With no network name, process options to find or determine # name for new network if !network_name - subnet = IPAddr.new("#{root_options[:ip]}/#{root_options[:mask]}") - network = "#{subnet}/#{root_options[:mask]}" + subnet = IPAddr.new("#{root_options[:ip]}/#{root_options[:netmask]}") + network = "#{subnet}/#{root_options[:netmask]}" network_options[:subnet] = network existing_network = env[:machine].provider.driver. network_defined?(network) From 670bef6596c8b0a5907bb79e400dee65b6058905 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Tue, 19 Mar 2019 14:20:14 -0700 Subject: [PATCH 39/64] Allow custom subnet to be provided when private network type is dhcp --- plugins/providers/docker/action/connect_networks.rb | 2 +- plugins/providers/docker/action/prepare_networks.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/providers/docker/action/connect_networks.rb b/plugins/providers/docker/action/connect_networks.rb index 9f29e2031..929701579 100644 --- a/plugins/providers/docker/action/connect_networks.rb +++ b/plugins/providers/docker/action/connect_networks.rb @@ -55,7 +55,7 @@ module VagrantPlugins end @logger.debug("Connecting network #{network_name} to container guest #{machine.name}") - if options[:ip] + if options[:ip] && options[:type] != "dhcp" if IPAddr.new(options[:ip]).ipv4? network_options[:ip] = options[:ip] else diff --git a/plugins/providers/docker/action/prepare_networks.rb b/plugins/providers/docker/action/prepare_networks.rb index 68145f1ef..b7cb4e1f0 100644 --- a/plugins/providers/docker/action/prepare_networks.rb +++ b/plugins/providers/docker/action/prepare_networks.rb @@ -85,7 +85,7 @@ module VagrantPlugins network_name = root_options[:name] end - if root_options[:type].to_s == "dhcp" + if root_options[:type].to_s == "dhcp" && !root_options[:ip] network_name = "vagrant_network" if !network_name return [network_name, network_options] end From 623a1815aeaab03ac3e61ca993db622507cc2b83 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Tue, 19 Mar 2019 14:35:40 -0700 Subject: [PATCH 40/64] Allow use of subnet option when defining private network with dhcp type --- .../docker/action/prepare_networks.rb | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/plugins/providers/docker/action/prepare_networks.rb b/plugins/providers/docker/action/prepare_networks.rb index b7cb4e1f0..cc9796f6e 100644 --- a/plugins/providers/docker/action/prepare_networks.rb +++ b/plugins/providers/docker/action/prepare_networks.rb @@ -85,18 +85,23 @@ module VagrantPlugins network_name = root_options[:name] end - if root_options[:type].to_s == "dhcp" && !root_options[:ip] - network_name = "vagrant_network" if !network_name - return [network_name, network_options] + if root_options[:type].to_s == "dhcp" + if !root_options[:ip] && !root_options[:subnet] + network_name = "vagrant_network" if !network_name + return [network_name, network_options] + end + if root_options[:subnet] + addr = IPAddr.new(root_options[:subnet]) + root_options[:netmask] = addr.prefix + end end - if !root_options[:ip] + if root_options[:ip] + addr = IPAddr.new(root_options[:ip]) + elsif addr.nil? raise Errors::NetworkIPAddressRequired end - # Validate the IP address - addr = IPAddr.new(root_options[:ip]) - # If address is ipv6, enable ipv6 support network_options[:ipv6] = addr.ipv6? @@ -116,7 +121,7 @@ module VagrantPlugins # With no network name, process options to find or determine # name for new network if !network_name - subnet = IPAddr.new("#{root_options[:ip]}/#{root_options[:netmask]}") + subnet = IPAddr.new("#{addr}/#{root_options[:netmask]}") network = "#{subnet}/#{root_options[:netmask]}" network_options[:subnet] = network existing_network = env[:machine].provider.driver. From c09bce5386e6f6e116f58016a1e71c2a5d3671eb Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 20 Mar 2019 13:35:46 -0700 Subject: [PATCH 41/64] Fixup docker action destroy_network test --- .../docker/action/destroy_network_test.rb | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/test/unit/plugins/providers/docker/action/destroy_network_test.rb b/test/unit/plugins/providers/docker/action/destroy_network_test.rb index 54e3624a5..e6856f022 100644 --- a/test/unit/plugins/providers/docker/action/destroy_network_test.rb +++ b/test/unit/plugins/providers/docker/action/destroy_network_test.rb @@ -48,10 +48,13 @@ describe VagrantPlugins::DockerProvider::Action::DestroyNetwork do end describe "#call" do + let(:network_names) { ["vagrant_network_172.20.0.0/16", "vagrant_network_2a02:6b8:b010:9020:1::/80"] } + it "calls the next action in the chain" do allow(driver).to receive(:host_vm?).and_return(false) allow(driver).to receive(:existing_network?).and_return(true) allow(driver).to receive(:network_used?).and_return(true) + allow(driver).to receive(:list_network_names).and_return([]) called = false app = ->(*args) { called = true } @@ -63,29 +66,31 @@ describe VagrantPlugins::DockerProvider::Action::DestroyNetwork do end it "calls the proper driver method to destroy the network" do + allow(driver).to receive(:list_network_names).and_return(network_names) allow(driver).to receive(:host_vm?).and_return(false) - allow(driver).to receive(:existing_network?).with("vagrant_network_172.20.0.0/16"). + allow(driver).to receive(:existing_named_network?).with("vagrant_network_172.20.0.0/16"). and_return(true) allow(driver).to receive(:network_used?).with("vagrant_network_172.20.0.0/16"). and_return(false) - allow(driver).to receive(:existing_network?).with("vagrant_network_2a02:6b8:b010:9020:1::/80"). + allow(driver).to receive(:existing_named_network?).with("vagrant_network_2a02:6b8:b010:9020:1::/80"). and_return(true) allow(driver).to receive(:network_used?).with("vagrant_network_2a02:6b8:b010:9020:1::/80"). and_return(false) - expect(driver).to receive(:rm_network).with("vagrant_network_172.20.0.0/16") - expect(driver).to receive(:rm_network).with("vagrant_network_2a02:6b8:b010:9020:1::/80") + expect(driver).to receive(:rm_network).with("vagrant_network_172.20.0.0/16").twice + expect(driver).to receive(:rm_network).with("vagrant_network_2a02:6b8:b010:9020:1::/80").twice subject.call(env) end it "doesn't destroy the network if another container is still using it" do allow(driver).to receive(:host_vm?).and_return(false) - allow(driver).to receive(:existing_network?).with("vagrant_network_172.20.0.0/16"). + allow(driver).to receive(:list_network_names).and_return(network_names) + allow(driver).to receive(:existing_named_network?).with("vagrant_network_172.20.0.0/16"). and_return(true) allow(driver).to receive(:network_used?).with("vagrant_network_172.20.0.0/16"). and_return(true) - allow(driver).to receive(:existing_network?).with("vagrant_network_2a02:6b8:b010:9020:1::/80"). + allow(driver).to receive(:existing_named_network?).with("vagrant_network_2a02:6b8:b010:9020:1::/80"). and_return(true) allow(driver).to receive(:network_used?).with("vagrant_network_2a02:6b8:b010:9020:1::/80"). and_return(true) From eb75431c4a9cb3891b74858f322f5c8f7e27109b Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 20 Mar 2019 14:50:28 -0700 Subject: [PATCH 42/64] Update docker driver and docker driver unit tests --- plugins/providers/docker/driver.rb | 7 +- .../plugins/providers/docker/driver_test.rb | 234 ++++++++++++++++++ 2 files changed, 240 insertions(+), 1 deletion(-) diff --git a/plugins/providers/docker/driver.rb b/plugins/providers/docker/driver.rb index 7af69efda..acad830cf 100644 --- a/plugins/providers/docker/driver.rb +++ b/plugins/providers/docker/driver.rb @@ -214,7 +214,7 @@ module VagrantPlugins output = execute(*command) begin JSON.load(output) - rescue JSON::ParseError + rescue JSON::ParserError @logger.warn("Failed to parse network inspection of network: #{network}") @logger.debug("Failed network output content: `#{output.inspect}`") nil @@ -256,7 +256,11 @@ module VagrantPlugins # Docker network helpers # ###################### + # Determines if a given network has been defined through vagrant with a given + # subnet string + # # @param [String] subnet_string - Subnet to look for + # @return [String] network name - Name of network with requested subnet.`nil` if not found def network_defined?(subnet_string) all_networks = list_network_names @@ -298,6 +302,7 @@ module VagrantPlugins # with the given name # # @param [String] network_name - name of network to look for + # @return [Bool] def existing_named_network?(network_name) result = list_network_names result.any?{|net_name| net_name == network_name} diff --git a/test/unit/plugins/providers/docker/driver_test.rb b/test/unit/plugins/providers/docker/driver_test.rb index d4e66b72e..6504876cb 100644 --- a/test/unit/plugins/providers/docker/driver_test.rb +++ b/test/unit/plugins/providers/docker/driver_test.rb @@ -10,6 +10,149 @@ describe VagrantPlugins::DockerProvider::Driver do allow(subject).to receive(:execute) { |*args| @cmd = args.join(' ') } end + let(:docker_network_struct) { +[ + { + "Name": "bridge", + "Id": "ae74f6cc18bbcde86326937797070b814cc71bfc4a6d8e3e8cf3b2cc5c7f4a7d", + "Created": "2019-03-20T14:10:06.313314662-07:00", + "Scope": "local", + "Driver": "bridge", + "EnableIPv6": false, + "IPAM": { + "Driver": "default", + "Options": nil, + "Config": [ + { + "Subnet": "172.17.0.0/16", + "Gateway": "172.17.0.1" + } + ] + }, + "Internal": false, + "Attachable": false, + "Ingress": false, + "ConfigFrom": { + "Network": "" + }, + "ConfigOnly": false, + "Containers": { + "a1ee9b12bcea8268495b1f43e8d1285df1925b7174a695075f6140adb9415d87": { + "Name": "vagrant-sandbox_docker-1_1553116237", + "EndpointID": "fc1b0ed6e4f700cf88bb26a98a0722655191542e90df3e3492461f4d1f3c0cae", + "MacAddress": "02:42:ac:11:00:02", + "IPv4Address": "172.17.0.2/16", + "IPv6Address": "" + } + }, + "Options": { + "com.docker.network.bridge.default_bridge": "true", + "com.docker.network.bridge.enable_icc": "true", + "com.docker.network.bridge.enable_ip_masquerade": "true", + "com.docker.network.bridge.host_binding_ipv4": "0.0.0.0", + "com.docker.network.bridge.name": "docker0", + "com.docker.network.driver.mtu": "1500" + }, + "Labels": {} + }, + { + "Name": "host", + "Id": "2a2845e77550e33bf3e97bda8b71477ac7d3ccf78bc9102585fdb6056fb84cbf", + "Created": "2018-09-28T10:54:08.633543196-07:00", + "Scope": "local", + "Driver": "host", + "EnableIPv6": false, + "IPAM": { + "Driver": "default", + "Options": nil, + "Config": [] + }, + "Internal": false, + "Attachable": false, + "Ingress": false, + "ConfigFrom": { + "Network": "" + }, + "ConfigOnly": false, + "Containers": {}, + "Options": {}, + "Labels": {} + }, + { + "Name": "vagrant_network", + "Id": "93385d4fd3cf7083a36e62fa72a0ad0a21203d0ddf48409c32b550cd8462b3ba", + "Created": "2019-03-20T14:10:36.828235585-07:00", + "Scope": "local", + "Driver": "bridge", + "EnableIPv6": false, + "IPAM": { + "Driver": "default", + "Options": {}, + "Config": [ + { + "Subnet": "172.18.0.0/16", + "Gateway": "172.18.0.1" + } + ] + }, + "Internal": false, + "Attachable": false, + "Ingress": false, + "ConfigFrom": { + "Network": "" + }, + "ConfigOnly": false, + "Containers": { + "a1ee9b12bcea8268495b1f43e8d1285df1925b7174a695075f6140adb9415d87": { + "Name": "vagrant-sandbox_docker-1_1553116237", + "EndpointID": "9502cd9d37ae6815e3ffeb0bc2de9b84f79e7223e8a1f8f4ccc79459e96c7914", + "MacAddress": "02:42:ac:12:00:02", + "IPv4Address": "172.18.0.2/16", + "IPv6Address": "" + } + }, + "Options": {}, + "Labels": {} + }, + { + "Name": "vagrant_network_172.20.0.0/16", + "Id": "649f0ab3ef0eef6f2a025c0d0398bd7b9b4d05ec88b0d7bd573b44153d903cfb", + "Created": "2019-03-20T14:10:37.088885647-07:00", + "Scope": "local", + "Driver": "bridge", + "EnableIPv6": false, + "IPAM": { + "Driver": "default", + "Options": {}, + "Config": [ + { + "Subnet": "172.20.0.0/16" + } + ] + }, + "Internal": false, + "Attachable": false, + "Ingress": false, + "ConfigFrom": { + "Network": "" + }, + "ConfigOnly": false, + "Containers": { + "a1ee9b12bcea8268495b1f43e8d1285df1925b7174a695075f6140adb9415d87": { + "Name": "vagrant-sandbox_docker-1_1553116237", + "EndpointID": "e19156f8018f283468227fa97c145f4ea0eaba652fb7e977a0c759b1c3ec168a", + "MacAddress": "02:42:ac:14:80:02", + "IPv4Address": "172.20.0.2/16", + "IPv6Address": "" + } + }, + "Options": {}, + "Labels": {} + } +].to_json } + + + describe '#create' do let(:params) { { image: 'jimi/hendrix:electric-ladyland', @@ -295,4 +438,95 @@ describe VagrantPlugins::DockerProvider::Driver do subject.rm_network("vagrant_network") end end + + describe '#network_defined?' do + let(:subnet_string) { "172.20.0.0/16" } + let(:network_names) { ["vagrant_network_172.20.0.0/16", "bridge", "null" ] } + + it "returns network name if defined" do + allow(subject).to receive(:list_network_names).and_return(network_names) + allow(subject).to receive(:inspect_network).and_return(JSON.load(docker_network_struct)) + + network_name = subject.network_defined?(subnet_string) + expect(network_name).to eq("vagrant_network_172.20.0.0/16") + end + + it "returns nil name if not defined" do + allow(subject).to receive(:list_network_names).and_return(network_names) + allow(subject).to receive(:inspect_network).and_return(JSON.load(docker_network_struct)) + + network_name = subject.network_defined?("120.20.0.0/24") + expect(network_name).to eq(nil) + end + end + + describe '#network_containing_address' do + let(:address) { "172.20.128.2" } + let(:network_names) { ["vagrant_network_172.20.0.0/16", "bridge", "null" ] } + + it "returns the network name if it contains the requested address" do + allow(subject).to receive(:list_network_names).and_return(network_names) + allow(subject).to receive(:inspect_network).and_return(JSON.load(docker_network_struct)) + + network_name = subject.network_containing_address(address) + expect(network_name).to eq("vagrant_network_172.20.0.0/16") + end + + it "returns nil if no networks contain the requested address" do + allow(subject).to receive(:list_network_names).and_return(network_names) + allow(subject).to receive(:inspect_network).and_return(JSON.load(docker_network_struct)) + + network_name = subject.network_containing_address("127.0.0.1") + expect(network_name).to eq(nil) + end + end + + describe '#existing_named_network?' do + let(:network_names) { ["vagrant_network_172.20.0.0/16", "bridge", "null" ] } + + it "returns true if the network exists" do + allow(subject).to receive(:list_network_names).and_return(network_names) + + expect(subject.existing_named_network?("vagrant_network_172.20.0.0/16")).to be_truthy + end + + it "returns false if the network does not exist" do + allow(subject).to receive(:list_network_names).and_return(network_names) + + expect(subject.existing_named_network?("vagrant_network_17.0.0/16")).to be_falsey + end + end + + describe '#list_network_names' do + let(:unparsed_network_names) { "vagrant_network_172.20.0.0/16\nbridge\nnull" } + let(:network_names) { ["vagrant_network_172.20.0.0/16", "bridge", "null" ] } + + it "lists the network names" do + allow(subject).to receive(:list_network).with("--format={{.Name}}"). + and_return(unparsed_network_names) + + expect(subject.list_network_names).to eq(network_names) + end + end + + describe '#network_used?' do + let(:network_name) { "vagrant_network_172.20.0.0/16" } + it "returns nil if no networks" do + allow(subject).to receive(:inspect_network).with(network_name).and_return(nil) + + expect(subject.network_used?(network_name)).to eq(nil) + end + + it "returns true if network has containers in use" do + allow(subject).to receive(:inspect_network).with(network_name).and_return([JSON.load(docker_network_struct).last]) + + expect(subject.network_used?(network_name)).to be_truthy + end + + it "returns false if network has containers in use" do + allow(subject).to receive(:inspect_network).with("host").and_return([JSON.load(docker_network_struct)[1]]) + + expect(subject.network_used?("host")).to be_falsey + end + end end From 1027636e41920eddeb0ce20bb9f5210dfd3066e6 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 20 Mar 2019 15:14:39 -0700 Subject: [PATCH 43/64] Split up and rename unit tests for docker network operations --- .../docker/action/prepare_networks.rb | 2 + ...twork_test.rb => connect_networks_test.rb} | 4 +- .../docker/action/prepare_networks_test.rb | 150 ++++++++++++++++++ 3 files changed, 154 insertions(+), 2 deletions(-) rename test/unit/plugins/providers/docker/action/{network_test.rb => connect_networks_test.rb} (98%) create mode 100644 test/unit/plugins/providers/docker/action/prepare_networks_test.rb diff --git a/plugins/providers/docker/action/prepare_networks.rb b/plugins/providers/docker/action/prepare_networks.rb index cc9796f6e..4740ae0d2 100644 --- a/plugins/providers/docker/action/prepare_networks.rb +++ b/plugins/providers/docker/action/prepare_networks.rb @@ -1,6 +1,8 @@ require 'ipaddr' require 'log4r' +require 'vagrant/util/scoped_hash_override' + module VagrantPlugins module DockerProvider module Action diff --git a/test/unit/plugins/providers/docker/action/network_test.rb b/test/unit/plugins/providers/docker/action/connect_networks_test.rb similarity index 98% rename from test/unit/plugins/providers/docker/action/network_test.rb rename to test/unit/plugins/providers/docker/action/connect_networks_test.rb index c3d09e726..5e5ea798b 100644 --- a/test/unit/plugins/providers/docker/action/network_test.rb +++ b/test/unit/plugins/providers/docker/action/connect_networks_test.rb @@ -1,7 +1,7 @@ require_relative "../../../../base" -require_relative "../../../../../../plugins/providers/docker/action/network" +require_relative "../../../../../../plugins/providers/docker/action/connect_networks" -describe VagrantPlugins::DockerProvider::Action::Network do +describe VagrantPlugins::DockerProvider::Action::ConnectNetworks do include_context "unit" include_context "virtualbox" diff --git a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb new file mode 100644 index 000000000..6668a10f9 --- /dev/null +++ b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb @@ -0,0 +1,150 @@ +require_relative "../../../../base" +require_relative "../../../../../../plugins/providers/docker/action/prepare_networks" + +describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do + include_context "unit" + include_context "virtualbox" + + let(:sandbox) { isolated_environment } + + let(:iso_env) do + # We have to create a Vagrantfile so there is a root path + sandbox.vagrantfile("") + sandbox.create_vagrant_env + end + + let(:machine) do + iso_env.machine(iso_env.machine_names[0], :docker).tap do |m| + allow(m.provider).to receive(:driver).and_return(driver) + allow(m.config.vm).to receive(:networks).and_return(networks) + end + end + + let(:env) {{ machine: machine, ui: machine.ui, root_path: Pathname.new(".") }} + let(:app) { lambda { |*args| }} + let(:driver) { double("driver", create: "abcd1234") } + + let(:networks) { [[:private_network, + {:ip=>"172.20.128.2", + :subnet=>"172.20.0.0/16", + :driver=>"bridge", + :internal=>"true", + :alias=>"mynetwork", + :protocol=>"tcp", + :id=>"80e017d5-388f-4a2f-a3de-f8dce8156a58"}], + [:private_network, + {:type=>"dhcp", + :ipv6=>"true", + :subnet=>"2a02:6b8:b010:9020:1::/80", + :protocol=>"tcp", + :id=>"b8f23054-38d5-45c3-99ea-d33fc5d1b9f2"}], + [:forwarded_port, + {:guest=>22, :host=>2200, :host_ip=>"127.0.0.1", :id=>"ssh", :auto_correct=>true, :protocol=>"tcp"}]] + } + + let(:invalid_network) { + [[:private_network, + {:ipv6=>"true", + :protocol=>"tcp", + :id=>"b8f23054-38d5-45c3-99ea-d33fc5d1b9f2"}]] + } + + subject { described_class.new(app, env) } + + after do + sandbox.close + end + + describe "#call" do + it "calls the next action in the chain" do + allow(driver).to receive(:host_vm?).and_return(false) + allow(driver).to receive(:existing_network?).and_return(false) + allow(driver).to receive(:create_network).and_return(true) + allow(driver).to receive(:connect_network).and_return(true) + allow(driver).to receive(:subnet_defined?).and_return(nil) + + called = false + app = ->(*args) { called = true } + + action = described_class.new(app, env) + action.call(env) + + expect(called).to eq(true) + end + + it "calls the proper driver methods to setup a network" do + allow(driver).to receive(:host_vm?).and_return(false) + allow(driver).to receive(:existing_network?).and_return(false) + allow(driver).to receive(:create_network).and_return(true) + allow(driver).to receive(:connect_network).and_return(true) + allow(driver).to receive(:subnet_defined?).and_return(nil) + + + expect(subject).to receive(:generate_create_cli_arguments). + with(networks[0][1]).and_return(["--subnet=172.20.0.0/16", "--driver=bridge", "--internal=true"]) + expect(subject).to receive(:generate_create_cli_arguments). + with(networks[1][1]).and_return(["--ipv6=true", "--subnet=2a02:6b8:b010:9020:1::/80"]) + expect(subject).to receive(:generate_connect_cli_arguments). + with(networks[0][1]).and_return(["--ipv6=true", "--subnet=2a02:6b8:b010:9020:1::/80"]) + expect(subject).to receive(:generate_connect_cli_arguments). + with(networks[1][1]).and_return([]) + + expect(driver).to receive(:create_network).twice + expect(driver).to receive(:connect_network).twice + + subject.call(env) + end + + it "uses an existing network if a matching subnet is found" do + allow(driver).to receive(:host_vm?).and_return(false) + allow(driver).to receive(:existing_network?).and_return(true) + allow(driver).to receive(:create_network).and_return(true) + allow(driver).to receive(:connect_network).and_return(true) + allow(driver).to receive(:subnet_defined?).and_return("my_cool_subnet_network") + + expect(driver).not_to receive(:create_network) + + subject.call(env) + end + + it "raises an error if an inproper network configuration is given" do + allow(machine.config.vm).to receive(:networks).and_return(invalid_network) + allow(driver).to receive(:host_vm?).and_return(false) + allow(driver).to receive(:existing_network?).and_return(false) + + expect{ subject.call(env) }.to raise_error(VagrantPlugins::DockerProvider::Errors::NetworkInvalidOption) + end + end + + describe "#generate_connect_cli_arguments" do + let(:network_options) { + {:ip=>"172.20.128.2", + :subnet=>"172.20.0.0/16", + :driver=>"bridge", + :internal=>"true", + :alias=>"mynetwork", + :protocol=>"tcp", + :id=>"80e017d5-388f-4a2f-a3de-f8dce8156a58"} } + + it "returns an array of cli arguments" do + cli_args = subject.generate_connect_cli_arguments(network_options) + expect(cli_args).to eq(["--ip", "172.20.128.2", "--alias=mynetwork"]) + end + end + + describe "#generate_create_cli_arguments" do + let(:network_options) { + {:ip=>"172.20.128.2", + :subnet=>"172.20.0.0/16", + :driver=>"bridge", + :internal=>"true", + :alias=>"mynetwork", + :protocol=>"tcp", + :id=>"80e017d5-388f-4a2f-a3de-f8dce8156a58"} } + + it "returns an array of cli arguments" do + cli_args = subject.generate_create_cli_arguments(network_options) + expect(cli_args).to eq(["--subnet=172.20.0.0/16", "--driver=bridge", "--internal=true"]) + end + end +end From 75d6c1738605031c21e5493804b457990123b44c Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Wed, 20 Mar 2019 16:41:42 -0700 Subject: [PATCH 44/64] Fix up call method and add todo for prepare networks --- .../docker/action/prepare_networks_test.rb | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb index 6668a10f9..6815356d1 100644 --- a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb +++ b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb @@ -58,7 +58,7 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do describe "#call" do it "calls the next action in the chain" do allow(driver).to receive(:host_vm?).and_return(false) - allow(driver).to receive(:existing_network?).and_return(false) + allow(driver).to receive(:existing_named_network?).and_return(false) allow(driver).to receive(:create_network).and_return(true) allow(driver).to receive(:connect_network).and_return(true) allow(driver).to receive(:subnet_defined?).and_return(nil) @@ -67,6 +67,10 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do app = ->(*args) { called = true } action = described_class.new(app, env) + + allow(action).to receive(:process_public_network).and_return(["name", {}]) + allow(action).to receive(:process_private_network).and_return(["name", {}]) + action.call(env) expect(called).to eq(true) @@ -74,23 +78,26 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do it "calls the proper driver methods to setup a network" do allow(driver).to receive(:host_vm?).and_return(false) - allow(driver).to receive(:existing_network?).and_return(false) + allow(driver).to receive(:existing_named_network?).and_return(false) allow(driver).to receive(:create_network).and_return(true) allow(driver).to receive(:connect_network).and_return(true) allow(driver).to receive(:subnet_defined?).and_return(nil) + allow(driver).to receive(:network_containing_address). + with("172.20.128.2").and_return(nil) + allow(driver).to receive(:network_defined?).with("172.20.128.0/24"). + and_return(false) + allow(driver).to receive(:network_defined?).with("2a02:6b8:b010:9020:1::/80"). + and_return(false) + # TODO: Still need to finish up the rest of this method + # with the proper generate create tests expect(subject).to receive(:generate_create_cli_arguments). with(networks[0][1]).and_return(["--subnet=172.20.0.0/16", "--driver=bridge", "--internal=true"]) expect(subject).to receive(:generate_create_cli_arguments). with(networks[1][1]).and_return(["--ipv6=true", "--subnet=2a02:6b8:b010:9020:1::/80"]) - expect(subject).to receive(:generate_connect_cli_arguments). - with(networks[0][1]).and_return(["--ipv6=true", "--subnet=2a02:6b8:b010:9020:1::/80"]) - expect(subject).to receive(:generate_connect_cli_arguments). - with(networks[1][1]).and_return([]) expect(driver).to receive(:create_network).twice - expect(driver).to receive(:connect_network).twice subject.call(env) end From 5215354d16e6bbdca16288b13d1e7d8c242b4a7f Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 21 Mar 2019 10:50:45 -0700 Subject: [PATCH 45/64] Fix missing docker error classes, and finish out initial #call tests --- plugins/providers/docker/errors.rb | 12 +++ templates/locales/providers_docker.yml | 2 + .../docker/action/prepare_networks_test.rb | 74 ++++++++++++++----- 3 files changed, 71 insertions(+), 17 deletions(-) diff --git a/plugins/providers/docker/errors.rb b/plugins/providers/docker/errors.rb index dc4e2e549..84b5e8f4e 100644 --- a/plugins/providers/docker/errors.rb +++ b/plugins/providers/docker/errors.rb @@ -45,6 +45,18 @@ module VagrantPlugins error_key(:docker_provider_nfs_without_privileged) end + class NetworkAddressInvalid < DockerError + error_key(:network_address_invalid) + end + + class NetworkIPAddressRequired < DockerError + error_key(:network_address_required) + end + + class NetworkSubnetInvalid < DockerError + error_key(:network_subnet_invalid) + end + class NetworkInvalidOption < DockerError error_key(:network_invalid_option) end diff --git a/templates/locales/providers_docker.yml b/templates/locales/providers_docker.yml index 6d5c13635..00c929e8f 100644 --- a/templates/locales/providers_docker.yml +++ b/templates/locales/providers_docker.yml @@ -236,6 +236,8 @@ en: Configured address: %{address} Network name: %{network_name} + network_address_required: |- + An IP address is required if not using `type: "dhcp"` or not specifying a `subnet`. network_invalid_option: |- Invalid option given for docker network for guest "%{container}". Must specify either a `subnet` or use `type: "dhcp"`. diff --git a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb index 6815356d1..d12fe391f 100644 --- a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb +++ b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb @@ -32,6 +32,11 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do :alias=>"mynetwork", :protocol=>"tcp", :id=>"80e017d5-388f-4a2f-a3de-f8dce8156a58"}], + [:public_network, + {:ip=>"172.30.130.2", + :subnet=>"172.30.0.0/16", + :driver=>"bridge", + :id=>"30e017d5-488f-5a2f-a3ke-k8dce8246b60"}], [:private_network, {:type=>"dhcp", :ipv6=>"true", @@ -60,8 +65,6 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do allow(driver).to receive(:host_vm?).and_return(false) allow(driver).to receive(:existing_named_network?).and_return(false) allow(driver).to receive(:create_network).and_return(true) - allow(driver).to receive(:connect_network).and_return(true) - allow(driver).to receive(:subnet_defined?).and_return(nil) called = false app = ->(*args) { called = true } @@ -79,39 +82,76 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do it "calls the proper driver methods to setup a network" do allow(driver).to receive(:host_vm?).and_return(false) allow(driver).to receive(:existing_named_network?).and_return(false) - allow(driver).to receive(:create_network).and_return(true) - allow(driver).to receive(:connect_network).and_return(true) - allow(driver).to receive(:subnet_defined?).and_return(nil) allow(driver).to receive(:network_containing_address). with("172.20.128.2").and_return(nil) + allow(driver).to receive(:network_containing_address). + with("192.168.1.1").and_return(nil) allow(driver).to receive(:network_defined?).with("172.20.128.0/24"). and_return(false) + allow(driver).to receive(:network_defined?).with("172.30.128.0/24"). + and_return(false) allow(driver).to receive(:network_defined?).with("2a02:6b8:b010:9020:1::/80"). and_return(false) + allow(subject).to receive(:request_public_gateway).and_return("1234") + allow(subject).to receive(:request_public_iprange).and_return("1234") - # TODO: Still need to finish up the rest of this method - # with the proper generate create tests - expect(subject).to receive(:generate_create_cli_arguments). - with(networks[0][1]).and_return(["--subnet=172.20.0.0/16", "--driver=bridge", "--internal=true"]) - expect(subject).to receive(:generate_create_cli_arguments). - with(networks[1][1]).and_return(["--ipv6=true", "--subnet=2a02:6b8:b010:9020:1::/80"]) + allow(machine.ui).to receive(:ask).and_return("1") + + expect(driver).to receive(:create_network). + with("vagrant_network_172.20.128.0/24", ["--subnet", "172.20.128.0/24"]) + expect(driver).to receive(:create_network). + with("vagrant_network_public_wlp4s0", ["--opt", "parent=wlp4s0", "--subnet", "192.168.1.0/24", "--driver", "macvlan", "--gateway", "1234", "--ip-range", "1234"]) + expect(driver).to receive(:create_network). + with("vagrant_network_2a02:6b8:b010:9020:1::/80", ["--ipv6", "--subnet", "2a02:6b8:b010:9020:1::/80"]) - expect(driver).to receive(:create_network).twice subject.call(env) + + expect(env[:docker_connects]).to eq({0=>"vagrant_network_172.20.128.0/24", 1=>"vagrant_network_public_wlp4s0", 2=>"vagrant_network_2a02:6b8:b010:9020:1::/80"}) end it "uses an existing network if a matching subnet is found" do allow(driver).to receive(:host_vm?).and_return(false) - allow(driver).to receive(:existing_network?).and_return(true) - allow(driver).to receive(:create_network).and_return(true) - allow(driver).to receive(:connect_network).and_return(true) - allow(driver).to receive(:subnet_defined?).and_return("my_cool_subnet_network") + allow(driver).to receive(:network_containing_address). + with("172.20.128.2").and_return(nil) + allow(driver).to receive(:network_containing_address). + with("192.168.1.1").and_return(nil) + allow(driver).to receive(:network_defined?).with("172.20.128.0/24"). + and_return("vagrant_network_172.20.128.0/24") + allow(driver).to receive(:network_defined?).with("172.30.128.0/24"). + and_return("vagrant_network_public_wlp4s0") + allow(driver).to receive(:network_defined?).with("2a02:6b8:b010:9020:1::/80"). + and_return("vagrant_network_2a02:6b8:b010:9020:1::/80") + + expect(driver).to receive(:existing_named_network?). + with("vagrant_network_172.20.128.0/24").and_return(true) + expect(driver).to receive(:existing_named_network?). + with("vagrant_network_public_wlp4s0").and_return(true) + expect(driver).to receive(:existing_named_network?). + with("vagrant_network_public_wlp4s0").and_return(true) + expect(driver).to receive(:existing_named_network?). + with("vagrant_network_2a02:6b8:b010:9020:1::/80").and_return(true) + + allow(machine.ui).to receive(:ask).and_return("1") expect(driver).not_to receive(:create_network) + expect(subject).to receive(:validate_network_configuration!). + with("vagrant_network_172.20.128.0/24", networks[0][1], + {:ipv6=>false, :subnet=>"172.20.128.0/24"}, driver) + + expect(subject).to receive(:validate_network_configuration!). + with("vagrant_network_public_wlp4s0", networks[1][1], + {"opt"=>"parent=wlp4s0", "subnet"=>"192.168.1.0/24", "driver"=>"macvlan", "gateway"=>"192.168.1.1", "ipv6"=>false}, driver) + + + expect(subject).to receive(:validate_network_configuration!). + with("vagrant_network_2a02:6b8:b010:9020:1::/80", networks[2][1], + {:ipv6=>true, :subnet=>"2a02:6b8:b010:9020:1::/80"}, driver) + subject.call(env) + end it "raises an error if an inproper network configuration is given" do @@ -119,7 +159,7 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do allow(driver).to receive(:host_vm?).and_return(false) allow(driver).to receive(:existing_network?).and_return(false) - expect{ subject.call(env) }.to raise_error(VagrantPlugins::DockerProvider::Errors::NetworkInvalidOption) + expect{ subject.call(env) }.to raise_error(VagrantPlugins::DockerProvider::Errors::NetworkIPAddressRequired) end end From 96a19aa00c431bc2fa6bfcf9d07c51f684002e38 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 21 Mar 2019 11:15:41 -0700 Subject: [PATCH 46/64] Fix how options to cli args are handled Since options could also be defined as strings, convert it all to string and compare those instead --- .../docker/action/prepare_networks.rb | 6 ++-- .../docker/action/prepare_networks_test.rb | 32 +++++++++---------- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/plugins/providers/docker/action/prepare_networks.rb b/plugins/providers/docker/action/prepare_networks.rb index 4740ae0d2..e1ff59992 100644 --- a/plugins/providers/docker/action/prepare_networks.rb +++ b/plugins/providers/docker/action/prepare_networks.rb @@ -20,13 +20,13 @@ module VagrantPlugins # Generate CLI arguments for creating the docker network. # # @param [Hash] options Options from the network config - # @returns[Array Network create arguments + # @returns[Array] Network create arguments def generate_create_cli_arguments(options) options.map do |key, value| # If value is false, option is not set - next if value == false + next if value.to_s == "false" # If value is true, consider feature flag with no value - opt = value == true ? [] : [value] + opt = value.to_s == "true" ? [] : [value] opt.unshift("--#{key.to_s.tr("_", "-")}") end.flatten.compact end diff --git a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb index d12fe391f..84b5c829c 100644 --- a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb +++ b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb @@ -163,22 +163,6 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do end end - describe "#generate_connect_cli_arguments" do - let(:network_options) { - {:ip=>"172.20.128.2", - :subnet=>"172.20.0.0/16", - :driver=>"bridge", - :internal=>"true", - :alias=>"mynetwork", - :protocol=>"tcp", - :id=>"80e017d5-388f-4a2f-a3de-f8dce8156a58"} } - - it "returns an array of cli arguments" do - cli_args = subject.generate_connect_cli_arguments(network_options) - expect(cli_args).to eq(["--ip", "172.20.128.2", "--alias=mynetwork"]) - end - end - describe "#generate_create_cli_arguments" do let(:network_options) { {:ip=>"172.20.128.2", @@ -189,9 +173,23 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do :protocol=>"tcp", :id=>"80e017d5-388f-4a2f-a3de-f8dce8156a58"} } + let(:false_network_options) { + {:ip=>"172.20.128.2", + :subnet=>"172.20.0.0/16", + :driver=>"bridge", + :internal=>"false", + :alias=>"mynetwork", + :protocol=>"tcp", + :id=>"80e017d5-388f-4a2f-a3de-f8dce8156a58"} } + it "returns an array of cli arguments" do cli_args = subject.generate_create_cli_arguments(network_options) - expect(cli_args).to eq(["--subnet=172.20.0.0/16", "--driver=bridge", "--internal=true"]) + expect(cli_args).to eq( ["--ip", "172.20.128.2", "--subnet", "172.20.0.0/16", "--driver", "bridge", "--internal", "--alias", "mynetwork", "--protocol", "tcp", "--id", "80e017d5-388f-4a2f-a3de-f8dce8156a58"]) + end + + it "removes option if set to false" do + cli_args = subject.generate_create_cli_arguments(false_network_options) + expect(cli_args).to eq( ["--ip", "172.20.128.2", "--subnet", "172.20.0.0/16", "--driver", "bridge", "--alias", "mynetwork", "--protocol", "tcp", "--id", "80e017d5-388f-4a2f-a3de-f8dce8156a58"]) end end end From 6ce453ab70ea22127114e09c45f9c439638c96bc Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 21 Mar 2019 11:16:26 -0700 Subject: [PATCH 47/64] Delete old file and add blank describe blocks for remaining tests --- .../docker/action/connect_networks_test.rb | 150 ------------------ .../docker/action/prepare_networks_test.rb | 21 +++ 2 files changed, 21 insertions(+), 150 deletions(-) delete mode 100644 test/unit/plugins/providers/docker/action/connect_networks_test.rb diff --git a/test/unit/plugins/providers/docker/action/connect_networks_test.rb b/test/unit/plugins/providers/docker/action/connect_networks_test.rb deleted file mode 100644 index 5e5ea798b..000000000 --- a/test/unit/plugins/providers/docker/action/connect_networks_test.rb +++ /dev/null @@ -1,150 +0,0 @@ -require_relative "../../../../base" -require_relative "../../../../../../plugins/providers/docker/action/connect_networks" - -describe VagrantPlugins::DockerProvider::Action::ConnectNetworks do - include_context "unit" - include_context "virtualbox" - - let(:sandbox) { isolated_environment } - - let(:iso_env) do - # We have to create a Vagrantfile so there is a root path - sandbox.vagrantfile("") - sandbox.create_vagrant_env - end - - let(:machine) do - iso_env.machine(iso_env.machine_names[0], :docker).tap do |m| - allow(m.provider).to receive(:driver).and_return(driver) - allow(m.config.vm).to receive(:networks).and_return(networks) - end - end - - let(:env) {{ machine: machine, ui: machine.ui, root_path: Pathname.new(".") }} - let(:app) { lambda { |*args| }} - let(:driver) { double("driver", create: "abcd1234") } - - let(:networks) { [[:private_network, - {:ip=>"172.20.128.2", - :subnet=>"172.20.0.0/16", - :driver=>"bridge", - :internal=>"true", - :alias=>"mynetwork", - :protocol=>"tcp", - :id=>"80e017d5-388f-4a2f-a3de-f8dce8156a58"}], - [:private_network, - {:type=>"dhcp", - :ipv6=>"true", - :subnet=>"2a02:6b8:b010:9020:1::/80", - :protocol=>"tcp", - :id=>"b8f23054-38d5-45c3-99ea-d33fc5d1b9f2"}], - [:forwarded_port, - {:guest=>22, :host=>2200, :host_ip=>"127.0.0.1", :id=>"ssh", :auto_correct=>true, :protocol=>"tcp"}]] - } - - let(:invalid_network) { - [[:private_network, - {:ipv6=>"true", - :protocol=>"tcp", - :id=>"b8f23054-38d5-45c3-99ea-d33fc5d1b9f2"}]] - } - - subject { described_class.new(app, env) } - - after do - sandbox.close - end - - describe "#call" do - it "calls the next action in the chain" do - allow(driver).to receive(:host_vm?).and_return(false) - allow(driver).to receive(:existing_network?).and_return(false) - allow(driver).to receive(:create_network).and_return(true) - allow(driver).to receive(:connect_network).and_return(true) - allow(driver).to receive(:subnet_defined?).and_return(nil) - - called = false - app = ->(*args) { called = true } - - action = described_class.new(app, env) - action.call(env) - - expect(called).to eq(true) - end - - it "calls the proper driver methods to setup a network" do - allow(driver).to receive(:host_vm?).and_return(false) - allow(driver).to receive(:existing_network?).and_return(false) - allow(driver).to receive(:create_network).and_return(true) - allow(driver).to receive(:connect_network).and_return(true) - allow(driver).to receive(:subnet_defined?).and_return(nil) - - - expect(subject).to receive(:generate_create_cli_arguments). - with(networks[0][1]).and_return(["--subnet=172.20.0.0/16", "--driver=bridge", "--internal=true"]) - expect(subject).to receive(:generate_create_cli_arguments). - with(networks[1][1]).and_return(["--ipv6=true", "--subnet=2a02:6b8:b010:9020:1::/80"]) - expect(subject).to receive(:generate_connect_cli_arguments). - with(networks[0][1]).and_return(["--ipv6=true", "--subnet=2a02:6b8:b010:9020:1::/80"]) - expect(subject).to receive(:generate_connect_cli_arguments). - with(networks[1][1]).and_return([]) - - expect(driver).to receive(:create_network).twice - expect(driver).to receive(:connect_network).twice - - subject.call(env) - end - - it "uses an existing network if a matching subnet is found" do - allow(driver).to receive(:host_vm?).and_return(false) - allow(driver).to receive(:existing_network?).and_return(true) - allow(driver).to receive(:create_network).and_return(true) - allow(driver).to receive(:connect_network).and_return(true) - allow(driver).to receive(:subnet_defined?).and_return("my_cool_subnet_network") - - expect(driver).not_to receive(:create_network) - - subject.call(env) - end - - it "raises an error if an inproper network configuration is given" do - allow(machine.config.vm).to receive(:networks).and_return(invalid_network) - allow(driver).to receive(:host_vm?).and_return(false) - allow(driver).to receive(:existing_network?).and_return(false) - - expect{ subject.call(env) }.to raise_error(VagrantPlugins::DockerProvider::Errors::NetworkInvalidOption) - end - end - - describe "#generate_connect_cli_arguments" do - let(:network_options) { - {:ip=>"172.20.128.2", - :subnet=>"172.20.0.0/16", - :driver=>"bridge", - :internal=>"true", - :alias=>"mynetwork", - :protocol=>"tcp", - :id=>"80e017d5-388f-4a2f-a3de-f8dce8156a58"} } - - it "returns an array of cli arguments" do - cli_args = subject.generate_connect_cli_arguments(network_options) - expect(cli_args).to eq(["--ip", "172.20.128.2", "--alias=mynetwork"]) - end - end - - describe "#generate_create_cli_arguments" do - let(:network_options) { - {:ip=>"172.20.128.2", - :subnet=>"172.20.0.0/16", - :driver=>"bridge", - :internal=>"true", - :alias=>"mynetwork", - :protocol=>"tcp", - :id=>"80e017d5-388f-4a2f-a3de-f8dce8156a58"} } - - it "returns an array of cli arguments" do - cli_args = subject.generate_create_cli_arguments(network_options) - expect(cli_args).to eq(["--subnet=172.20.0.0/16", "--driver=bridge", "--internal=true"]) - end - end -end diff --git a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb index 84b5c829c..1932aabf8 100644 --- a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb +++ b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb @@ -192,4 +192,25 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do expect(cli_args).to eq( ["--ip", "172.20.128.2", "--subnet", "172.20.0.0/16", "--driver", "bridge", "--alias", "mynetwork", "--protocol", "tcp", "--id", "80e017d5-388f-4a2f-a3de-f8dce8156a58"]) end end + + describe "#list_interfaces" do + end + + describe "#validate_network_name!" do + end + + describe "#validate_network_configuration!" do + end + + describe "#process_private_network" do + end + + describe "#process_public_network" do + end + + describe "#request_public_gateway" do + end + + describe "#request_public_iprange" do + end end From 8c169714c5528a78a6093719d11e1aec1ab92141 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 21 Mar 2019 11:39:16 -0700 Subject: [PATCH 48/64] Ensure variable names exist in method --- .../docker/action/prepare_networks.rb | 11 ++++++----- .../docker/action/prepare_networks_test.rb | 19 +++++++++++++++---- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/plugins/providers/docker/action/prepare_networks.rb b/plugins/providers/docker/action/prepare_networks.rb index e1ff59992..fba101204 100644 --- a/plugins/providers/docker/action/prepare_networks.rb +++ b/plugins/providers/docker/action/prepare_networks.rb @@ -42,12 +42,13 @@ module VagrantPlugins # Validates that a network name exists. If it does not # exist, an exception is raised. # - # @param [String] name Name of existing network + # @param [String] network_name Name of existing network + # @param [Hash] env Local call env # @return [Boolean] - def validate_network_name!(name) + def validate_network_name!(network_name, env) if !env[:machine].provider.driver.existing_named_network?(network_name) raise Errors::NetworkNameUndefined, - network_name: name + network_name: network_name end true end @@ -83,7 +84,7 @@ module VagrantPlugins # @param [Hash] env Local call env # @return [String, Hash] Network name and updated network_options def process_private_network(root_options, network_options, env) - if root_options[:name] && validate_network_name!(root_options[:name]) + if root_options[:name] && validate_network_name!(root_options[:name], env) network_name = root_options[:name] end @@ -150,7 +151,7 @@ module VagrantPlugins # @param [Hash] env Local call env # @return [String, Hash] Network name and updated network_options def process_public_network(root_options, net_options, env) - if root_options[:name] && validate_network_name!(root_options[:name]) + if root_options[:name] && validate_network_name!(root_options[:name], env) network_name = root_options[:name] end if !network_name diff --git a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb index 1932aabf8..95663f6f3 100644 --- a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb +++ b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb @@ -105,7 +105,6 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do expect(driver).to receive(:create_network). with("vagrant_network_2a02:6b8:b010:9020:1::/80", ["--ipv6", "--subnet", "2a02:6b8:b010:9020:1::/80"]) - subject.call(env) expect(env[:docker_connects]).to eq({0=>"vagrant_network_172.20.128.0/24", 1=>"vagrant_network_public_wlp4s0", 2=>"vagrant_network_2a02:6b8:b010:9020:1::/80"}) @@ -193,10 +192,22 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do end end - describe "#list_interfaces" do - end - describe "#validate_network_name!" do + let(:netname) { "vagrant_network" } + + it "returns true if name exists" do + allow(driver).to receive(:existing_named_network?).with(netname). + and_return(true) + + expect(subject.validate_network_name!(netname, env)).to be_truthy + end + + it "raises an error if name does not exist" do + allow(driver).to receive(:existing_named_network?).with(netname). + and_return(false) + + expect{subject.validate_network_name!(netname, env)}.to raise_error(VagrantPlugins::DockerProvider::Errors::NetworkNameUndefined) + end end describe "#validate_network_configuration!" do From 32807d70c7f7689837a58d061e9c2d2163b0cf81 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 21 Mar 2019 13:03:39 -0700 Subject: [PATCH 49/64] Mock up public/private network calls for #call test --- .../providers/docker/action/prepare_networks_test.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb index 95663f6f3..82eb22ac0 100644 --- a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb +++ b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb @@ -96,6 +96,16 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do allow(subject).to receive(:request_public_gateway).and_return("1234") allow(subject).to receive(:request_public_iprange).and_return("1234") + + expect(subject).to receive(:process_private_network).with(networks[0][1], {}, env). + and_return(["vagrant_network_172.20.128.0/24", {:ipv6=>false, :subnet=>"172.20.128.0/24"}]) + + expect(subject).to receive(:process_public_network).with(networks[1][1], {}, env). + and_return(["vagrant_network_public_wlp4s0", {"opt"=>"parent=wlp4s0", "subnet"=>"192.168.1.0/24", "driver"=>"macvlan", "gateway"=>"1234", "ipv6"=>false, "ip_range"=>"1234"}]) + + expect(subject).to receive(:process_private_network).with(networks[2][1], {}, env). + and_return(["vagrant_network_2a02:6b8:b010:9020:1::/80", {:ipv6=>true, :subnet=>"2a02:6b8:b010:9020:1::/80"}]) + allow(machine.ui).to receive(:ask).and_return("1") expect(driver).to receive(:create_network). From 98e41eb936948429ede361588816b160edcc18cb Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 21 Mar 2019 13:26:37 -0700 Subject: [PATCH 50/64] Mock out public/private network calls for existing subnet tests --- .../docker/action/prepare_networks_test.rb | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb index 82eb22ac0..3b00215ef 100644 --- a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb +++ b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb @@ -96,7 +96,6 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do allow(subject).to receive(:request_public_gateway).and_return("1234") allow(subject).to receive(:request_public_iprange).and_return("1234") - expect(subject).to receive(:process_private_network).with(networks[0][1], {}, env). and_return(["vagrant_network_172.20.128.0/24", {:ipv6=>false, :subnet=>"172.20.128.0/24"}]) @@ -132,6 +131,9 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do and_return("vagrant_network_public_wlp4s0") allow(driver).to receive(:network_defined?).with("2a02:6b8:b010:9020:1::/80"). and_return("vagrant_network_2a02:6b8:b010:9020:1::/80") + allow(subject).to receive(:request_public_gateway).and_return("1234") + allow(subject).to receive(:request_public_iprange).and_return("1234") + allow(machine.ui).to receive(:ask).and_return("1") expect(driver).to receive(:existing_named_network?). with("vagrant_network_172.20.128.0/24").and_return(true) @@ -142,7 +144,11 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do expect(driver).to receive(:existing_named_network?). with("vagrant_network_2a02:6b8:b010:9020:1::/80").and_return(true) - allow(machine.ui).to receive(:ask).and_return("1") + expect(subject).to receive(:process_private_network).with(networks[0][1], {}, env). + and_return(["vagrant_network_172.20.128.0/24", {:ipv6=>false, :subnet=>"172.20.128.0/24"}]) + + expect(subject).to receive(:process_private_network).with(networks[2][1], {}, env). + and_return(["vagrant_network_2a02:6b8:b010:9020:1::/80", {:ipv6=>true, :subnet=>"2a02:6b8:b010:9020:1::/80"}]) expect(driver).not_to receive(:create_network) @@ -221,6 +227,20 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do end describe "#validate_network_configuration!" do + let(:netname) { "vagrant_network_172.20.128.0/24" } + let(:options) { {:ip=>"172.20.128.2", :subnet=>"172.20.0.0/16", :driver=>"bridge", :internal=>"true", :alias=>"mynetwork", :protocol=>"tcp", :id=>"80e017d5-388f-4a2f-a3de-f8dce8156a58", :netmask=>24} } + let(:network_options) { {:ipv6=>false, :subnet=>"172.20.128.0/24"} } + + it "returns true if all options are valid" do + allow(driver).to receive(:network_containing_address).with(options[:ip]). + and_return(netname) + allow(driver).to receive(:network_containing_address).with(network_options[:subnet]). + and_return(netname) + + expect(subject.validate_network_configuration!(netname, options, network_options, driver)). + to be_truthy + end + end describe "#process_private_network" do From 36a41957c959423b35077095c9d22d3d967d364c Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 21 Mar 2019 13:51:07 -0700 Subject: [PATCH 51/64] Fix travis ci tests for public gateway and ip range issues --- .../providers/docker/action/prepare_networks_test.rb | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb index 3b00215ef..caad13d10 100644 --- a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb +++ b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb @@ -131,25 +131,23 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do and_return("vagrant_network_public_wlp4s0") allow(driver).to receive(:network_defined?).with("2a02:6b8:b010:9020:1::/80"). and_return("vagrant_network_2a02:6b8:b010:9020:1::/80") - allow(subject).to receive(:request_public_gateway).and_return("1234") - allow(subject).to receive(:request_public_iprange).and_return("1234") allow(machine.ui).to receive(:ask).and_return("1") expect(driver).to receive(:existing_named_network?). with("vagrant_network_172.20.128.0/24").and_return(true) expect(driver).to receive(:existing_named_network?). with("vagrant_network_public_wlp4s0").and_return(true) - expect(driver).to receive(:existing_named_network?). - with("vagrant_network_public_wlp4s0").and_return(true) expect(driver).to receive(:existing_named_network?). with("vagrant_network_2a02:6b8:b010:9020:1::/80").and_return(true) expect(subject).to receive(:process_private_network).with(networks[0][1], {}, env). and_return(["vagrant_network_172.20.128.0/24", {:ipv6=>false, :subnet=>"172.20.128.0/24"}]) + expect(subject).to receive(:process_public_network).with(networks[1][1], {}, env). + and_return(["vagrant_network_public_wlp4s0", {"opt"=>"parent=wlp4s0", "subnet"=>"192.168.1.0/24", "driver"=>"macvlan", "gateway"=>"1234", "ipv6"=>false, "ip_range"=>"1234"}]) + expect(subject).to receive(:process_private_network).with(networks[2][1], {}, env). and_return(["vagrant_network_2a02:6b8:b010:9020:1::/80", {:ipv6=>true, :subnet=>"2a02:6b8:b010:9020:1::/80"}]) - expect(driver).not_to receive(:create_network) expect(subject).to receive(:validate_network_configuration!). @@ -158,15 +156,13 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do expect(subject).to receive(:validate_network_configuration!). with("vagrant_network_public_wlp4s0", networks[1][1], - {"opt"=>"parent=wlp4s0", "subnet"=>"192.168.1.0/24", "driver"=>"macvlan", "gateway"=>"192.168.1.1", "ipv6"=>false}, driver) - + {"opt"=>"parent=wlp4s0", "subnet"=>"192.168.1.0/24", "driver"=>"macvlan", "gateway"=>"1234", "ipv6"=>false, "ip_range"=>"1234"}, driver) expect(subject).to receive(:validate_network_configuration!). with("vagrant_network_2a02:6b8:b010:9020:1::/80", networks[2][1], {:ipv6=>true, :subnet=>"2a02:6b8:b010:9020:1::/80"}, driver) subject.call(env) - end it "raises an error if an inproper network configuration is given" do From 2bc6fa854ae9fdd31dedda00b7c3b94a79e4fcb8 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 21 Mar 2019 13:51:24 -0700 Subject: [PATCH 52/64] Add tests for validating network configurations --- .../docker/action/prepare_networks_test.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb index caad13d10..ba903ed84 100644 --- a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb +++ b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb @@ -237,6 +237,22 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do to be_truthy end + it "raises an error of the address is invalid" do + allow(driver).to receive(:network_containing_address).with(options[:ip]). + and_return("fakename") + expect{subject.validate_network_configuration!(netname, options, network_options, driver)}. + to raise_error(VagrantPlugins::DockerProvider::Errors::NetworkAddressInvalid) + end + + it "raises an error of the subnet is invalid" do + allow(driver).to receive(:network_containing_address).with(options[:ip]). + and_return(netname) + allow(driver).to receive(:network_containing_address).with(network_options[:subnet]). + and_return("fakename") + + expect{subject.validate_network_configuration!(netname, options, network_options, driver)}. + to raise_error(VagrantPlugins::DockerProvider::Errors::NetworkSubnetInvalid) + end end describe "#process_private_network" do From 82700d95b3d5e78e12f3d428ebf12d28ffc96628 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 21 Mar 2019 15:29:04 -0700 Subject: [PATCH 53/64] Ensure subnet is used if specified from user config options --- .../docker/action/prepare_networks.rb | 11 ++++++++-- .../docker/action/prepare_networks_test.rb | 22 +++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/plugins/providers/docker/action/prepare_networks.rb b/plugins/providers/docker/action/prepare_networks.rb index fba101204..21d3607af 100644 --- a/plugins/providers/docker/action/prepare_networks.rb +++ b/plugins/providers/docker/action/prepare_networks.rb @@ -124,11 +124,18 @@ module VagrantPlugins # With no network name, process options to find or determine # name for new network if !network_name - subnet = IPAddr.new("#{addr}/#{root_options[:netmask]}") - network = "#{subnet}/#{root_options[:netmask]}" + if !root_options[:subnet] + # Only generate a subnet if not given one + subnet = IPAddr.new("#{addr}/#{root_options[:netmask]}") + network = "#{subnet}/#{root_options[:netmask]}" + else + network = root_options[:subnet] + end + network_options[:subnet] = network existing_network = env[:machine].provider.driver. network_defined?(network) + if !existing_network network_name = "vagrant_network_#{network}" else diff --git a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb index ba903ed84..4f42e84f3 100644 --- a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb +++ b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb @@ -256,6 +256,28 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do end describe "#process_private_network" do + let(:options) { {:ip=>"172.20.128.2", :subnet=>"172.20.0.0/16", :driver=>"bridge", :internal=>"true", :alias=>"mynetwork", :protocol=>"tcp", :id=>"80e017d5-388f-4a2f-a3de-f8dce8156a58", :netmask=>24} } + let(:dhcp_options) { {type: "dhcp"} } + let(:bad_options) { {driver: "bridge"} } + + it "generates a network name and config for a dhcp private network" do + network_name, network_options = subject.process_private_network(dhcp_options, {}, env) + + expect(network_name).to eq("vagrant_network") + expect(network_options).to eq({}) + end + + it "generates a network name and options for a static ip" do + allow(driver).to receive(:network_defined?).and_return(nil) + network_name, network_options = subject.process_private_network(options, {}, env) + expect(network_name).to eq("vagrant_network_172.20.0.0/16") + expect(network_options).to eq({:ipv6=>false, :subnet=>"172.20.0.0/16"}) + end + + it "raises an error if no ip address or type `dhcp` was given" do + expect{subject.process_private_network(bad_options, {}, env)}. + to raise_error(VagrantPlugins::DockerProvider::Errors::NetworkIPAddressRequired) + end end describe "#process_public_network" do From 88a18fe2c5f2cee5eb1686eaa9a43709b1868038 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 21 Mar 2019 16:06:11 -0700 Subject: [PATCH 54/64] Add public network tests for docker provider --- plugins/providers/docker/errors.rb | 4 ++++ templates/locales/providers_docker.yml | 3 +++ .../docker/action/prepare_networks_test.rb | 15 +++++++++++++++ 3 files changed, 22 insertions(+) diff --git a/plugins/providers/docker/errors.rb b/plugins/providers/docker/errors.rb index 84b5e8f4e..11ae6fd68 100644 --- a/plugins/providers/docker/errors.rb +++ b/plugins/providers/docker/errors.rb @@ -69,6 +69,10 @@ module VagrantPlugins error_key(:network_name_undefined) end + class NetworkNoInterfaces < DockerError + error_key(:network_no_interfaces) + end + class PackageNotSupported < DockerError error_key(:package_not_supported) end diff --git a/templates/locales/providers_docker.yml b/templates/locales/providers_docker.yml index 00c929e8f..5c6b00018 100644 --- a/templates/locales/providers_docker.yml +++ b/templates/locales/providers_docker.yml @@ -252,6 +252,9 @@ en: The Docker provider was unable to configure networking using the provided network name `%{network_name}`. Please ensure the network name is correct and exists, then try again. + network_no_interfaces: |- + The Docker provider was unable to list any available interfaces to bridge + the public network with. network_subnet_invalid: |- The configured network subnet is not valid for the defined network. Please update the network settings and try again. diff --git a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb index 4f42e84f3..f4e1504e0 100644 --- a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb +++ b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb @@ -281,6 +281,21 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do end describe "#process_public_network" do + let(:options) { {:ip=>"172.30.130.2", :subnet=>"172.30.0.0/16", :driver=>"bridge", :id=>"30e017d5-488f-5a2f-a3ke-k8dce8246b60"} } + + it "raises an error if there are no network interfaces" do + expect(subject).to receive(:list_interfaces).and_return([]) + + expect{subject.process_public_network(options, {}, env)}. + to raise_error(VagrantPlugins::DockerProvider::Errors::NetworkNoInterfaces) + end + + it "generates a network name and configuration" do + allow(machine.ui).to receive(:ask).and_return("1") + + network_name, network_options = subject.process_public_network(options, {}, env) + expect(network_name).to eq("vagrant_network_public") + end end describe "#request_public_gateway" do From 6bffdca97297879a93da2d12db3ea66bf008b928 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Thu, 21 Mar 2019 16:06:24 -0700 Subject: [PATCH 55/64] Add beginning of connect network tests for docker provider --- .../docker/action/connect_networks.rb | 6 +- .../docker/action/connect_networks_test.rb | 92 +++++++++++++++++++ 2 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 test/unit/plugins/providers/docker/action/connect_networks_test.rb diff --git a/plugins/providers/docker/action/connect_networks.rb b/plugins/providers/docker/action/connect_networks.rb index 929701579..b1d1670a5 100644 --- a/plugins/providers/docker/action/connect_networks.rb +++ b/plugins/providers/docker/action/connect_networks.rb @@ -1,6 +1,8 @@ require 'ipaddr' require 'log4r' +require 'vagrant/util/scoped_hash_override' + module VagrantPlugins module DockerProvider module Action @@ -20,9 +22,9 @@ module VagrantPlugins def generate_connect_cli_arguments(options) options.map do |key, value| # If value is false, option is not set - next if value == false + next if value.to_s == "false" # If value is true, consider feature flag with no value - opt = value == true ? [] : [value] + opt = value.to_s == "true" ? [] : [value] opt.unshift("--#{key.to_s.tr("_", "-")}") end.flatten.compact end diff --git a/test/unit/plugins/providers/docker/action/connect_networks_test.rb b/test/unit/plugins/providers/docker/action/connect_networks_test.rb new file mode 100644 index 000000000..2ff29565d --- /dev/null +++ b/test/unit/plugins/providers/docker/action/connect_networks_test.rb @@ -0,0 +1,92 @@ +require_relative "../../../../base" +require_relative "../../../../../../plugins/providers/docker/action/connect_networks" + + +describe VagrantPlugins::DockerProvider::Action::ConnectNetworks do + include_context "unit" + include_context "virtualbox" + + let(:sandbox) { isolated_environment } + + let(:iso_env) do + # We have to create a Vagrantfile so there is a root path + sandbox.vagrantfile("") + sandbox.create_vagrant_env + end + + let(:machine) do + iso_env.machine(iso_env.machine_names[0], :docker).tap do |m| + allow(m.provider).to receive(:driver).and_return(driver) + allow(m.config.vm).to receive(:networks).and_return(networks) + end + end + + let(:docker_connects) { {0=>"vagrant_network_172.20.128.0/24", 1=>"vagrant_network_public_wlp4s0", 2=>"vagrant_network_2a02:6b8:b010:9020:1::/80"} } + + let(:env) {{ machine: machine, ui: machine.ui, root_path: Pathname.new("."), + docker_connects: docker_connects }} + let(:app) { lambda { |*args| }} + let(:driver) { double("driver", create: "abcd1234") } + + let(:networks) { [[:private_network, + {:ip=>"172.20.128.2", + :subnet=>"172.20.0.0/16", + :driver=>"bridge", + :internal=>"true", + :alias=>"mynetwork", + :protocol=>"tcp", + :id=>"80e017d5-388f-4a2f-a3de-f8dce8156a58"}], + [:public_network, + {:ip=>"172.30.130.2", + :subnet=>"172.30.0.0/16", + :driver=>"bridge", + :id=>"30e017d5-488f-5a2f-a3ke-k8dce8246b60"}], + [:private_network, + {:type=>"dhcp", + :ipv6=>"true", + :subnet=>"2a02:6b8:b010:9020:1::/80", + :protocol=>"tcp", + :id=>"b8f23054-38d5-45c3-99ea-d33fc5d1b9f2"}], + [:forwarded_port, + {:guest=>22, :host=>2200, :host_ip=>"127.0.0.1", :id=>"ssh", :auto_correct=>true, :protocol=>"tcp"}]] + } + + subject { described_class.new(app, env) } + + after do + sandbox.close + end + + describe "#call" do + it "calls the next action in the chain" do + allow(driver).to receive(:host_vm?).and_return(false) + allow(driver).to receive(:connect_network).and_return(true) + + called = false + app = ->(*args) { called = true } + + action = described_class.new(app, env) + + action.call(env) + + expect(called).to eq(true) + end + + it "connects all of the avaiable networks to a container" do + end + + it "raises an error if the network name is missing" do + end + end + + describe "#generate_connect_cli_arguments" do + it "removes false values" do + end + + it "removes true and leaves flag value in arguments" do + end + + it "takes options and generates cli flags" do + end + end +end From b56f89775ddff54d69ff0042950211b695b41b75 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 22 Mar 2019 08:24:22 -0700 Subject: [PATCH 56/64] Add more mocks for public network Ensure test values are used rather than real values from machine --- .../providers/docker/action/prepare_networks_test.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb index f4e1504e0..22f2aae90 100644 --- a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb +++ b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb @@ -282,6 +282,7 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do describe "#process_public_network" do let(:options) { {:ip=>"172.30.130.2", :subnet=>"172.30.0.0/16", :driver=>"bridge", :id=>"30e017d5-488f-5a2f-a3ke-k8dce8246b60"} } + let(:ipaddr) { double("ipaddr", prefix: 22, succ: "10.1.10.2", ipv6?: false) } it "raises an error if there are no network interfaces" do expect(subject).to receive(:list_interfaces).and_return([]) @@ -292,6 +293,12 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do it "generates a network name and configuration" do allow(machine.ui).to receive(:ask).and_return("1") + allow(subject).to receive(:request_public_gateway).and_return("1234") + allow(subject).to receive(:request_public_iprange).and_return("1234") + allow(IPAddr).to receive(:new).and_return(ipaddr) + allow(driver).to receive(:existing_named_network?).and_return(false) + allow(driver).to receive(:network_containing_address). + with("10.1.10.2").and_return("vagrant_network_public") network_name, network_options = subject.process_public_network(options, {}, env) expect(network_name).to eq("vagrant_network_public") From 36f2aaf55e34706689846d11220b5403810963dd Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 22 Mar 2019 08:45:14 -0700 Subject: [PATCH 57/64] Add test for public network gateway request --- .../docker/action/prepare_networks_test.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb index 22f2aae90..2b5b0fb4e 100644 --- a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb +++ b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb @@ -306,6 +306,19 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do end describe "#request_public_gateway" do + let(:options) { {:ip=>"172.30.130.2", :subnet=>"172.30.0.0/16", :driver=>"bridge", :id=>"30e017d5-488f-5a2f-a3ke-k8dce8246b60"} } + let(:ipaddr) { double("ipaddr", to_s: "172.30.130.2", prefix: 22, succ: "172.30.130.3", + ipv6?: false) } + + it "requests a gateway" do + allow(IPAddr).to receive(:new).and_return(ipaddr) + allow(ipaddr).to receive(:include?).and_return(false) + allow(machine.ui).to receive(:ask).and_return("1") + + addr = subject.request_public_gateway(options, "bridge", env) + + expect(addr).to eq("172.30.130.2") + end end describe "#request_public_iprange" do From 4f80a9e6d500204e5df2910cb89e25c153cf3036 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 22 Mar 2019 09:02:40 -0700 Subject: [PATCH 58/64] Add test for requesting public ip range for docker network provider --- .../providers/docker/action/prepare_networks.rb | 1 + .../docker/action/prepare_networks_test.rb | 14 ++++++++++++++ 2 files changed, 15 insertions(+) diff --git a/plugins/providers/docker/action/prepare_networks.rb b/plugins/providers/docker/action/prepare_networks.rb index 21d3607af..4d1904e85 100644 --- a/plugins/providers/docker/action/prepare_networks.rb +++ b/plugins/providers/docker/action/prepare_networks.rb @@ -282,6 +282,7 @@ module VagrantPlugins begin range = IPAddr.new(range) if !subnet.include?(range) + puts "we in here" env[:ui].warn(I18n.t( "docker_provider.network_bridge_iprange_outofbounds", subnet: network_options[:subnet], diff --git a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb index 2b5b0fb4e..1a2ccb7eb 100644 --- a/test/unit/plugins/providers/docker/action/prepare_networks_test.rb +++ b/test/unit/plugins/providers/docker/action/prepare_networks_test.rb @@ -322,5 +322,19 @@ describe VagrantPlugins::DockerProvider::Action::PrepareNetworks do end describe "#request_public_iprange" do + let(:options) { {:ip=>"172.30.130.2", :subnet=>"172.30.0.0/16", :driver=>"bridge", :id=>"30e017d5-488f-5a2f-a3ke-k8dce8246b60"} } + let(:ipaddr) { double("ipaddr", to_s: "172.30.100.2", prefix: 22, succ: "172.30.100.3", + ipv6?: false) } + let(:subnet) { double("ipaddr", to_s: "172.30.130.2", prefix: 22, succ: "172.30.130.3", + ipv6?: false) } + + it "requests a public ip range" do + allow(IPAddr).to receive(:new).with(options[:subnet]).and_return(subnet) + allow(IPAddr).to receive(:new).with("172.30.130.2").and_return(ipaddr) + allow(subnet).to receive(:include?).and_return(true) + allow(machine.ui).to receive(:ask).and_return(options[:ip]) + + addr = subject.request_public_iprange(options, "bridge", env) + end end end From f8744b66f0141405110eee013c317ad0c367585e Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 22 Mar 2019 09:24:26 -0700 Subject: [PATCH 59/64] Add connect cli argument tests --- .../docker/action/connect_networks_test.rb | 40 ++++++++++++++++++- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/test/unit/plugins/providers/docker/action/connect_networks_test.rb b/test/unit/plugins/providers/docker/action/connect_networks_test.rb index 2ff29565d..8d57f1d4b 100644 --- a/test/unit/plugins/providers/docker/action/connect_networks_test.rb +++ b/test/unit/plugins/providers/docker/action/connect_networks_test.rb @@ -16,12 +16,14 @@ describe VagrantPlugins::DockerProvider::Action::ConnectNetworks do let(:machine) do iso_env.machine(iso_env.machine_names[0], :docker).tap do |m| + allow(m).to receive(:id).and_return("12345") allow(m.provider).to receive(:driver).and_return(driver) + allow(m.provider).to receive(:host_vm?).and_return(false) allow(m.config.vm).to receive(:networks).and_return(networks) end end - let(:docker_connects) { {0=>"vagrant_network_172.20.128.0/24", 1=>"vagrant_network_public_wlp4s0", 2=>"vagrant_network_2a02:6b8:b010:9020:1::/80"} } + let(:docker_connects) { {0=>"vagrant_network_172.20.0.0/16", 1=>"vagrant_network_public_wlp4s0", 2=>"vagrant_network_2a02:6b8:b010:9020:1::/80"} } let(:env) {{ machine: machine, ui: machine.ui, root_path: Pathname.new("."), docker_connects: docker_connects }} @@ -73,20 +75,54 @@ describe VagrantPlugins::DockerProvider::Action::ConnectNetworks do end it "connects all of the avaiable networks to a container" do + expect(driver).to receive(:connect_network).with("vagrant_network_172.20.0.0/16", "12345", ["--ip", "172.20.128.2", "--alias", "mynetwork"]) + expect(driver).to receive(:connect_network).with("vagrant_network_public_wlp4s0", "12345", ["--ip", "172.30.130.2"]) + expect(driver).to receive(:connect_network).with("vagrant_network_2a02:6b8:b010:9020:1::/80", "12345", []) + + subject.call(env) end - it "raises an error if the network name is missing" do + context "with missing env values" do + it "raises an error if the network name is missing" do + env[:docker_connects] = {} + + expect{subject.call(env)}.to raise_error(VagrantPlugins::DockerProvider::Errors::NetworkNameMissing) + end end end describe "#generate_connect_cli_arguments" do + let(:network_options) { + {:ip=>"172.20.128.2", + :subnet=>"172.20.0.0/16", + :driver=>"bridge", + :internal=>"true", + :alias=>"mynetwork", + :protocol=>"tcp", + :id=>"80e017d5-388f-4a2f-a3de-f8dce8156a58"} } + + let(:false_network_options) { + {:ip=>"172.20.128.2", + :subnet=>"172.20.0.0/16", + :driver=>"bridge", + :internal=>"false", + :alias=>"mynetwork", + :protocol=>"tcp", + :id=>"80e017d5-388f-4a2f-a3de-f8dce8156a58"} } + it "removes false values" do + cli_args = subject.generate_connect_cli_arguments(false_network_options) + expect(cli_args).to eq(["--ip", "172.20.128.2", "--subnet", "172.20.0.0/16", "--driver", "bridge", "--alias", "mynetwork", "--protocol", "tcp", "--id", "80e017d5-388f-4a2f-a3de-f8dce8156a58"]) end it "removes true and leaves flag value in arguments" do + cli_args = subject.generate_connect_cli_arguments(network_options) + expect(cli_args).to eq(["--ip", "172.20.128.2", "--subnet", "172.20.0.0/16", "--driver", "bridge", "--internal", "--alias", "mynetwork", "--protocol", "tcp", "--id", "80e017d5-388f-4a2f-a3de-f8dce8156a58"]) end it "takes options and generates cli flags" do + cli_args = subject.generate_connect_cli_arguments(network_options) + expect(cli_args).to eq(["--ip", "172.20.128.2", "--subnet", "172.20.0.0/16", "--driver", "bridge", "--internal", "--alias", "mynetwork", "--protocol", "tcp", "--id", "80e017d5-388f-4a2f-a3de-f8dce8156a58"]) end end end From 979dd37e46cb67e3a5cf0efd4dd5724ea429cecc Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 22 Mar 2019 10:10:44 -0700 Subject: [PATCH 60/64] Remove comment about private networks only working with docker provider --- website/source/docs/docker/networking.html.md | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/website/source/docs/docker/networking.html.md b/website/source/docs/docker/networking.html.md index 0f3b4b295..e6996c4d0 100644 --- a/website/source/docs/docker/networking.html.md +++ b/website/source/docs/docker/networking.html.md @@ -31,15 +31,12 @@ there are no more containers using the network. ## Docker Network Options -Only the network option `:private_network` is currently supported with the docker -provider in Vagrant. - -Most of the options given to `:private_network` align with the command line flags +Most of the options given align with the command line flags for the [docker network create](https://docs.docker.com/engine/reference/commandline/network_create/) command. However, if you want the container to have a specific IP instead of using DHCP, you also will have to specify a subnet due to how docker networks behave. -It should also be noted that if you want a specific IPv6 address, your `:private_network` +It should also be noted that if you want a specific IPv6 address, your network option should use `ip6` rather than `ip`. If you just want to use DHCP, you can simply say `type: "dhcp"` insetad. More examples are shared below which demonstrate creating a few common network interfaces. From 9d200bdb72b19fd1abe4d0389473cb97bd9702a4 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Fri, 22 Mar 2019 13:50:13 -0700 Subject: [PATCH 61/64] Add note about specific create and connect flag options for Vagrant --- website/source/docs/docker/networking.html.md | 23 ++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/website/source/docs/docker/networking.html.md b/website/source/docs/docker/networking.html.md index e6996c4d0..258c2cbf6 100644 --- a/website/source/docs/docker/networking.html.md +++ b/website/source/docs/docker/networking.html.md @@ -33,8 +33,25 @@ there are no more containers using the network. Most of the options given align with the command line flags for the [docker network create](https://docs.docker.com/engine/reference/commandline/network_create/) -command. However, if you want the container to have a specific IP instead of using -DHCP, you also will have to specify a subnet due to how docker networks behave. +command. If there are any specific options you want to enable from the `docker network create` +command, you can specify them like this: + +```ruby +docker.vm.network :private_network, type: "dhcp", docker_network__internal: true +``` + +This will enable the `internal` option for the network when created with `docker network create`. + +Where `option` corresponds to the given flag that will be provided to the `docker network create` +command. Similarly, if there is a value you wish to enable when connecting a container +to a given network, you can use the following value in your network config: + +```ruby +docker_connect__option: "value" +``` + +It should be noted that if you want the container to have a specific IP instead +of using DHCP, you also will have to specify a subnet due to how docker networks behave. It should also be noted that if you want a specific IPv6 address, your network option should use `ip6` rather than `ip`. If you just want to use DHCP, you can @@ -52,7 +69,7 @@ The following Vagrantfile will generate these networks for a container: ```ruby Vagrant.configure("2") do |config| config.vm.define "docker" do |docker| - docker.vm.network :private_network, type: "dhcp" + docker.vm.network :private_network, type: "dhcp", docker_network__internal: true docker.vm.network :private_network, ip: "172.20.128.2", subnet: "172.20.0.0/16" docker.vm.network :private_network, type: "dhcp", ipv6: "true", subnet: "2a02:6b8:b010:9020:1::/80" From f4c6aa8aa1dedc58adbd94e19d85b04bdf4b0642 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Mon, 25 Mar 2019 13:25:49 -0700 Subject: [PATCH 62/64] Update docker provider networking docs from pull request feedback --- website/source/docs/docker/networking.html.md | 63 ++++++++++--------- 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/website/source/docs/docker/networking.html.md b/website/source/docs/docker/networking.html.md index 258c2cbf6..f7a8045d8 100644 --- a/website/source/docs/docker/networking.html.md +++ b/website/source/docs/docker/networking.html.md @@ -31,8 +31,16 @@ there are no more containers using the network. ## Docker Network Options -Most of the options given align with the command line flags -for the [docker network create](https://docs.docker.com/engine/reference/commandline/network_create/) +Most of the options work similar to other Vagrant providers. Defining either an +ip or using `type: 'dhcp'` will give you a network on your container. + +```ruby +docker.vm.network :private_network, type: "dhcp" +docker.vm.network :private_network, ip: "172.20.128.2" +``` + +If you want to set something specific with a new network you can use scoped options +which align with the command line flags for the [docker network create](https://docs.docker.com/engine/reference/commandline/network_create/) command. If there are any specific options you want to enable from the `docker network create` command, you can specify them like this: @@ -50,13 +58,13 @@ to a given network, you can use the following value in your network config: docker_connect__option: "value" ``` -It should be noted that if you want the container to have a specific IP instead -of using DHCP, you also will have to specify a subnet due to how docker networks behave. +The docker provider will automatically determine a usable subnet for your containers +network, however if you wish to specify a subnet and or netmask, you can use the +`subnet` and or `netmask` options for this. By default, Vagrant will use a netmask +of `/24` for IPv4 and `/64` for IPv6. -It should also be noted that if you want a specific IPv6 address, your network -option should use `ip6` rather than `ip`. If you just want to use DHCP, you can -simply say `type: "dhcp"` insetad. More examples are shared below which demonstrate -creating a few common network interfaces. +More examples are shared below which demonstrate creating a few common network +interfaces. ## Docker Network Example @@ -71,8 +79,8 @@ Vagrant.configure("2") do |config| config.vm.define "docker" do |docker| docker.vm.network :private_network, type: "dhcp", docker_network__internal: true docker.vm.network :private_network, - ip: "172.20.128.2", subnet: "172.20.0.0/16" - docker.vm.network :private_network, type: "dhcp", ipv6: "true", subnet: "2a02:6b8:b010:9020:1::/80" + ip: "172.20.128.2", netmask: "16" + docker.vm.network :private_network, type: "dhcp", subnet: "2a02:6b8:b010:9020:1::/80" docker.vm.provider "docker" do |d| d.build_dir = "docker_build_dir" end @@ -116,32 +124,27 @@ brian@localghost:vagrant-sandbox % docker exec 370f4e5d2217 ip addr valid_lft forever preferred_lft forever ``` -## Useful Debugging Tips +You can also connect your containers to a docker network that was created outside +of Vagrant: -If you provide Vagrant with a faulty config option when setting up a network, Vagrant -will pass that option along to the `docker network` commands it uses. That command -line tool should give you some insight if there is something wrong with the option -you configured: +``` +$ docker network create my-custom-network --subnet=172.20.0.0/16 +``` ```ruby -docker.vm.network :private_network, - ip: "172.20.128.2", subnet: "172.20.0.0/16", - unsupported: "option" +Vagrant.configure("2") do |config| + config.vm.define "docker" do |docker| + docker.vm.network :private_network, type: "dhcp" name: "my-custom-network" + docker.vm.provider "docker" do |d| + d.build_dir = "docker_build_dir" + end + end +end ``` -``` -A Docker command executed by Vagrant didn't complete successfully! -The command run along with the output from the command is shown -below. +Vagrant will not delete or modify these outside networks when deleting the container, however. -Command: ["docker", "network", "create", "vagrant_network_172.20.0.0/16", "--subnet=172.20.0.0/16", "--unsupported=option", {:notify=>[:stdout, :stderr]}] - -Stderr: unknown flag: --unsupported -See 'docker network create --help'. - - -Stdout: -``` +## Useful Debugging Tips The `docker network` command provides some helpful insights to what might be going on with the networks Vagrant creates. For example, if you want to know what networks From 1e856ecf8ab116db8490a34a1241ce7c61dbe508 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 25 Mar 2019 15:13:19 -0700 Subject: [PATCH 63/64] Add information on public_network for docker provider --- website/source/docs/docker/networking.html.md | 63 +++++++++++++++++-- 1 file changed, 59 insertions(+), 4 deletions(-) diff --git a/website/source/docs/docker/networking.html.md b/website/source/docs/docker/networking.html.md index f7a8045d8..7ce082e24 100644 --- a/website/source/docs/docker/networking.html.md +++ b/website/source/docs/docker/networking.html.md @@ -58,10 +58,65 @@ to a given network, you can use the following value in your network config: docker_connect__option: "value" ``` -The docker provider will automatically determine a usable subnet for your containers -network, however if you wish to specify a subnet and or netmask, you can use the -`subnet` and or `netmask` options for this. By default, Vagrant will use a netmask -of `/24` for IPv4 and `/64` for IPv6. +When the docker provider creates a new network a netmask is required. If the netmask +is not provided, Vagrant will default to a `/24` for IPv4 and `/64` for IPv6. To provide +a different mask, set it using the `netmask` option: + +```ruby +docker.vm.network :private_network, ip: "172.20.128.2", netmask: 16 +``` + +For networks which set the type to "dhcp", it is also possible to specify a specific +subnet for the network connection. This allows containers to connect to networks other +than the default `vagrant_network` network. The docker provider supports specifying +the desired subnet in two ways. The first is by using the `ip` and `netmask` options: + +```ruby +docker.vm.network :private_network, type: "dhcp", ip: "172.20.128.0", netmask: 24 +``` + +The second is by using the `subnet` option: + +```ruby +docker.vm.network :private_network, type: "dhcp", subnet: "172.20.128.0/24" +``` + +### Public Networks + +The Vagrant docker provider also supports defining public networks. The easiest way +to define a public network is by setting the `type` option to "dhcp": + +```ruby +docker.vm.network :public_network, type: "dhcp" +``` + +A bridge interface is required when setting up a public network. When no bridge +device name is provided, Vagrant will prompt for the appropriate device to use. This +can also be set using the `bridge` option: + +```ruby +docker.vm.network :public_network, type: "dhcp", bridge: "eth0" +``` + +The `bridge` option also supports a list of interfaces which can be used for +setting up the network. Vagrant will inspect the defined interfaces and use +the first active interface when setting up the network: + +```ruby +docker.vm.network :public_network, type: "dhcp", bridge: ["eth0", "wlan0"] +``` + +Finally, the subnet for the bridge interface must be known when setting up +the docker network. Even though a DHCP service may be available on the public +network, docker will manage IP addresses provided to containers. This means +that the subnet provided when defining the network should not be included +within the subnet managed by the DHCP service. Vagrant will prompt for subnet +information, however, it can also be provided in the Vagrantfile using the +`subnet` option: + +```ruby +docker.vm.network :public_network, type: "dhcp", bridge: "eth0", subnet: "192.168.1.252/30" +``` More examples are shared below which demonstrate creating a few common network interfaces. From 8339e63423998323ec88f5e64aa7ef3f96d3aad5 Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 25 Mar 2019 15:26:18 -0700 Subject: [PATCH 64/64] Fix subnet to ip_range and add gateway information for public networks --- website/source/docs/docker/networking.html.md | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/website/source/docs/docker/networking.html.md b/website/source/docs/docker/networking.html.md index 7ce082e24..29961e6a6 100644 --- a/website/source/docs/docker/networking.html.md +++ b/website/source/docs/docker/networking.html.md @@ -106,16 +106,26 @@ the first active interface when setting up the network: docker.vm.network :public_network, type: "dhcp", bridge: ["eth0", "wlan0"] ``` -Finally, the subnet for the bridge interface must be known when setting up +The available IP range for the bridge interface must be known when setting up the docker network. Even though a DHCP service may be available on the public network, docker will manage IP addresses provided to containers. This means -that the subnet provided when defining the network should not be included -within the subnet managed by the DHCP service. Vagrant will prompt for subnet -information, however, it can also be provided in the Vagrantfile using the -`subnet` option: +that the subnet provided when defining the available IP range for the network +should not be included within the subnet managed by the DHCP service. Vagrant +will prompt for the available IP range information, however, it can also be +provided in the Vagrantfile using the `docker_network__ip_range` option: ```ruby -docker.vm.network :public_network, type: "dhcp", bridge: "eth0", subnet: "192.168.1.252/30" +docker.vm.network :public_network, type: "dhcp", bridge: "eth0", docker_network__ip_range: "192.168.1.252/30" +``` + +Finally, the gateway for the interface is required during setup. The docker +provider will default the gateway address to the first address available for +the subnet of the bridge device. Vagrant will prompt for confirmation to use +the default address. The address can also be manually set in the Vagrantfile +using the `docker_network__gateway` option: + +```ruby +docker.vm.network :public_network, type: "dhcp", bridge: "eth0", docker_network__gateway: "192.168.1.2" ``` More examples are shared below which demonstrate creating a few common network