From 5ed5868067152077661f00eb94fb86ea2b29872a Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 12 Mar 2019 10:36:57 -0700 Subject: [PATCH] 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