From fe2710705e94934ed27d4a732521301a54074318 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Fri, 23 Oct 2020 09:31:33 -0700 Subject: [PATCH 01/17] Add --docker-network flag --- cmd/minikube/cmd/start_flags.go | 3 +++ pkg/drivers/kic/types.go | 1 + pkg/minikube/config/types.go | 1 + pkg/minikube/registry/drvs/docker/docker.go | 1 + 4 files changed, 6 insertions(+) diff --git a/cmd/minikube/cmd/start_flags.go b/cmd/minikube/cmd/start_flags.go index 922248bae84c..d25ad1c69b35 100644 --- a/cmd/minikube/cmd/start_flags.go +++ b/cmd/minikube/cmd/start_flags.go @@ -108,6 +108,7 @@ const ( kicBaseImage = "base-image" startOutput = "output" ports = "ports" + dockerNetwork = "docker-network" ) // initMinikubeFlags includes commandline flags for minikube. @@ -148,6 +149,7 @@ func initMinikubeFlags() { startCmd.Flags().Bool(deleteOnFailure, false, "If set, delete the current cluster if start fails and try again. Defaults to false.") startCmd.Flags().Bool(forceSystemd, false, "If set, force the container runtime to use sytemd as cgroup manager. Currently available for docker and crio. Defaults to false.") startCmd.Flags().StringP(startOutput, "o", "text", "Format to print stdout in. Options include: [text,json]") + startCmd.Flags().StringP(dockerNetwork, "", "", "Docker network to run minikube with. Only available with the docker driver. If left empty, minikube will create a new network.") } // initKubernetesFlags inits the commandline flags for Kubernetes related options @@ -292,6 +294,7 @@ func generateClusterConfig(cmd *cobra.Command, existing *config.ClusterConfig, k EmbedCerts: viper.GetBool(embedCerts), MinikubeISO: viper.GetString(isoURL), KicBaseImage: viper.GetString(kicBaseImage), + DockerNetwork: viper.GetString(dockerNetwork), Memory: mem, CPUs: viper.GetInt(cpus), DiskSize: diskSize, diff --git a/pkg/drivers/kic/types.go b/pkg/drivers/kic/types.go index 46bd685e58bb..eacd029d247f 100644 --- a/pkg/drivers/kic/types.go +++ b/pkg/drivers/kic/types.go @@ -61,5 +61,6 @@ type Config struct { Envs map[string]string // key,value of environment variables passed to the node KubernetesVersion string // Kubernetes version to install ContainerRuntime string // container runtime kic is running + DockerNetwork string // docker network to run with kic ExtraArgs []string // a list of any extra option to pass to oci binary during creation time, for example --expose 8080... } diff --git a/pkg/minikube/config/types.go b/pkg/minikube/config/types.go index e56086b96007..76085140c086 100644 --- a/pkg/minikube/config/types.go +++ b/pkg/minikube/config/types.go @@ -72,6 +72,7 @@ type ClusterConfig struct { VerifyComponents map[string]bool // map of components to verify and wait for after start. StartHostTimeout time.Duration ExposedPorts []string // Only used by the docker and podman driver + DockerNetwork string // only used by docker driver } // KubernetesConfig contains the parameters used to configure the VM Kubernetes. diff --git a/pkg/minikube/registry/drvs/docker/docker.go b/pkg/minikube/registry/drvs/docker/docker.go index df2771e80449..c8616c97259e 100644 --- a/pkg/minikube/registry/drvs/docker/docker.go +++ b/pkg/minikube/registry/drvs/docker/docker.go @@ -79,6 +79,7 @@ func configure(cc config.ClusterConfig, n config.Node) (interface{}, error) { KubernetesVersion: cc.KubernetesConfig.KubernetesVersion, ContainerRuntime: cc.KubernetesConfig.ContainerRuntime, ExtraArgs: extraArgs, + DockerNetwork: cc.DockerNetwork, }), nil } From 8b4c48143b1871be3374fd009dd6e3f4afa16438 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Fri, 23 Oct 2020 09:50:23 -0700 Subject: [PATCH 02/17] implement --docker-network flag --- pkg/drivers/kic/kic.go | 10 +++++++--- pkg/drivers/kic/oci/network_create.go | 11 ++++++++--- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index 10c5aebdcc5d..6ee4284e6928 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -83,10 +83,14 @@ func (d *Driver) Create() error { APIServerPort: d.NodeConfig.APIServerPort, } - if gateway, err := oci.CreateNetwork(d.OCIBinary, d.NodeConfig.ClusterName); err != nil { + networkName := d.NodeConfig.DockerNetwork + if networkName == "" { + networkName = d.NodeConfig.ClusterName + } + if gateway, err := oci.CreateNetwork(d.OCIBinary, networkName); err != nil { out.WarningT("Unable to create dedicated network, this might result in cluster IP change after restart: {{.error}}", out.V{"error": err}) - } else { - params.Network = d.NodeConfig.ClusterName + } else if gateway != nil { + params.Network = networkName ip := gateway.To4() // calculate the container IP based on guessing the machine index ip[3] += byte(driver.IndexFromMachineName(d.NodeConfig.MachineName)) diff --git a/pkg/drivers/kic/oci/network_create.go b/pkg/drivers/kic/oci/network_create.go index f39e2a52a2a9..1226bddf4d78 100644 --- a/pkg/drivers/kic/oci/network_create.go +++ b/pkg/drivers/kic/oci/network_create.go @@ -44,9 +44,14 @@ func CreateNetwork(ociBin string, name string) (net.IP, error) { return createDockerNetwork(name) } -func createDockerNetwork(clusterName string) (net.IP, error) { +func createDockerNetwork(networkName string) (net.IP, error) { + // check if the network is docker native, if this is the case return + if networkName == "bridge" { + return nil, nil + } + // check if the network already exists - subnet, gateway, err := dockerNetworkInspect(clusterName) + subnet, gateway, err := dockerNetworkInspect(networkName) if err == nil { klog.Infof("Found existing network with subnet %s and gateway %s.", subnet, gateway) return gateway, nil @@ -57,7 +62,7 @@ func createDockerNetwork(clusterName string) (net.IP, error) { // Rather than iterate through all of the valid subnets, give up at 20 to avoid a lengthy user delay for something that is unlikely to work. // will be like 192.168.49.0/24 ,...,192.168.239.0/24 for attempts < 20 { - gateway, err = tryCreateDockerNetwork(subnetAddr, defaultSubnetMask, clusterName) + gateway, err = tryCreateDockerNetwork(subnetAddr, defaultSubnetMask, networkName) if err == nil { return gateway, nil } From 9474740fd6305e7ee0d9b24b536f6b374b958b59 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Fri, 23 Oct 2020 09:55:48 -0700 Subject: [PATCH 03/17] Add integration test --- test/integration/docker_network_test.go | 59 +++++++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 test/integration/docker_network_test.go diff --git a/test/integration/docker_network_test.go b/test/integration/docker_network_test.go new file mode 100644 index 000000000000..7d700f1e8940 --- /dev/null +++ b/test/integration/docker_network_test.go @@ -0,0 +1,59 @@ +// +build integration + +/* +Copyright 2020 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package integration + +import ( + "context" + "os/exec" + "testing" +) + +func TestDockerNetworkFlag(t *testing.T) { + if !KicDriver() { + t.Skip("only runs with docker driver") + } + + tests := []struct { + description string + flag string + }{ + { + description: "create custom network", + flag: "--docker-network=\"\"", + }, { + description: "use default bridge network", + flag: "--docker-network=bridge", + }, + } + + for _, test := range tests { + t.Run(test.description, func(t *testing.T) { + profile := UniqueProfileName("docker-network") + ctx, cancel := context.WithTimeout(context.Background(), Minutes(5)) + defer Cleanup(t, profile, cancel) + + startArgs := []string{"start", "-p", profile, test.flag} + c := exec.CommandContext(ctx, Target(), startArgs...) + rr, err := Run(t, c) + if err != nil { + t.Fatalf("%v failed: %v\n%v", rr.Command(), err, rr.Output()) + } + }) + } +} From e807362329260c4bec0fce37134c89f81802f423 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Fri, 23 Oct 2020 10:04:11 -0700 Subject: [PATCH 04/17] update docs --- site/content/en/docs/commands/start.md | 1 + 1 file changed, 1 insertion(+) diff --git a/site/content/en/docs/commands/start.md b/site/content/en/docs/commands/start.md index 43c6c5108135..f0a1172b1ef6 100644 --- a/site/content/en/docs/commands/start.md +++ b/site/content/en/docs/commands/start.md @@ -38,6 +38,7 @@ minikube start [flags] --dns-domain string The cluster dns domain name used in the Kubernetes cluster (default "cluster.local") --dns-proxy Enable proxy for NAT DNS requests (virtualbox driver only) --docker-env stringArray Environment variables to pass to the Docker daemon. (format: key=value) + --docker-network string Docker network to run minikube with. Only available with the docker driver. If left empty, minikube will create a new network. --docker-opt stringArray Specify arbitrary flags to pass to the Docker daemon. (format: key=value) --download-only If true, only download and cache files for later use - don't install or start anything. --driver string Used to specify the driver to run Kubernetes in. The list of available drivers depends on operating system. From 20db128cb8d0d2878a70c79df0e25a03a82c5f2a Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Fri, 23 Oct 2020 13:10:01 -0700 Subject: [PATCH 05/17] add warning --- pkg/drivers/kic/oci/network_create.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/drivers/kic/oci/network_create.go b/pkg/drivers/kic/oci/network_create.go index 1226bddf4d78..d9a60eac6ee8 100644 --- a/pkg/drivers/kic/oci/network_create.go +++ b/pkg/drivers/kic/oci/network_create.go @@ -27,6 +27,7 @@ import ( "github.com/pkg/errors" "k8s.io/klog/v2" + "k8s.io/minikube/pkg/minikube/out" ) // firstSubnetAddr subnet to be used on first kic cluster @@ -47,6 +48,7 @@ func CreateNetwork(ociBin string, name string) (net.IP, error) { func createDockerNetwork(networkName string) (net.IP, error) { // check if the network is docker native, if this is the case return if networkName == "bridge" { + out.WarningT("Using the bridge network may result in minikube IP changing on restart") return nil, nil } From ea6ca08d673598a5bc73d3caf79cb765612c92af Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 26 Oct 2020 11:52:26 -0700 Subject: [PATCH 06/17] Change flag name to --network and exit if used without docker driver --- cmd/minikube/cmd/start_flags.go | 10 +++++++--- pkg/drivers/kic/kic.go | 2 +- pkg/drivers/kic/types.go | 2 +- pkg/minikube/config/types.go | 2 +- pkg/minikube/registry/drvs/docker/docker.go | 2 +- 5 files changed, 11 insertions(+), 7 deletions(-) diff --git a/cmd/minikube/cmd/start_flags.go b/cmd/minikube/cmd/start_flags.go index d25ad1c69b35..e6277d4e1519 100644 --- a/cmd/minikube/cmd/start_flags.go +++ b/cmd/minikube/cmd/start_flags.go @@ -108,7 +108,7 @@ const ( kicBaseImage = "base-image" startOutput = "output" ports = "ports" - dockerNetwork = "docker-network" + containerNetwork = "network" ) // initMinikubeFlags includes commandline flags for minikube. @@ -149,7 +149,7 @@ func initMinikubeFlags() { startCmd.Flags().Bool(deleteOnFailure, false, "If set, delete the current cluster if start fails and try again. Defaults to false.") startCmd.Flags().Bool(forceSystemd, false, "If set, force the container runtime to use sytemd as cgroup manager. Currently available for docker and crio. Defaults to false.") startCmd.Flags().StringP(startOutput, "o", "text", "Format to print stdout in. Options include: [text,json]") - startCmd.Flags().StringP(dockerNetwork, "", "", "Docker network to run minikube with. Only available with the docker driver. If left empty, minikube will create a new network.") + startCmd.Flags().StringP(containerNetwork, "", "", "Docker network to run minikube with. Only available with the docker driver. If left empty, minikube will create a new network.") } // initKubernetesFlags inits the commandline flags for Kubernetes related options @@ -288,13 +288,17 @@ func generateClusterConfig(cmd *cobra.Command, existing *config.ClusterConfig, k out.WarningT("With --network-plugin=cni, you will need to provide your own CNI. See --cni flag as a user-friendly alternative") } + if !driver.IsKIC(drvName) && viper.GetString(containerNetwork) != "" { + exit.Advice(reason.Usage, "--network flag is only valid with the docker driver", "remove the --network flag from `minikube start`") + } + cc = config.ClusterConfig{ Name: ClusterFlagValue(), KeepContext: viper.GetBool(keepContext), EmbedCerts: viper.GetBool(embedCerts), MinikubeISO: viper.GetString(isoURL), KicBaseImage: viper.GetString(kicBaseImage), - DockerNetwork: viper.GetString(dockerNetwork), + ContainerNetwork: viper.GetString(containerNetwork), Memory: mem, CPUs: viper.GetInt(cpus), DiskSize: diskSize, diff --git a/pkg/drivers/kic/kic.go b/pkg/drivers/kic/kic.go index 6ee4284e6928..b679ff98b164 100644 --- a/pkg/drivers/kic/kic.go +++ b/pkg/drivers/kic/kic.go @@ -83,7 +83,7 @@ func (d *Driver) Create() error { APIServerPort: d.NodeConfig.APIServerPort, } - networkName := d.NodeConfig.DockerNetwork + networkName := d.NodeConfig.Network if networkName == "" { networkName = d.NodeConfig.ClusterName } diff --git a/pkg/drivers/kic/types.go b/pkg/drivers/kic/types.go index eacd029d247f..946235c111a7 100644 --- a/pkg/drivers/kic/types.go +++ b/pkg/drivers/kic/types.go @@ -61,6 +61,6 @@ type Config struct { Envs map[string]string // key,value of environment variables passed to the node KubernetesVersion string // Kubernetes version to install ContainerRuntime string // container runtime kic is running - DockerNetwork string // docker network to run with kic + Network string // network to run with kic ExtraArgs []string // a list of any extra option to pass to oci binary during creation time, for example --expose 8080... } diff --git a/pkg/minikube/config/types.go b/pkg/minikube/config/types.go index 76085140c086..010d292e912a 100644 --- a/pkg/minikube/config/types.go +++ b/pkg/minikube/config/types.go @@ -72,7 +72,7 @@ type ClusterConfig struct { VerifyComponents map[string]bool // map of components to verify and wait for after start. StartHostTimeout time.Duration ExposedPorts []string // Only used by the docker and podman driver - DockerNetwork string // only used by docker driver + ContainerNetwork string // only used by docker driver } // KubernetesConfig contains the parameters used to configure the VM Kubernetes. diff --git a/pkg/minikube/registry/drvs/docker/docker.go b/pkg/minikube/registry/drvs/docker/docker.go index c8616c97259e..fdc50bb37355 100644 --- a/pkg/minikube/registry/drvs/docker/docker.go +++ b/pkg/minikube/registry/drvs/docker/docker.go @@ -79,7 +79,7 @@ func configure(cc config.ClusterConfig, n config.Node) (interface{}, error) { KubernetesVersion: cc.KubernetesConfig.KubernetesVersion, ContainerRuntime: cc.KubernetesConfig.ContainerRuntime, ExtraArgs: extraArgs, - DockerNetwork: cc.DockerNetwork, + Network: cc.ContainerNetwork, }), nil } From 267769c64018ded667a9c42ee6bd1482de838f4e Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 26 Oct 2020 15:14:58 -0700 Subject: [PATCH 07/17] update docs --- site/content/en/docs/commands/start.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/content/en/docs/commands/start.md b/site/content/en/docs/commands/start.md index f0a1172b1ef6..efbe555cf119 100644 --- a/site/content/en/docs/commands/start.md +++ b/site/content/en/docs/commands/start.md @@ -38,7 +38,6 @@ minikube start [flags] --dns-domain string The cluster dns domain name used in the Kubernetes cluster (default "cluster.local") --dns-proxy Enable proxy for NAT DNS requests (virtualbox driver only) --docker-env stringArray Environment variables to pass to the Docker daemon. (format: key=value) - --docker-network string Docker network to run minikube with. Only available with the docker driver. If left empty, minikube will create a new network. --docker-opt stringArray Specify arbitrary flags to pass to the Docker daemon. (format: key=value) --download-only If true, only download and cache files for later use - don't install or start anything. --driver string Used to specify the driver to run Kubernetes in. The list of available drivers depends on operating system. @@ -78,6 +77,7 @@ minikube start [flags] --mount-string string The argument to pass the minikube mount command on start. --nat-nic-type string NIC Type used for nat network. One of Am79C970A, Am79C973, 82540EM, 82543GC, 82545EM, or virtio (virtualbox driver only) (default "virtio") --native-ssh Use native Golang SSH client (default true). Set to 'false' to use the command line 'ssh' command when accessing the docker machine. Useful for the machine drivers when they will not start with 'Waiting for SSH'. (default true) + --network string Docker network to run minikube with. Only available with the docker driver. If left empty, minikube will create a new network. --network-plugin string Kubelet network plug-in to use (default: auto) --nfs-share strings Local folders to share with Guest via NFS mounts (hyperkit driver only) --nfs-shares-root string Where to root the NFS Shares, defaults to /nfsshares (hyperkit driver only) (default "/nfsshares") From cca624698eeb925f1f026234e8a8e3feb1f9c372 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Fri, 6 Nov 2020 12:17:48 -0800 Subject: [PATCH 08/17] switch exit to warning --- cmd/minikube/cmd/start_flags.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/minikube/cmd/start_flags.go b/cmd/minikube/cmd/start_flags.go index fbcce53c17dc..c7b6996a409e 100644 --- a/cmd/minikube/cmd/start_flags.go +++ b/cmd/minikube/cmd/start_flags.go @@ -292,7 +292,7 @@ func generateClusterConfig(cmd *cobra.Command, existing *config.ClusterConfig, k } if !driver.IsKIC(drvName) && viper.GetString(containerNetwork) != "" { - exit.Advice(reason.Usage, "--network flag is only valid with the docker driver", "remove the --network flag from `minikube start`") + out.WarningT("--network flag is only valid with the docker driver, it will be ignored") } cc = config.ClusterConfig{ From ad85a7a57e2c0c7a0a71622c6c37747b83c85a6b Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Thu, 12 Nov 2020 10:36:41 -0800 Subject: [PATCH 09/17] rename containernetwork to network --- cmd/minikube/cmd/start_flags.go | 8 ++++---- pkg/minikube/config/types.go | 2 +- pkg/minikube/registry/drvs/docker/docker.go | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/minikube/cmd/start_flags.go b/cmd/minikube/cmd/start_flags.go index 79f489934165..ce9dd67afaa2 100644 --- a/cmd/minikube/cmd/start_flags.go +++ b/cmd/minikube/cmd/start_flags.go @@ -107,7 +107,7 @@ const ( forceSystemd = "force-systemd" kicBaseImage = "base-image" ports = "ports" - containerNetwork = "network" + network = "network" startNamespace = "namespace" ) @@ -152,7 +152,7 @@ func initMinikubeFlags() { startCmd.Flags().Bool(preload, true, "If set, download tarball of preloaded images if available to improve start time. Defaults to true.") startCmd.Flags().Bool(deleteOnFailure, false, "If set, delete the current cluster if start fails and try again. Defaults to false.") startCmd.Flags().Bool(forceSystemd, false, "If set, force the container runtime to use sytemd as cgroup manager. Currently available for docker and crio. Defaults to false.") - startCmd.Flags().StringP(containerNetwork, "", "", "Docker network to run minikube with. Only available with the docker driver. If left empty, minikube will create a new network.") + startCmd.Flags().StringP(network, "", "", "Docker network to run minikube with. Only available with the docker driver. If left empty, minikube will create a new network.") startCmd.Flags().StringVarP(&outputFormat, "output", "o", "text", "Format to print stdout in. Options include: [text,json]") } @@ -293,7 +293,7 @@ func generateClusterConfig(cmd *cobra.Command, existing *config.ClusterConfig, k out.WarningT("With --network-plugin=cni, you will need to provide your own CNI. See --cni flag as a user-friendly alternative") } - if !driver.IsKIC(drvName) && viper.GetString(containerNetwork) != "" { + if !driver.IsKIC(drvName) && viper.GetString(network) != "" { out.WarningT("--network flag is only valid with the docker driver, it will be ignored") } @@ -303,7 +303,7 @@ func generateClusterConfig(cmd *cobra.Command, existing *config.ClusterConfig, k EmbedCerts: viper.GetBool(embedCerts), MinikubeISO: viper.GetString(isoURL), KicBaseImage: viper.GetString(kicBaseImage), - ContainerNetwork: viper.GetString(containerNetwork), + Network: viper.GetString(network), Memory: mem, CPUs: viper.GetInt(cpus), DiskSize: diskSize, diff --git a/pkg/minikube/config/types.go b/pkg/minikube/config/types.go index bb9f580b3d69..cdd0cf82746b 100644 --- a/pkg/minikube/config/types.go +++ b/pkg/minikube/config/types.go @@ -72,7 +72,7 @@ type ClusterConfig struct { VerifyComponents map[string]bool // map of components to verify and wait for after start. StartHostTimeout time.Duration ExposedPorts []string // Only used by the docker and podman driver - ContainerNetwork string // only used by docker driver + Network string // only used by docker driver } // KubernetesConfig contains the parameters used to configure the VM Kubernetes. diff --git a/pkg/minikube/registry/drvs/docker/docker.go b/pkg/minikube/registry/drvs/docker/docker.go index fdc50bb37355..4a16ded7469d 100644 --- a/pkg/minikube/registry/drvs/docker/docker.go +++ b/pkg/minikube/registry/drvs/docker/docker.go @@ -79,7 +79,7 @@ func configure(cc config.ClusterConfig, n config.Node) (interface{}, error) { KubernetesVersion: cc.KubernetesConfig.KubernetesVersion, ContainerRuntime: cc.KubernetesConfig.ContainerRuntime, ExtraArgs: extraArgs, - Network: cc.ContainerNetwork, + Network: cc.Network, }), nil } From 14db0debef54361a09d436782c5e0a46f9f658db Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 17 Nov 2020 12:15:42 -0800 Subject: [PATCH 10/17] review comments --- pkg/drivers/kic/oci/network_create.go | 2 +- .../{docker_network_test.go => kic_custom_network_test.go} | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) rename test/integration/{docker_network_test.go => kic_custom_network_test.go} (94%) diff --git a/pkg/drivers/kic/oci/network_create.go b/pkg/drivers/kic/oci/network_create.go index 745fcc2ceadc..17c462da6661 100644 --- a/pkg/drivers/kic/oci/network_create.go +++ b/pkg/drivers/kic/oci/network_create.go @@ -51,7 +51,7 @@ func CreateNetwork(ociBin string, name string) (net.IP, error) { func createDockerNetwork(networkName string) (net.IP, error) { // check if the network is docker native, if this is the case return - if networkName == "bridge" { + if networkName == dockerDefaultBridge { out.WarningT("Using the bridge network may result in minikube IP changing on restart") return nil, nil } diff --git a/test/integration/docker_network_test.go b/test/integration/kic_custom_network_test.go similarity index 94% rename from test/integration/docker_network_test.go rename to test/integration/kic_custom_network_test.go index 7d700f1e8940..76ea4ddd4dcd 100644 --- a/test/integration/docker_network_test.go +++ b/test/integration/kic_custom_network_test.go @@ -24,7 +24,7 @@ import ( "testing" ) -func TestDockerNetworkFlag(t *testing.T) { +func TestKicCustomNetwork(t *testing.T) { if !KicDriver() { t.Skip("only runs with docker driver") } @@ -35,7 +35,6 @@ func TestDockerNetworkFlag(t *testing.T) { }{ { description: "create custom network", - flag: "--docker-network=\"\"", }, { description: "use default bridge network", flag: "--docker-network=bridge", From a6a6671175052f6345380250a8103d352a796949 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Wed, 2 Dec 2020 13:19:04 -0800 Subject: [PATCH 11/17] make integration tests more robust --- test/integration/kic_custom_network_test.go | 51 +++++++++++++++++++-- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/test/integration/kic_custom_network_test.go b/test/integration/kic_custom_network_test.go index 76ea4ddd4dcd..2f3c8f5bab3a 100644 --- a/test/integration/kic_custom_network_test.go +++ b/test/integration/kic_custom_network_test.go @@ -20,8 +20,12 @@ package integration import ( "context" + "fmt" "os/exec" + "strings" "testing" + + "k8s.io/minikube/pkg/drivers/kic/oci" ) func TestKicCustomNetwork(t *testing.T) { @@ -31,13 +35,13 @@ func TestKicCustomNetwork(t *testing.T) { tests := []struct { description string - flag string + networkName string }{ { description: "create custom network", }, { description: "use default bridge network", - flag: "--docker-network=bridge", + networkName: "bridge", }, } @@ -47,12 +51,53 @@ func TestKicCustomNetwork(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), Minutes(5)) defer Cleanup(t, profile, cancel) - startArgs := []string{"start", "-p", profile, test.flag} + startArgs := []string{"start", "-p", profile, fmt.Sprintf("--network=%s", test.networkName)} c := exec.CommandContext(ctx, Target(), startArgs...) rr, err := Run(t, c) if err != nil { t.Fatalf("%v failed: %v\n%v", rr.Command(), err, rr.Output()) } + nn := test.networkName + if nn == "" { + nn = profile + } + verifyNetworkExists(ctx, t, nn) }) } } + +func TestKicExistingNetwork(t *testing.T) { + // create custom network + networkName := "existing-network" + if _, err := oci.CreateNetwork(oci.Docker, networkName); err != nil { + t.Fatalf("error creating network: %v", err) + } + defer func() { + if err := oci.DeleteKICNetworks(); err != nil { + t.Logf("error deleting kic network, may need to delete manually: %v", err) + } + }() + profile := UniqueProfileName("existing-network") + ctx, cancel := context.WithTimeout(context.Background(), Minutes(5)) + defer Cleanup(t, profile, cancel) + + verifyNetworkExists(ctx, t, networkName) + + startArgs := []string{"start", "-p", profile, fmt.Sprintf("--network=%s", networkName)} + c := exec.CommandContext(ctx, Target(), startArgs...) + rr, err := Run(t, c) + if err != nil { + t.Fatalf("%v failed: %v\n%v", rr.Command(), err, rr.Output()) + } +} + +func verifyNetworkExists(ctx context.Context, t *testing.T, networkName string) { + c := exec.CommandContext(ctx, "docker", "network", "ls", "--format", "{{.Name}}") + rr, err := Run(t, c) + if err != nil { + t.Fatalf("%v failed: %v\n%v", rr.Command(), err, rr.Output()) + } + if output := rr.Output(); !strings.Contains(output, networkName) { + t.Fatalf("%s network is not listed by [%v]: %v", networkName, c.Args, output) + } +} From 37bcf6d521b09163616e3ac32f39b1c704b14f97 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Thu, 3 Dec 2020 11:13:19 -0800 Subject: [PATCH 12/17] fix lint --- go.mod | 2 ++ test/integration/kic_custom_network_test.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index a3d449a00a09..6df5ebf832ba 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.15 require ( cloud.google.com/go/storage v1.10.0 + contrib.go.opencensus.io/exporter/stackdriver v0.12.1 github.com/Azure/azure-sdk-for-go v42.3.0+incompatible github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/trace v0.13.0 github.com/Microsoft/go-winio v0.4.15-0.20190919025122-fc70bd9a86b5 // indirect @@ -75,6 +76,7 @@ require ( github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect github.com/xeipuuv/gojsonschema v0.0.0-20180618132009-1d523034197f github.com/zchee/go-vmnet v0.0.0-20161021174912-97ebf9174097 + go.opencensus.io v0.22.4 go.opentelemetry.io/otel v0.13.0 go.opentelemetry.io/otel/sdk v0.13.0 golang.org/x/build v0.0.0-20190927031335-2835ba2e683f diff --git a/test/integration/kic_custom_network_test.go b/test/integration/kic_custom_network_test.go index 2f3c8f5bab3a..13c07ec7d57d 100644 --- a/test/integration/kic_custom_network_test.go +++ b/test/integration/kic_custom_network_test.go @@ -73,7 +73,7 @@ func TestKicExistingNetwork(t *testing.T) { t.Fatalf("error creating network: %v", err) } defer func() { - if err := oci.DeleteKICNetworks(); err != nil { + if err := oci.DeleteKICNetworks(oci.Docker); err != nil { t.Logf("error deleting kic network, may need to delete manually: %v", err) } }() From 787cf77d6c13389d9aaa256793a1a8127b545bf7 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Wed, 16 Dec 2020 11:11:39 -0800 Subject: [PATCH 13/17] Only run existing network test with kic driver --- test/integration/kic_custom_network_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/integration/kic_custom_network_test.go b/test/integration/kic_custom_network_test.go index 13c07ec7d57d..d3b43c4d3edb 100644 --- a/test/integration/kic_custom_network_test.go +++ b/test/integration/kic_custom_network_test.go @@ -67,6 +67,9 @@ func TestKicCustomNetwork(t *testing.T) { } func TestKicExistingNetwork(t *testing.T) { + if !KicDriver() { + t.Skip("only runs with docker driver") + } // create custom network networkName := "existing-network" if _, err := oci.CreateNetwork(oci.Docker, networkName); err != nil { From 81f00fc3e3b5e70d5e23ba8f5cafbecd93c671b9 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Mon, 21 Dec 2020 16:58:49 -0800 Subject: [PATCH 14/17] update description --- cmd/minikube/cmd/start_flags.go | 2 +- site/content/en/docs/commands/start.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/minikube/cmd/start_flags.go b/cmd/minikube/cmd/start_flags.go index 2238d0508cee..9a656904772d 100644 --- a/cmd/minikube/cmd/start_flags.go +++ b/cmd/minikube/cmd/start_flags.go @@ -153,7 +153,7 @@ func initMinikubeFlags() { startCmd.Flags().Bool(preload, true, "If set, download tarball of preloaded images if available to improve start time. Defaults to true.") startCmd.Flags().Bool(deleteOnFailure, false, "If set, delete the current cluster if start fails and try again. Defaults to false.") startCmd.Flags().Bool(forceSystemd, false, "If set, force the container runtime to use sytemd as cgroup manager. Currently available for docker and crio. Defaults to false.") - startCmd.Flags().StringP(network, "", "", "Docker network to run minikube with. Only available with the docker driver. If left empty, minikube will create a new network.") + startCmd.Flags().StringP(network, "", "", "network to run minikube with. Only available with the docker/podman drivers. If left empty, minikube will create a new network.") startCmd.Flags().StringVarP(&outputFormat, "output", "o", "text", "Format to print stdout in. Options include: [text,json]") startCmd.Flags().StringP(trace, "", "", "Send trace events. Options include: [gcp]") } diff --git a/site/content/en/docs/commands/start.md b/site/content/en/docs/commands/start.md index fdc410436bab..edaea7cb3a19 100644 --- a/site/content/en/docs/commands/start.md +++ b/site/content/en/docs/commands/start.md @@ -77,7 +77,7 @@ minikube start [flags] --namespace string The named space to activate after start (default "default") --nat-nic-type string NIC Type used for nat network. One of Am79C970A, Am79C973, 82540EM, 82543GC, 82545EM, or virtio (virtualbox driver only) (default "virtio") --native-ssh Use native Golang SSH client (default true). Set to 'false' to use the command line 'ssh' command when accessing the docker machine. Useful for the machine drivers when they will not start with 'Waiting for SSH'. (default true) - --network string Docker network to run minikube with. Only available with the docker driver. If left empty, minikube will create a new network. + --network string network to run minikube with. Only available with the docker/podman drivers. If left empty, minikube will create a new network. --network-plugin string Kubelet network plug-in to use (default: auto) --nfs-share strings Local folders to share with Guest via NFS mounts (hyperkit driver only) --nfs-shares-root string Where to root the NFS Shares, defaults to /nfsshares (hyperkit driver only) (default "/nfsshares") From bd413794c2a46eaf3552dd9764fd82438a09de6d Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 22 Dec 2020 14:56:27 -0800 Subject: [PATCH 15/17] Avoid creating network if it is the default --- pkg/drivers/kic/oci/network_create.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/drivers/kic/oci/network_create.go b/pkg/drivers/kic/oci/network_create.go index 5a49fc77e646..49291d3a1504 100644 --- a/pkg/drivers/kic/oci/network_create.go +++ b/pkg/drivers/kic/oci/network_create.go @@ -44,7 +44,7 @@ const dockerDefaultBridge = "bridge" const podmanDefaultBridge = "podman" // CreateNetwork creates a network returns gateway and error, minikube creates one network per cluster -func CreateNetwork(ociBin string, clusterName string) (net.IP, error) { +func CreateNetwork(ociBin string, networkName string) (net.IP, error) { var defaultBridgeName string if ociBin == Docker { defaultBridgeName = dockerDefaultBridge @@ -52,9 +52,12 @@ func CreateNetwork(ociBin string, clusterName string) (net.IP, error) { if ociBin == Podman { defaultBridgeName = podmanDefaultBridge } + if networkName == defaultBridgeName { + return nil, nil + } // check if the network already exists - info, err := containerNetworkInspect(ociBin, clusterName) + info, err := containerNetworkInspect(ociBin, networkName) if err == nil { klog.Infof("Found existing network %+v", info) return info.gateway, nil @@ -71,7 +74,7 @@ func CreateNetwork(ociBin string, clusterName string) (net.IP, error) { // Rather than iterate through all of the valid subnets, give up at 20 to avoid a lengthy user delay for something that is unlikely to work. // will be like 192.168.49.0/24 ,...,192.168.239.0/24 for attempts < 20 { - info.gateway, err = tryCreateDockerNetwork(ociBin, subnetAddr, defaultSubnetMask, info.mtu, clusterName) + info.gateway, err = tryCreateDockerNetwork(ociBin, subnetAddr, defaultSubnetMask, info.mtu, networkName) if err == nil { return info.gateway, nil } From a3b80e1fffaf17bd96bf2ec7dc5bbd0284e8a74e Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Tue, 22 Dec 2020 14:57:04 -0800 Subject: [PATCH 16/17] add log --- pkg/drivers/kic/oci/network_create.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/drivers/kic/oci/network_create.go b/pkg/drivers/kic/oci/network_create.go index 49291d3a1504..3f145e2e84e1 100644 --- a/pkg/drivers/kic/oci/network_create.go +++ b/pkg/drivers/kic/oci/network_create.go @@ -53,6 +53,7 @@ func CreateNetwork(ociBin string, networkName string) (net.IP, error) { defaultBridgeName = podmanDefaultBridge } if networkName == defaultBridgeName { + klog.Infof("skipping creating network since default network %s was specified", networkName) return nil, nil } From e210a72bfacce350de3cdf88c1724cac6d1354b1 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Wed, 23 Dec 2020 14:03:10 -0800 Subject: [PATCH 17/17] update warning --- cmd/minikube/cmd/start_flags.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/minikube/cmd/start_flags.go b/cmd/minikube/cmd/start_flags.go index 35897b5dff8c..1988677eeb42 100644 --- a/cmd/minikube/cmd/start_flags.go +++ b/cmd/minikube/cmd/start_flags.go @@ -296,7 +296,7 @@ func generateClusterConfig(cmd *cobra.Command, existing *config.ClusterConfig, k } if !driver.IsKIC(drvName) && viper.GetString(network) != "" { - out.WarningT("--network flag is only valid with the docker driver, it will be ignored") + out.WarningT("--network flag is only valid with the docker/podman drivers, it will be ignored") } cc = config.ClusterConfig{