Move hostIP detection from master to server

Add PublicAddress in test files Move valid public addr into util
nikhiljindal · Nov 30, 2015 · 477da92 · 477da92
1 parent fc927e8
commit 477da92
Show file tree

Hide file tree

Showing 8 changed files with 47 additions and 18 deletions.
diff --git a/cmd/integration/integration.go b/cmd/integration/integration.go
@@ -170,6 +170,13 @@ func startComponents(firstManifestURL, secondManifestURL string) (string, string
 		glog.Fatalf("No public address for %s", host)
 	}
 
+	// The caller of master.New should guarantee pulicAddress is properly set
+	hostIP, err := util.ValidPublicAddrForMaster(publicAddress)
+	if err != nil {
+		glog.Fatalf("Unable to find suitable network address.error='%v' . "+
+			"Fail to get a valid public address for master.", err)
+	}
+
 	// Create a master and install handlers into mux.
 	m := master.New(&master.Config{
 		StorageDestinations:   storageDestinations,
@@ -182,7 +189,7 @@ func startComponents(firstManifestURL, secondManifestURL string) (string, string
 		Authorizer:            apiserver.NewAlwaysAllowAuthorizer(),
 		AdmissionControl:      admit.NewAlwaysAdmit(),
 		ReadWritePort:         portNumber,
-		PublicAddress:         publicAddress,
+		PublicAddress:         hostIP,
 		CacheTimeout:          2 * time.Second,
 		StorageVersions:       storageVersions,
 	})

diff --git a/cmd/kube-apiserver/app/server.go b/cmd/kube-apiserver/app/server.go
@@ -367,12 +367,17 @@ func (s *APIServer) Run(_ []string) error {
 	s.verifyClusterIPFlags()
 
 	// If advertise-address is not specified, use bind-address. If bind-address
-	// is not usable (unset, 0.0.0.0, or loopback), setDefaults() in
-	// pkg/master/master.go will do the right thing and use the host's default
-	// interface.
+	// is not usable (unset, 0.0.0.0, or loopback), we will use the host's default
+	// interface as valid public addr for master (see: util#ValidPublicAddrForMaster)
 	if s.AdvertiseAddress == nil || s.AdvertiseAddress.IsUnspecified() {
-		s.AdvertiseAddress = s.BindAddress
+		hostIP, err := util.ValidPublicAddrForMaster(s.BindAddress)
+		if err != nil {
+			glog.Fatalf("Unable to find suitable network address.error='%v' . "+
+				"Try to set the AdvertiseAddress directly or provide a valid BindAddress to fix this.", err)
+		}
+		s.AdvertiseAddress = hostIP
 	}
+	glog.Infof("Will report %v as public IP address.", s.AdvertiseAddress)
 
 	if (s.EtcdConfigFile != "" && len(s.EtcdServerList) != 0) || (s.EtcdConfigFile == "" && len(s.EtcdServerList) == 0) {
 		glog.Fatalf("Specify either --etcd-servers or --etcd-config")

diff --git a/pkg/master/master.go b/pkg/master/master.go
@@ -401,19 +401,6 @@ func setDefaults(c *Config) {
 	if c.CacheTimeout == 0 {
 		c.CacheTimeout = 5 * time.Second
 	}
-	for c.PublicAddress == nil || c.PublicAddress.IsUnspecified() || c.PublicAddress.IsLoopback() {
-		// TODO: This should be done in the caller and just require a
-		// valid value to be passed in.
-		hostIP, err := util.ChooseHostInterface()
-		if err != nil {
-			glog.Fatalf("Unable to find suitable network address.error='%v' . "+
-				"Will try again in 5 seconds. Set the public address directly to avoid this wait.", err)
-			time.Sleep(5 * time.Second)
-			continue
-		}
-		c.PublicAddress = hostIP
-		glog.Infof("Will report %v as public IP address.", c.PublicAddress)
-	}
 	if c.RequestContextMapper == nil {
 		c.RequestContextMapper = api.NewRequestContextMapper()
 	}

diff --git a/pkg/master/master_test.go b/pkg/master/master_test.go
@@ -74,6 +74,7 @@ func setUp(t *testing.T) (Master, *etcdtesting.EtcdTestServer, Config, *assert.A
 	storageVersions[""] = testapi.Default.Version()
 	storageVersions["extensions"] = testapi.Extensions.GroupAndVersion()
 	config.StorageVersions = storageVersions
+	config.PublicAddress = net.ParseIP("192.168.10.4")
 	master.nodeRegistry = registrytest.NewNodeRegistry([]string{"node1", "node2"}, api.NodeResources{})
 
 	return master, server, config, assert.New(t)

diff --git a/pkg/util/util.go b/pkg/util/util.go
@@ -512,3 +512,17 @@ func ReadDirNoExit(dirname string) ([]os.FileInfo, []error, error) {
 
 	return list, errs, nil
 }
+
+// If bind-address is usable, return it directly
+// If bind-address is not usable (unset, 0.0.0.0, or loopback), we will use the host's default
+// interface.
+func ValidPublicAddrForMaster(bindAddress net.IP) (net.IP, error) {
+	if bindAddress == nil || bindAddress.IsUnspecified() || bindAddress.IsLoopback() {
+		hostIP, err := ChooseHostInterface()
+		if err != nil {
+			return nil, err
+		}
+		bindAddress = hostIP
+	}
+	return bindAddress, nil
+}
diff --git a/test/integration/auth_test.go b/test/integration/auth_test.go
@@ -28,6 +28,7 @@ import (
 	"errors"
 	"fmt"
 	"io/ioutil"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -420,6 +421,7 @@ func TestAuthModeAlwaysAllow(t *testing.T) {
 		Authorizer:            apiserver.NewAlwaysAllowAuthorizer(),
 		AdmissionControl:      admit.NewAlwaysAdmit(),
 		StorageVersions:       storageVersions,
+		PublicAddress:         net.ParseIP("192.168.10.4"),
 	})
 
 	transport := http.DefaultTransport
@@ -549,6 +551,7 @@ func TestAuthModeAlwaysDeny(t *testing.T) {
 		Authorizer:            apiserver.NewAlwaysDenyAuthorizer(),
 		AdmissionControl:      admit.NewAlwaysAdmit(),
 		StorageVersions:       storageVersions,
+		PublicAddress:         net.ParseIP("192.168.10.4"),
 	})
 
 	transport := http.DefaultTransport
@@ -630,6 +633,7 @@ func TestAliceNotForbiddenOrUnauthorized(t *testing.T) {
 		Authorizer:            allowAliceAuthorizer{},
 		AdmissionControl:      admit.NewAlwaysAdmit(),
 		StorageVersions:       storageVersions,
+		PublicAddress:         net.ParseIP("192.168.10.4"),
 	})
 
 	previousResourceVersion := make(map[string]float64)
@@ -730,6 +734,7 @@ func TestBobIsForbidden(t *testing.T) {
 		Authorizer:            allowAliceAuthorizer{},
 		AdmissionControl:      admit.NewAlwaysAdmit(),
 		StorageVersions:       storageVersions,
+		PublicAddress:         net.ParseIP("192.168.10.4"),
 	})
 
 	transport := http.DefaultTransport
@@ -804,6 +809,7 @@ func TestUnknownUserIsUnauthorized(t *testing.T) {
 		Authorizer:            allowAliceAuthorizer{},
 		AdmissionControl:      admit.NewAlwaysAdmit(),
 		StorageVersions:       storageVersions,
+		PublicAddress:         net.ParseIP("192.168.10.4"),
 	})
 
 	transport := http.DefaultTransport
@@ -903,6 +909,7 @@ func TestAuthorizationAttributeDetermination(t *testing.T) {
 		Authorizer:            trackingAuthorizer,
 		AdmissionControl:      admit.NewAlwaysAdmit(),
 		StorageVersions:       storageVersions,
+		PublicAddress:         net.ParseIP("192.168.10.4"),
 	})
 
 	transport := http.DefaultTransport
@@ -997,6 +1004,7 @@ func TestNamespaceAuthorization(t *testing.T) {
 		Authorizer:            a,
 		AdmissionControl:      admit.NewAlwaysAdmit(),
 		StorageVersions:       storageVersions,
+		PublicAddress:         net.ParseIP("192.168.10.4"),
 	})
 
 	previousResourceVersion := make(map[string]float64)
@@ -1125,6 +1133,7 @@ func TestKindAuthorization(t *testing.T) {
 		Authorizer:            a,
 		AdmissionControl:      admit.NewAlwaysAdmit(),
 		StorageVersions:       storageVersions,
+		PublicAddress:         net.ParseIP("192.168.10.4"),
 	})
 
 	previousResourceVersion := make(map[string]float64)
@@ -1240,6 +1249,7 @@ func TestReadOnlyAuthorization(t *testing.T) {
 		Authorizer:            a,
 		AdmissionControl:      admit.NewAlwaysAdmit(),
 		StorageVersions:       storageVersions,
+		PublicAddress:         net.ParseIP("192.168.10.4"),
 	})
 
 	transport := http.DefaultTransport

diff --git a/test/integration/scheduler_test.go b/test/integration/scheduler_test.go
@@ -22,6 +22,7 @@ package integration
 
 import (
 	"fmt"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"sync"
@@ -91,6 +92,7 @@ func TestUnschedulableNodes(t *testing.T) {
 		Authorizer:            apiserver.NewAlwaysAllowAuthorizer(),
 		AdmissionControl:      admit.NewAlwaysAdmit(),
 		StorageVersions:       storageVersions,
+		PublicAddress:         net.ParseIP("192.168.10.4"),
 	})
 
 	restClient := client.NewOrDie(&client.Config{Host: s.URL, GroupVersion: testapi.Default.GroupVersion()})
@@ -341,6 +343,7 @@ func BenchmarkScheduling(b *testing.B) {
 		Authorizer:            apiserver.NewAlwaysAllowAuthorizer(),
 		AdmissionControl:      admit.NewAlwaysAdmit(),
 		StorageVersions:       storageVersions,
+		PublicAddress:         net.ParseIP("192.168.10.4"),
 	})
 
 	c := client.NewOrDie(&client.Config{

diff --git a/test/integration/secret_test.go b/test/integration/secret_test.go
@@ -21,6 +21,7 @@ package integration
 // This file tests use of the secrets API resource.
 
 import (
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"testing"
@@ -81,6 +82,7 @@ func TestSecrets(t *testing.T) {
 		Authorizer:            apiserver.NewAlwaysAllowAuthorizer(),
 		AdmissionControl:      admit.NewAlwaysAdmit(),
 		StorageVersions:       storageVersions,
+		PublicAddress:         net.ParseIP("192.168.10.4"),
 	})
 
 	framework.DeleteAllEtcdKeys()