Fix kubeadm etcd manifests to use brackets around IPv6 addrs

When 'kubeadm init ...' is used with an IPv6 kubeadm configuration,
kubeadm currently generates an etcd.yaml manifest that uses IP:port
combinatins where the IP is an IPv6 address, but it is not enclosed
in square brackets, e.g.:
    - --advertise-client-urls=https://fd00:20::2:2379
For IPv6 advertise addresses, this should be of the form:
    - --advertise-client-urls=https://[fd00:20::2]:2379

The lack of brackets around IPv6 addresses in cases like this is
causing failures to bring up IPv6-only clusters with Kubeadm as
described in kubernetes/kubeadm Issues #1212.

This format error is fixed by using net.JoinHostPort() to generate
URLs as shown above.

Fixes kubernetes/kubeadm Issue #1212

cmd/kubeadm/app/phases/etcd/local.go

@@ -90,7 +90,7 @@ func CreateStackedEtcdStaticPodManifestFile(client clientset.Interface, manifest
 	}
 
 	// notifies the other members of the etcd cluster about the joining member
-	etcdPeerAddress := fmt.Sprintf("https://%s:%d", cfg.LocalAPIEndpoint.AdvertiseAddress, kubeadmconstants.EtcdListenPeerPort)
+	etcdPeerAddress := etcdutil.GetPeerURL(cfg)
 
 	klog.V(1).Infof("Adding etcd member: %s", etcdPeerAddress)
 	initialCluster, err := etcdClient.AddMember(cfg.NodeRegistration.Name, etcdPeerAddress)
@@ -141,10 +141,10 @@ func GetEtcdPodSpec(cfg *kubeadmapi.InitConfiguration, initialCluster []etcdutil
 func getEtcdCommand(cfg *kubeadmapi.InitConfiguration, initialCluster []etcdutil.Member) []string {
 	defaultArguments := map[string]string{
 		"name":                        cfg.GetNodeName(),
-		"listen-client-urls":          fmt.Sprintf("https://127.0.0.1:%d,https://%s:%d", kubeadmconstants.EtcdListenClientPort, cfg.LocalAPIEndpoint.AdvertiseAddress, kubeadmconstants.EtcdListenClientPort),
-		"advertise-client-urls":       fmt.Sprintf("https://%s:%d", cfg.LocalAPIEndpoint.AdvertiseAddress, kubeadmconstants.EtcdListenClientPort),
-		"listen-peer-urls":            fmt.Sprintf("https://%s:%d", cfg.LocalAPIEndpoint.AdvertiseAddress, kubeadmconstants.EtcdListenPeerPort),
-		"initial-advertise-peer-urls": fmt.Sprintf("https://%s:%d", cfg.LocalAPIEndpoint.AdvertiseAddress, kubeadmconstants.EtcdListenPeerPort),
+		"listen-client-urls":          fmt.Sprintf("%s,%s", etcdutil.GetClientURLByIP("127.0.0.1"), etcdutil.GetClientURL(cfg)),
+		"advertise-client-urls":       etcdutil.GetClientURL(cfg),
+		"listen-peer-urls":            etcdutil.GetPeerURL(cfg),
+		"initial-advertise-peer-urls": etcdutil.GetPeerURL(cfg),
 		"data-dir":                    cfg.Etcd.Local.DataDir,
 		"cert-file":                   filepath.Join(cfg.CertificatesDir, kubeadmconstants.EtcdServerCertName),
 		"key-file":                    filepath.Join(cfg.CertificatesDir, kubeadmconstants.EtcdServerKeyName),
@@ -158,7 +158,7 @@ func getEtcdCommand(cfg *kubeadmapi.InitConfiguration, initialCluster []etcdutil
 	}
 
 	if len(initialCluster) == 0 {
-		defaultArguments["initial-cluster"] = fmt.Sprintf("%s=https://%s:%d", cfg.GetNodeName(), cfg.LocalAPIEndpoint.AdvertiseAddress, kubeadmconstants.EtcdListenPeerPort)
+		defaultArguments["initial-cluster"] = fmt.Sprintf("%s=%s", cfg.GetNodeName(), etcdutil.GetPeerURL(cfg))
 	} else {
 		// NB. the joining etcd instance should be part of the initialCluster list
 		endpoints := []string{}

cmd/kubeadm/app/phases/etcd/local_test.go

@@ -115,27 +115,16 @@ func TestCreateLocalEtcdStaticPodManifestFile(t *testing.T) {
 func TestGetEtcdCommand(t *testing.T) {
 	var tests = []struct {
 		name             string
-		cfg            *kubeadmapi.InitConfiguration
+		advertiseAddress string
+		nodeName         string
+		extraArgs        map[string]string
 		initialCluster   []etcdutil.Member
 		expected         []string
 	}{
 		{
 			name:             "Default args - with empty etcd initial cluster",
-			cfg: &kubeadmapi.InitConfiguration{
-				LocalAPIEndpoint: kubeadmapi.APIEndpoint{
-					AdvertiseAddress: "1.2.3.4",
-				},
-				NodeRegistration: kubeadmapi.NodeRegistrationOptions{
-					Name: "foo",
-				},
-				ClusterConfiguration: kubeadmapi.ClusterConfiguration{
-					Etcd: kubeadmapi.Etcd{
-						Local: &kubeadmapi.LocalEtcd{
-							DataDir: "/var/lib/etcd",
-						},
-					},
-				},
-			},
+			advertiseAddress: "1.2.3.4",
+			nodeName:         "foo",
 			expected: []string{
 				"etcd",
 				"--name=foo",
@@ -158,21 +147,8 @@ func TestGetEtcdCommand(t *testing.T) {
 		},
 		{
 			name:             "Default args - With an existing etcd cluster",
-			cfg: &kubeadmapi.InitConfiguration{
-				LocalAPIEndpoint: kubeadmapi.APIEndpoint{
-					AdvertiseAddress: "1.2.3.4",
-				},
-				NodeRegistration: kubeadmapi.NodeRegistrationOptions{
-					Name: "foo",
-				},
-				ClusterConfiguration: kubeadmapi.ClusterConfiguration{
-					Etcd: kubeadmapi.Etcd{
-						Local: &kubeadmapi.LocalEtcd{
-							DataDir: "/var/lib/etcd",
-						},
-					},
-				},
-			},
+			advertiseAddress: "1.2.3.4",
+			nodeName:         "foo",
 			initialCluster: []etcdutil.Member{
 				{Name: "foo", PeerURL: fmt.Sprintf("https://1.2.3.4:%d", kubeadmconstants.EtcdListenPeerPort)}, // NB. the joining etcd instance should be part of the initialCluster list
 				{Name: "bar", PeerURL: fmt.Sprintf("https://5.6.7.8:%d", kubeadmconstants.EtcdListenPeerPort)},
@@ -200,25 +176,12 @@ func TestGetEtcdCommand(t *testing.T) {
 		},
 		{
 			name:             "Extra args",
-			cfg: &kubeadmapi.InitConfiguration{
-				LocalAPIEndpoint: kubeadmapi.APIEndpoint{
-					AdvertiseAddress: "1.2.3.4",
-				},
-				NodeRegistration: kubeadmapi.NodeRegistrationOptions{
-					Name: "bar",
-				},
-				ClusterConfiguration: kubeadmapi.ClusterConfiguration{
-					Etcd: kubeadmapi.Etcd{
-						Local: &kubeadmapi.LocalEtcd{
-							DataDir: "/var/lib/etcd",
-							ExtraArgs: map[string]string{
+			advertiseAddress: "1.2.3.4",
+			nodeName:         "bar",
+			extraArgs: map[string]string{
 				"listen-client-urls":    "https://10.0.1.10:2379",
 				"advertise-client-urls": "https://10.0.1.10:2379",
 			},
-						},
-					},
-				},
-			},
 			expected: []string{
 				"etcd",
 				"--name=bar",
@@ -239,11 +202,51 @@ func TestGetEtcdCommand(t *testing.T) {
 				fmt.Sprintf("--initial-cluster=bar=https://1.2.3.4:%d", kubeadmconstants.EtcdListenPeerPort),
 			},
 		},
+		{
+			name:             "IPv6 advertise address",
+			advertiseAddress: "2001:db8::3",
+			nodeName:         "foo",
+			expected: []string{
+				"etcd",
+				"--name=foo",
+				fmt.Sprintf("--listen-client-urls=https://127.0.0.1:%d,https://[2001:db8::3]:%d", kubeadmconstants.EtcdListenClientPort, kubeadmconstants.EtcdListenClientPort),
+				fmt.Sprintf("--advertise-client-urls=https://[2001:db8::3]:%d", kubeadmconstants.EtcdListenClientPort),
+				fmt.Sprintf("--listen-peer-urls=https://[2001:db8::3]:%d", kubeadmconstants.EtcdListenPeerPort),
+				fmt.Sprintf("--initial-advertise-peer-urls=https://[2001:db8::3]:%d", kubeadmconstants.EtcdListenPeerPort),
+				"--data-dir=/var/lib/etcd",
+				"--cert-file=" + kubeadmconstants.EtcdServerCertName,
+				"--key-file=" + kubeadmconstants.EtcdServerKeyName,
+				"--trusted-ca-file=" + kubeadmconstants.EtcdCACertName,
+				"--client-cert-auth=true",
+				"--peer-cert-file=" + kubeadmconstants.EtcdPeerCertName,
+				"--peer-key-file=" + kubeadmconstants.EtcdPeerKeyName,
+				"--peer-trusted-ca-file=" + kubeadmconstants.EtcdCACertName,
+				"--snapshot-count=10000",
+				"--peer-client-cert-auth=true",
+				fmt.Sprintf("--initial-cluster=foo=https://[2001:db8::3]:%d", kubeadmconstants.EtcdListenPeerPort),
+			},
+		},
 	}
 
 	for _, rt := range tests {
 		t.Run(rt.name, func(t *testing.T) {
-			actual := getEtcdCommand(rt.cfg, rt.initialCluster)
+			cfg := &kubeadmapi.InitConfiguration{
+				LocalAPIEndpoint: kubeadmapi.APIEndpoint{
+					AdvertiseAddress: rt.advertiseAddress,
+				},
+				NodeRegistration: kubeadmapi.NodeRegistrationOptions{
+					Name: rt.nodeName,
+				},
+				ClusterConfiguration: kubeadmapi.ClusterConfiguration{
+					Etcd: kubeadmapi.Etcd{
+						Local: &kubeadmapi.LocalEtcd{
+							DataDir:   "/var/lib/etcd",
+							ExtraArgs: rt.extraArgs,
+						},
+					},
+				},
+			}
+			actual := getEtcdCommand(cfg, rt.initialCluster)
 			sort.Strings(actual)
 			sort.Strings(rt.expected)
 			if !reflect.DeepEqual(actual, rt.expected) {

cmd/kubeadm/app/phases/upgrade/staticpods.go

@@ -282,7 +282,7 @@ func performEtcdStaticPodUpgrade(client clientset.Interface, waiter apiclient.Wa
 	if err != nil {
 		return true, errors.Wrap(err, "failed to retrieve the current etcd version")
 	}
-	currentEtcdVersionStr, ok := currentEtcdVersions[fmt.Sprintf("https://%s:%d", cfg.LocalAPIEndpoint.AdvertiseAddress, constants.EtcdListenClientPort)]
+	currentEtcdVersionStr, ok := currentEtcdVersions[etcdutil.GetClientURL(cfg)]
 	if !ok {
 		fmt.Println(currentEtcdVersions)
 		return true, errors.Wrap(err, "failed to retrieve the current etcd version")

cmd/kubeadm/app/util/etcd/BUILD

@@ -24,6 +24,7 @@ go_test(
     embed = [":go_default_library"],
     deps = [
         "//cmd/kubeadm/app/apis/kubeadm:go_default_library",
+        "//cmd/kubeadm/app/constants:go_default_library",
         "//cmd/kubeadm/test:go_default_library",
     ],
 )

cmd/kubeadm/app/util/etcd/etcd.go

@@ -20,7 +20,9 @@ import (
 	"context"
 	"crypto/tls"
 	"fmt"
+	"net"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"time"
 
@@ -171,7 +173,7 @@ func NewFromCluster(client clientset.Interface, certificatesDir string) (*Client
 	// Get the list of etcd endpoints from cluster status
 	endpoints := []string{}
 	for _, e := range clusterStatus.APIEndpoints {
-		endpoints = append(endpoints, fmt.Sprintf("https://%s:%d", e.AdvertiseAddress, constants.EtcdListenClientPort))
+		endpoints = append(endpoints, GetClientURLByIP(e.AdvertiseAddress))
 	}
 	klog.V(1).Infof("etcd endpoints read from pods: %s", strings.Join(endpoints, ","))
 
@@ -358,3 +360,21 @@ func (c Client) WaitForClusterAvailable(delay time.Duration, retries int, retryI
 func CheckConfigurationIsHA(cfg *kubeadmapi.Etcd) bool {
 	return cfg.External != nil && len(cfg.External.Endpoints) > 1
 }
+
+// GetClientURL creates an HTTPS URL that uses the configured advertise
+// address and client port for the API controller
+func GetClientURL(cfg *kubeadmapi.InitConfiguration) string {
+	return "https://" + net.JoinHostPort(cfg.LocalAPIEndpoint.AdvertiseAddress, strconv.Itoa(constants.EtcdListenClientPort))
+}
+
+// GetPeerURL creates an HTTPS URL that uses the configured advertise
+// address and peer port for the API controller
+func GetPeerURL(cfg *kubeadmapi.InitConfiguration) string {
+	return "https://" + net.JoinHostPort(cfg.LocalAPIEndpoint.AdvertiseAddress, strconv.Itoa(constants.EtcdListenPeerPort))
+}
+
+// GetClientURLByIP creates an HTTPS URL based on an IP address
+// and the client listening port.
+func GetClientURLByIP(ip string) string {
+	return "https://" + net.JoinHostPort(ip, strconv.Itoa(constants.EtcdListenClientPort))
+}

cmd/kubeadm/app/util/etcd/etcd_test.go

@@ -17,12 +17,15 @@ limitations under the License.
 package etcd
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strconv"
 	"testing"
 
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
+	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	testutil "k8s.io/kubernetes/cmd/kubeadm/test"
 )
 
@@ -308,3 +311,90 @@ func TestCheckConfigurationIsHA(t *testing.T) {
 		})
 	}
 }
+
+func testGetURL(t *testing.T, getURLFunc func(*kubeadmapi.InitConfiguration) string, port int) {
+	portStr := strconv.Itoa(port)
+	var tests = []struct {
+		name             string
+		advertiseAddress string
+		expectedURL      string
+	}{
+		{
+			name:             "IPv4",
+			advertiseAddress: "10.10.10.10",
+			expectedURL:      fmt.Sprintf("https://10.10.10.10:%s", portStr),
+		},
+		{
+			name:             "IPv6",
+			advertiseAddress: "2001:db8::2",
+			expectedURL:      fmt.Sprintf("https://[2001:db8::2]:%s", portStr),
+		},
+		{
+			name:             "IPv4 localhost",
+			advertiseAddress: "127.0.0.1",
+			expectedURL:      fmt.Sprintf("https://127.0.0.1:%s", portStr),
+		},
+		{
+			name:             "IPv6 localhost",
+			advertiseAddress: "::1",
+			expectedURL:      fmt.Sprintf("https://[::1]:%s", portStr),
+		},
+	}
+
+	for _, test := range tests {
+		cfg := &kubeadmapi.InitConfiguration{
+			LocalAPIEndpoint: kubeadmapi.APIEndpoint{
+				AdvertiseAddress: test.advertiseAddress,
+			},
+		}
+		url := getURLFunc(cfg)
+		if url != test.expectedURL {
+			t.Errorf("expected %s, got %s", test.expectedURL, url)
+		}
+	}
+}
+
+func TestGetClientURL(t *testing.T) {
+	testGetURL(t, GetClientURL, constants.EtcdListenClientPort)
+}
+
+func TestGetPeerURL(t *testing.T) {
+	testGetURL(t, GetClientURL, constants.EtcdListenClientPort)
+}
+
+func TestGetClientURLByIP(t *testing.T) {
+	portStr := strconv.Itoa(constants.EtcdListenClientPort)
+	var tests = []struct {
+		name        string
+		ip          string
+		expectedURL string
+	}{
+		{
+			name:        "IPv4",
+			ip:          "10.10.10.10",
+			expectedURL: fmt.Sprintf("https://10.10.10.10:%s", portStr),
+		},
+		{
+			name:        "IPv6",
+			ip:          "2001:db8::2",
+			expectedURL: fmt.Sprintf("https://[2001:db8::2]:%s", portStr),
+		},
+		{
+			name:        "IPv4 localhost",
+			ip:          "127.0.0.1",
+			expectedURL: fmt.Sprintf("https://127.0.0.1:%s", portStr),
+		},
+		{
+			name:        "IPv6 localhost",
+			ip:          "::1",
+			expectedURL: fmt.Sprintf("https://[::1]:%s", portStr),
+		},
+	}
+
+	for _, test := range tests {
+		url := GetClientURLByIP(test.ip)
+		if url != test.expectedURL {
+			t.Errorf("expected %s, got %s", test.expectedURL, url)
+		}
+	}
+}