Merge pull request #35923 from deads2k/api-35-discovery-ip-stuff

Automatic merge from submit-queue

Remove non-generic options from genericapiserver.Config

Remove non-generic options from genericapiserver.Config.  Changes the discovery CIDR/IP information to an interface and then demotes several fields.

I haven't pulled from them genericapiserver.Options, but that's a future option we have.  Segregation as as a followup at the very least.

cmd/kube-apiserver/app/server.go

@@ -86,7 +86,11 @@ func Run(s *options.ServerRunOptions) error {
 							ApplyOptions(s.GenericServerRunOptions). // apply the options selected
 							Complete()                               // set default values based on the known values
 
-	if err := genericConfig.MaybeGenerateServingCerts(); err != nil {
+	serviceIPRange, apiServerServiceIP, err := genericapiserver.DefaultServiceIPRange(s.GenericServerRunOptions.ServiceClusterIPRange)
+	if err != nil {
+		glog.Fatalf("Error determining service IP ranges: %v", err)
+	}
+	if err := genericConfig.MaybeGenerateServingCerts(apiServerServiceIP); err != nil {
 		glog.Fatalf("Failed to generate service certificate: %v", err)
 	}
 
@@ -322,6 +326,15 @@ func Run(s *options.ServerRunOptions) error {
 		ProxyTransport:          proxyTransport,
 
 		Tunneler: tunneler,
+
+		ServiceIPRange:       serviceIPRange,
+		APIServerServiceIP:   apiServerServiceIP,
+		APIServerServicePort: 443,
+
+		ServiceNodePortRange:      s.GenericServerRunOptions.ServiceNodePortRange,
+		KubernetesServiceNodePort: s.GenericServerRunOptions.KubernetesServiceNodePort,
+
+		MasterCount: s.GenericServerRunOptions.MasterCount,
 	}
 
 	if s.GenericServerRunOptions.EnableWatchCache {

pkg/genericapiserver/BUILD

@@ -15,6 +15,7 @@ go_library(
     srcs = [
         "config.go",
         "default_storage_factory_builder.go",
+        "discovery.go",
         "doc.go",
         "genericapiserver.go",
         "healthz.go",
@@ -23,6 +24,7 @@ go_library(
         "resource_encoding_config.go",
         "reststorage_interfaces.go",
         "serve.go",
+        "services.go",
         "storage_factory.go",
         "tunneler.go",
     ],
@@ -102,11 +104,11 @@ go_test(
         "//pkg/auth/user:go_default_library",
         "//pkg/generated/openapi:go_default_library",
         "//pkg/genericapiserver/options:go_default_library",
-        "//pkg/registry/core/service/ipallocator:go_default_library",
         "//pkg/storage/etcd/testing:go_default_library",
         "//pkg/storage/storagebackend:go_default_library",
         "//pkg/util/cert:go_default_library",
         "//pkg/util/clock:go_default_library",
+        "//pkg/util/net:go_default_library",
         "//pkg/util/sets:go_default_library",
         "//pkg/version:go_default_library",
         "//vendor:github.com/go-openapi/spec",

pkg/genericapiserver/config.go

@@ -50,7 +50,6 @@ import (
 	"k8s.io/kubernetes/pkg/genericapiserver/options"
 	"k8s.io/kubernetes/pkg/genericapiserver/routes"
 	genericvalidation "k8s.io/kubernetes/pkg/genericapiserver/validation"
-	ipallocator "k8s.io/kubernetes/pkg/registry/core/service/ipallocator"
 	"k8s.io/kubernetes/pkg/runtime"
 	certutil "k8s.io/kubernetes/pkg/util/cert"
 	utilnet "k8s.io/kubernetes/pkg/util/net"
@@ -107,53 +106,25 @@ type Config struct {
 	// Note that it is up to the request handlers to ignore or honor this timeout. In seconds.
 	MinRequestTimeout int
 
-	// Number of masters running; all masters must be started with the
-	// same value for this field. (Numbers > 1 currently untested.)
-	MasterCount int
-
 	SecureServingInfo   *SecureServingInfo
 	InsecureServingInfo *ServingInfo
 
+	// DiscoveryAddresses is used to build the IPs pass to discovery.  If nil, the ExternalAddress is
+	// always reported
+	DiscoveryAddresses DiscoveryAddresses
+
 	// The port on PublicAddress where a read-write server will be installed.
 	// Defaults to 6443 if not set.
 	ReadWritePort int
 
-	// ExternalHost is the host name to use for external (public internet) facing URLs (e.g. Swagger)
+	// ExternalAddress is the host name to use for external (public internet) facing URLs (e.g. Swagger)
-	ExternalHost string
+	ExternalAddress string
 
 	// PublicAddress is the IP address where members of the cluster (kubelet,
 	// kube-proxy, services, etc.) can reach the GenericAPIServer.
 	// If nil or 0.0.0.0, the host's default interface will be used.
 	PublicAddress net.IP
 
-	// The range of IPs to be assigned to services with type=ClusterIP or greater
-	ServiceClusterIPRange *net.IPNet
-
-	// The IP address for the GenericAPIServer service (must be inside ServiceClusterIPRange)
-	ServiceReadWriteIP net.IP
-
-	// Port for the apiserver service.
-	ServiceReadWritePort int
-
-	// The range of ports to be assigned to services with type=NodePort or greater
-	ServiceNodePortRange utilnet.PortRange
-
-	// Additional ports to be exposed on the GenericAPIServer service
-	// extraServicePorts is injectable in the event that more ports
-	// (other than the default 443/tcp) are exposed on the GenericAPIServer
-	// and those ports need to be load balanced by the GenericAPIServer
-	// service because this pkg is linked by out-of-tree projects
-	// like openshift which want to use the GenericAPIServer but also do
-	// more stuff.
-	ExtraServicePorts []api.ServicePort
-	// Additional ports to be exposed on the GenericAPIServer endpoints
-	// Port names should align with ports defined in ExtraServicePorts
-	ExtraEndpointPorts []api.EndpointPort
-
-	// If non-zero, the "kubernetes" services uses this port as NodePort.
-	// TODO(sttts): move into master
-	KubernetesServiceNodePort int
-
 	// EnableOpenAPISupport enables OpenAPI support. Allow downstream customers to disable OpenAPI spec.
 	EnableOpenAPISupport bool
 
@@ -221,9 +192,7 @@ func NewConfig() *Config {
 
 	config := &Config{
 		Serializer:             api.Codecs,
-		MasterCount:            1,
 		ReadWritePort:          6443,
-		ServiceReadWritePort:   443,
 		RequestContextMapper:   api.NewRequestContextMapper(),
 		BuildHandlerChainsFunc: DefaultBuildHandlerChain,
 		LegacyAPIGroupPrefixes: sets.NewString(DefaultLegacyAPIPrefix),
@@ -317,14 +286,10 @@ func (c *Config) ApplyOptions(options *options.ServerRunOptions) *Config {
 	c.EnableGarbageCollection = options.EnableGarbageCollection
 	c.EnableProfiling = options.EnableProfiling
 	c.EnableSwaggerUI = options.EnableSwaggerUI
-	c.ExternalHost = options.ExternalHost
+	c.ExternalAddress = options.ExternalHost
-	c.KubernetesServiceNodePort = options.KubernetesServiceNodePort
-	c.MasterCount = options.MasterCount
 	c.MaxRequestsInFlight = options.MaxRequestsInFlight
 	c.MinRequestTimeout = options.MinRequestTimeout
 	c.PublicAddress = options.AdvertiseAddress
-	c.ServiceClusterIPRange = &options.ServiceClusterIPRange
-	c.ServiceNodePortRange = options.ServiceNodePortRange
 	c.SupportsBasicAuth = len(options.BasicAuthFile) > 0
 
 	return c
@@ -337,41 +302,12 @@ type completedConfig struct {
 // Complete fills in any fields not set that are required to have valid data and can be derived
 // from other fields.  If you're going to `ApplyOptions`, do that first.  It's mutating the receiver.
 func (c *Config) Complete() completedConfig {
-	if c.ServiceClusterIPRange == nil || c.ServiceClusterIPRange.IP == nil {
+	if len(c.ExternalAddress) == 0 && c.PublicAddress != nil {
-		defaultNet := "10.0.0.0/24"
-		glog.Warningf("Network range for service cluster IPs is unspecified. Defaulting to %v.", defaultNet)
-		_, serviceClusterIPRange, err := net.ParseCIDR(defaultNet)
-		if err != nil {
-			glog.Fatalf("Unable to parse CIDR: %v", err)
-		}
-		if size := ipallocator.RangeSize(serviceClusterIPRange); size < 8 {
-			glog.Fatalf("The service cluster IP range must be at least %d IP addresses", 8)
-		}
-		c.ServiceClusterIPRange = serviceClusterIPRange
-	}
-	if c.ServiceReadWriteIP == nil {
-		// Select the first valid IP from ServiceClusterIPRange to use as the GenericAPIServer service IP.
-		serviceReadWriteIP, err := ipallocator.GetIndexedIP(c.ServiceClusterIPRange, 1)
-		if err != nil {
-			glog.Fatalf("Failed to generate service read-write IP for GenericAPIServer service: %v", err)
-		}
-		glog.V(4).Infof("Setting GenericAPIServer service IP to %q (read-write).", serviceReadWriteIP)
-		c.ServiceReadWriteIP = serviceReadWriteIP
-	}
-	if c.ServiceNodePortRange.Size == 0 {
-		// TODO: Currently no way to specify an empty range (do we need to allow this?)
-		// We should probably allow this for clouds that don't require NodePort to do load-balancing (GCE)
-		// but then that breaks the strict nestedness of ServiceType.
-		// Review post-v1
-		c.ServiceNodePortRange = options.DefaultServiceNodePortRange
-		glog.Infof("Node port range unspecified. Defaulting to %v.", c.ServiceNodePortRange)
-	}
-	if len(c.ExternalHost) == 0 && c.PublicAddress != nil {
 		hostAndPort := c.PublicAddress.String()
 		if c.ReadWritePort != 0 {
 			hostAndPort = net.JoinHostPort(hostAndPort, strconv.Itoa(c.ReadWritePort))
 		}
-		c.ExternalHost = hostAndPort
+		c.ExternalAddress = hostAndPort
 	}
 	// All APIs will have the same authentication for now.
 	if c.OpenAPIConfig != nil && c.OpenAPIConfig.SecurityDefinitions != nil {
@@ -395,6 +331,10 @@ func (c *Config) Complete() completedConfig {
 			}
 		}
 	}
+	if c.DiscoveryAddresses == nil {
+		c.DiscoveryAddresses = DefaultDiscoveryAddresses{DefaultAddress: c.ExternalAddress}
+	}
+
 	return completedConfig{c}
 }
 
@@ -431,7 +371,7 @@ func (c completedConfig) New() (*GenericAPIServer, error) {
 	}
 
 	s := &GenericAPIServer{
-		ServiceClusterIPRange:  c.ServiceClusterIPRange,
+		discoveryAddresses:     c.DiscoveryAddresses,
 		LoopbackClientConfig:   c.LoopbackClientConfig,
 		legacyAPIGroupPrefixes: c.LegacyAPIGroupPrefixes,
 		admissionControl:       c.AdmissionControl,
@@ -441,14 +381,10 @@ func (c completedConfig) New() (*GenericAPIServer, error) {
 		minRequestTimeout:    time.Duration(c.MinRequestTimeout) * time.Second,
 		enableSwaggerSupport: c.EnableSwaggerSupport,
 
-		MasterCount:          c.MasterCount,
 		SecureServingInfo:   c.SecureServingInfo,
 		InsecureServingInfo: c.InsecureServingInfo,
-		ExternalAddress:      c.ExternalHost,
+		ExternalAddress:     c.ExternalAddress,
-		ServiceReadWriteIP:   c.ServiceReadWriteIP,
-		ServiceReadWritePort: c.ServiceReadWritePort,
 
-		KubernetesServiceNodePort: c.KubernetesServiceNodePort,
 		apiGroupsForDiscovery: map[string]unversioned.APIGroup{},
 
 		enableOpenAPISupport: c.EnableOpenAPISupport,
@@ -467,12 +403,11 @@ func (c completedConfig) New() (*GenericAPIServer, error) {
 }
 
 // MaybeGenerateServingCerts generates serving certificates if requested and needed.
-func (c completedConfig) MaybeGenerateServingCerts() error {
+func (c completedConfig) MaybeGenerateServingCerts(alternateIPs ...net.IP) error {
 	// It would be nice to set a fqdn subject alt name, but only the kubelets know, the apiserver is clueless
 	// alternateDNS = append(alternateDNS, "kubernetes.default.svc.CLUSTER.DNS.NAME")
 	if c.SecureServingInfo != nil && c.SecureServingInfo.ServerCert.Generate && !certutil.CanReadCertOrKey(c.SecureServingInfo.ServerCert.CertFile, c.SecureServingInfo.ServerCert.KeyFile) {
 		// TODO (cjcullen): Is ClusterIP the right address to sign a cert with?
-		alternateIPs := []net.IP{c.ServiceReadWriteIP}
 		alternateDNS := []string{"kubernetes.default.svc", "kubernetes.default", "kubernetes", "localhost"}
 
 		if cert, key, err := certutil.GenerateSelfSignedCertKey(c.PublicAddress.String(), alternateIPs, alternateDNS); err != nil {
@@ -555,7 +490,7 @@ func DefaultAndValidateRunOptions(options *options.ServerRunOptions) {
 	}
 	glog.Infof("Will report %v as public IP address.", options.AdvertiseAddress)
 
-	// Set default value for ExternalHost if not specified.
+	// Set default value for ExternalAddress if not specified.
 	if len(options.ExternalHost) == 0 {
 		// TODO: extend for other providers
 		if options.CloudProvider == "gce" {

pkg/genericapiserver/discovery.go

@@ -0,0 +1,72 @@
+/*
+Copyright 2016 The Kubernetes Authors.
+
+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 genericapiserver
+
+import (
+	"net"
+
+	"k8s.io/kubernetes/pkg/api/unversioned"
+)
+
+type DiscoveryAddresses interface {
+	ServerAddressByClientCIDRs(net.IP) []unversioned.ServerAddressByClientCIDR
+}
+
+// DefaultDiscoveryAddresses is a default implementation of DiscoveryAddresses that will work in most cases
+type DefaultDiscoveryAddresses struct {
+	// DiscoveryCIDRRules is a list of CIDRs and Addresses to use if a client is in the range
+	DiscoveryCIDRRules []DiscoveryCIDRRule
+
+	// DefaultAddress is the address (hostname or IP and port) that should be used in
+	// if no CIDR matches more specifically.
+	DefaultAddress string
+}
+
+// DiscoveryCIDRRule is a rule for adding an alternate path to the master based on matching CIDR
+type DiscoveryCIDRRule struct {
+	IPRange net.IPNet
+
+	// Address is the address (hostname or IP and port) that should be used in
+	// if this CIDR matches
+	Address string
+}
+
+func (d DefaultDiscoveryAddresses) ServerAddressByClientCIDRs(clientIP net.IP) []unversioned.ServerAddressByClientCIDR {
+	addressCIDRMap := []unversioned.ServerAddressByClientCIDR{
+		{
+			ClientCIDR:    "0.0.0.0/0",
+			ServerAddress: d.DefaultAddress,
+		},
+	}
+
+	for _, rule := range d.DiscoveryCIDRRules {
+		addressCIDRMap = append(addressCIDRMap, rule.ServerAddressByClientCIDRs(clientIP)...)
+	}
+	return addressCIDRMap
+}
+
+func (d DiscoveryCIDRRule) ServerAddressByClientCIDRs(clientIP net.IP) []unversioned.ServerAddressByClientCIDR {
+	addressCIDRMap := []unversioned.ServerAddressByClientCIDR{}
+
+	if d.IPRange.Contains(clientIP) {
+		addressCIDRMap = append(addressCIDRMap, unversioned.ServerAddressByClientCIDR{
+			ClientCIDR:    d.IPRange.String(),
+			ServerAddress: d.Address,
+		})
+	}
+	return addressCIDRMap
+}

pkg/genericapiserver/genericapiserver.go

@@ -19,10 +19,8 @@ package genericapiserver
 import (
 	"fmt"
 	"mime"
-	"net"
 	"net/http"
 	"sort"
-	"strconv"
 	"strings"
 	"sync"
 	"time"
@@ -78,12 +76,8 @@ type APIGroupInfo struct {
 
 // GenericAPIServer contains state for a Kubernetes cluster api server.
 type GenericAPIServer struct {
-	// ServiceClusterIPRange is used to build cluster IPs for discovery.  It is exposed so that `master.go` can
+	// discoveryAddresses is used to build cluster IPs for discovery.
-	// construct service storage.
+	discoveryAddresses DiscoveryAddresses
-	// TODO refactor this so that `master.go` drives the value used for discovery and the value here isn't exposed.
-	// that structure will force usage in the correct direction where the "owner" of the value is the source of
-	// truth for its value.
-	ServiceClusterIPRange *net.IPNet
 
 	// LoopbackClientConfig is a config for a privileged loopback connection to the API server
 	LoopbackClientConfig *restclient.Config
@@ -152,13 +146,6 @@ type GenericAPIServer struct {
 	healthzLock    sync.Mutex
 	healthzChecks  []healthz.HealthzChecker
 	healthzCreated bool
-
-	// See Config.$name for documentation of these flags:
-
-	MasterCount               int
-	KubernetesServiceNodePort int // TODO(sttts): move into master
-	ServiceReadWriteIP        net.IP
-	ServiceReadWritePort      int
 }
 
 func init() {
@@ -260,8 +247,10 @@ func (s *GenericAPIServer) InstallLegacyAPIGroup(apiPrefix string, apiGroupInfo
 	// Install the version handler.
 	// Add a handler at /<apiPrefix> to enumerate the supported api versions.
 	apiserver.AddApiWebService(s.Serializer, s.HandlerContainer.Container, apiPrefix, func(req *restful.Request) *unversioned.APIVersions {
+		clientIP := utilnet.GetClientIP(req.Request)
+
 		apiVersionsForDiscovery := unversioned.APIVersions{
-			ServerAddressByClientCIDRs: s.getServerAddressByClientCIDRs(req.Request),
+			ServerAddressByClientCIDRs: s.discoveryAddresses.ServerAddressByClientCIDRs(clientIP),
 			Versions:                   apiVersions,
 		}
 		return &apiVersionsForDiscovery
@@ -328,26 +317,6 @@ func (s *GenericAPIServer) RemoveAPIGroupForDiscovery(groupName string) {
 	delete(s.apiGroupsForDiscovery, groupName)
 }
 
-func (s *GenericAPIServer) getServerAddressByClientCIDRs(req *http.Request) []unversioned.ServerAddressByClientCIDR {
-	addressCIDRMap := []unversioned.ServerAddressByClientCIDR{
-		{
-			ClientCIDR:    "0.0.0.0/0",
-			ServerAddress: s.ExternalAddress,
-		},
-	}
-
-	// Add internal CIDR if the request came from internal IP.
-	clientIP := utilnet.GetClientIP(req)
-	clusterCIDR := s.ServiceClusterIPRange
-	if clusterCIDR.Contains(clientIP) {
-		addressCIDRMap = append(addressCIDRMap, unversioned.ServerAddressByClientCIDR{
-			ClientCIDR:    clusterCIDR.String(),
-			ServerAddress: net.JoinHostPort(s.ServiceReadWriteIP.String(), strconv.Itoa(s.ServiceReadWritePort)),
-		})
-	}
-	return addressCIDRMap
-}
-
 func (s *GenericAPIServer) getAPIGroupVersion(apiGroupInfo *APIGroupInfo, groupVersion unversioned.GroupVersion, apiPrefix string) (*apiserver.APIGroupVersion, error) {
 	storage := make(map[string]rest.Storage)
 	for k, v := range apiGroupInfo.VersionedResourcesStorageMap[groupVersion.Version] {
@@ -397,7 +366,8 @@ func (s *GenericAPIServer) DynamicApisDiscovery() *restful.WebService {
 			sortedGroups = append(sortedGroups, s.apiGroupsForDiscovery[groupName])
 		}
 
-		serverCIDR := s.getServerAddressByClientCIDRs(req.Request)
+		clientIP := utilnet.GetClientIP(req.Request)
+		serverCIDR := s.discoveryAddresses.ServerAddressByClientCIDRs(clientIP)
 		groups := make([]unversioned.APIGroup, len(sortedGroups))
 		for i := range sortedGroups {
 			groups[i] = sortedGroups[i]

pkg/genericapiserver/genericapiserver_test.go

@@ -25,7 +25,6 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"reflect"
-	"strconv"
 	"testing"
 
 	"k8s.io/kubernetes/pkg/api"
@@ -37,8 +36,8 @@ import (
 	"k8s.io/kubernetes/pkg/auth/authorizer"
 	"k8s.io/kubernetes/pkg/auth/user"
 	openapigen "k8s.io/kubernetes/pkg/generated/openapi"
-	ipallocator "k8s.io/kubernetes/pkg/registry/core/service/ipallocator"
 	etcdtesting "k8s.io/kubernetes/pkg/storage/etcd/testing"
+	utilnet "k8s.io/kubernetes/pkg/util/net"
 	"k8s.io/kubernetes/pkg/util/sets"
 	"k8s.io/kubernetes/pkg/version"
 
@@ -91,9 +90,6 @@ func TestNew(t *testing.T) {
 	assert.Equal(s.RequestContextMapper(), config.RequestContextMapper)
 
 	// these values get defaulted
-	_, serviceClusterIPRange, _ := net.ParseCIDR("10.0.0.0/24")
-	serviceReadWriteIP, _ := ipallocator.GetIndexedIP(serviceClusterIPRange, 1)
-	assert.Equal(s.ServiceReadWriteIP, serviceReadWriteIP)
 	assert.Equal(s.ExternalAddress, net.JoinHostPort(config.PublicAddress.String(), "6443"))
 }
 
@@ -366,7 +362,7 @@ func TestDiscoveryAtAPIS(t *testing.T) {
 	assert.Equal(extensions.GroupName, groupListGroup.Name)
 	assert.Equal(extensionsVersions, groupListGroup.Versions)
 	assert.Equal(extensionsPreferredVersion, groupListGroup.PreferredVersion)
-	assert.Equal(master.getServerAddressByClientCIDRs(&http.Request{}), groupListGroup.ServerAddressByClientCIDRs)
+	assert.Equal(master.discoveryAddresses.ServerAddressByClientCIDRs(utilnet.GetClientIP(&http.Request{})), groupListGroup.ServerAddressByClientCIDRs)
 
 	// Remove the group.
 	master.RemoveAPIGroupForDiscovery(extensions.GroupName)
@@ -379,20 +375,17 @@ func TestDiscoveryAtAPIS(t *testing.T) {
 }
 
 func TestGetServerAddressByClientCIDRs(t *testing.T) {
-	s, etcdserver, _, _ := newMaster(t)
-	defer etcdserver.Terminate(t)
-
 	publicAddressCIDRMap := []unversioned.ServerAddressByClientCIDR{
 		{
 			ClientCIDR:    "0.0.0.0/0",
-			ServerAddress: s.ExternalAddress,
+			ServerAddress: "ExternalAddress",
 		},
 	}
 	internalAddressCIDRMap := []unversioned.ServerAddressByClientCIDR{
 		publicAddressCIDRMap[0],
 		{
-			ClientCIDR:    s.ServiceClusterIPRange.String(),
+			ClientCIDR:    "10.0.0.0/24",
-			ServerAddress: net.JoinHostPort(s.ServiceReadWriteIP.String(), strconv.Itoa(s.ServiceReadWritePort)),
+			ServerAddress: "serviceIP",
 		},
 	}
 	internalIP := "10.0.0.1"
@@ -458,8 +451,13 @@ func TestGetServerAddressByClientCIDRs(t *testing.T) {
 		},
 	}
 
+	_, ipRange, _ := net.ParseCIDR("10.0.0.0/24")
+	discoveryAddresses := DefaultDiscoveryAddresses{DefaultAddress: "ExternalAddress"}
+	discoveryAddresses.DiscoveryCIDRRules = append(discoveryAddresses.DiscoveryCIDRRules,
+		DiscoveryCIDRRule{IPRange: *ipRange, Address: "serviceIP"})
+
 	for i, test := range testCases {
-		if a, e := s.getServerAddressByClientCIDRs(&test.Request), test.ExpectedMap; reflect.DeepEqual(e, a) != true {
+		if a, e := discoveryAddresses.ServerAddressByClientCIDRs(utilnet.GetClientIP(&test.Request)), test.ExpectedMap; reflect.DeepEqual(e, a) != true {
 			t.Fatalf("test case %d failed. expected: %v, actual: %v", i+1, e, a)
 		}
 	}

pkg/genericapiserver/services.go

@@ -0,0 +1,54 @@
+/*
+Copyright 2016 The Kubernetes Authors.
+
+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 genericapiserver
+
+import (
+	"fmt"
+	"net"
+
+	"github.com/golang/glog"
+
+	"k8s.io/kubernetes/pkg/registry/core/service/ipallocator"
+)
+
+// DefaultServiceIPRange takes a the serviceIPRange flag and returns the defaulted service ip range (if  needed),
+// api server service IP, and an error
+// TODO move this out of the genericapiserver package
+func DefaultServiceIPRange(passedServiceClusterIPRange net.IPNet) (net.IPNet, net.IP, error) {
+	serviceClusterIPRange := passedServiceClusterIPRange
+	if passedServiceClusterIPRange.IP == nil {
+		defaultNet := "10.0.0.0/24"
+		glog.Infof("Network range for service cluster IPs is unspecified. Defaulting to %v.", defaultNet)
+		_, defaultServiceClusterIPRange, err := net.ParseCIDR(defaultNet)
+		if err != nil {
+			return net.IPNet{}, net.IP{}, err
+		}
+		serviceClusterIPRange = *defaultServiceClusterIPRange
+	}
+	if size := ipallocator.RangeSize(&serviceClusterIPRange); size < 8 {
+		return net.IPNet{}, net.IP{}, fmt.Errorf("The service cluster IP range must be at least %d IP addresses", 8)
+	}
+
+	// Select the first valid IP from ServiceClusterIPRange to use as the GenericAPIServer service IP.
+	apiServerServiceIP, err := ipallocator.GetIndexedIP(&serviceClusterIPRange, 1)
+	if err != nil {
+		return net.IPNet{}, net.IP{}, err
+	}
+	glog.V(4).Infof("Setting service IP to %q (read-write).", apiServerServiceIP)
+
+	return serviceClusterIPRange, apiServerServiceIP, nil
+}

pkg/master/BUILD

@@ -52,6 +52,7 @@ go_library(
         "//pkg/apis/storage/v1beta1:go_default_library",
         "//pkg/client/clientset_generated/internalclientset/typed/core/internalversion:go_default_library",
         "//pkg/genericapiserver:go_default_library",
+        "//pkg/genericapiserver/options:go_default_library",
         "//pkg/healthz:go_default_library",
         "//pkg/kubelet/client:go_default_library",
         "//pkg/master/thirdparty:go_default_library",
@@ -116,7 +117,6 @@ go_test(
         "//pkg/generated/openapi:go_default_library",
         "//pkg/genericapiserver:go_default_library",
         "//pkg/kubelet/client:go_default_library",
-        "//pkg/registry/core/service/ipallocator:go_default_library",
         "//pkg/registry/registrytest:go_default_library",
         "//pkg/runtime:go_default_library",
         "//pkg/storage/etcd/testing:go_default_library",

pkg/master/controller.go

@@ -50,7 +50,7 @@ type Controller struct {
 
 	ServiceClusterIPRegistry rangeallocation.RangeRegistry
 	ServiceClusterIPInterval time.Duration
-	ServiceClusterIPRange    *net.IPNet
+	ServiceClusterIPRange    net.IPNet
 
 	ServiceNodePortRegistry rangeallocation.RangeRegistry
 	ServiceNodePortInterval time.Duration
@@ -87,21 +87,21 @@ func (c *Config) NewBootstrapController(legacyRESTStorage corerest.LegacyRESTSto
 		SystemNamespacesInterval: 1 * time.Minute,
 
 		ServiceClusterIPRegistry: legacyRESTStorage.ServiceClusterIPAllocator,
-		ServiceClusterIPRange:    c.GenericConfig.ServiceClusterIPRange,
+		ServiceClusterIPRange:    c.ServiceIPRange,
 		ServiceClusterIPInterval: 3 * time.Minute,
 
 		ServiceNodePortRegistry: legacyRESTStorage.ServiceNodePortAllocator,
-		ServiceNodePortRange:    c.GenericConfig.ServiceNodePortRange,
+		ServiceNodePortRange:    c.ServiceNodePortRange,
 		ServiceNodePortInterval: 3 * time.Minute,
 
 		PublicIP: c.GenericConfig.PublicAddress,
 
-		ServiceIP:                 c.GenericConfig.ServiceReadWriteIP,
+		ServiceIP:                 c.APIServerServiceIP,
-		ServicePort:               c.GenericConfig.ServiceReadWritePort,
+		ServicePort:               c.APIServerServicePort,
-		ExtraServicePorts:         c.GenericConfig.ExtraServicePorts,
+		ExtraServicePorts:         c.ExtraServicePorts,
-		ExtraEndpointPorts:        c.GenericConfig.ExtraEndpointPorts,
+		ExtraEndpointPorts:        c.ExtraEndpointPorts,
 		PublicServicePort:         c.GenericConfig.ReadWritePort,
-		KubernetesServiceNodePort: c.GenericConfig.KubernetesServiceNodePort,
+		KubernetesServiceNodePort: c.KubernetesServiceNodePort,
 	}
 }
 
@@ -117,7 +117,7 @@ func (c *Controller) Start() {
 		return
 	}
 
-	repairClusterIPs := servicecontroller.NewRepair(c.ServiceClusterIPInterval, c.ServiceRegistry, c.ServiceClusterIPRange, c.ServiceClusterIPRegistry)
+	repairClusterIPs := servicecontroller.NewRepair(c.ServiceClusterIPInterval, c.ServiceRegistry, &c.ServiceClusterIPRange, c.ServiceClusterIPRegistry)
 	repairNodePorts := portallocatorcontroller.NewRepair(c.ServiceNodePortInterval, c.ServiceRegistry, c.ServiceNodePortRange, c.ServiceNodePortRegistry)
 
 	// run all of the controllers once prior to returning from Start.

pkg/master/master.go

@@ -18,8 +18,10 @@ package master
 
 import (
 	"fmt"
+	"net"
 	"net/http"
 	"reflect"
+	"strconv"
 	"time"
 
 	"k8s.io/kubernetes/pkg/api"
@@ -37,9 +39,11 @@ import (
 	storageapiv1beta1 "k8s.io/kubernetes/pkg/apis/storage/v1beta1"
 	coreclient "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion"
 	"k8s.io/kubernetes/pkg/genericapiserver"
+	"k8s.io/kubernetes/pkg/genericapiserver/options"
 	"k8s.io/kubernetes/pkg/healthz"
 	kubeletclient "k8s.io/kubernetes/pkg/kubelet/client"
 	"k8s.io/kubernetes/pkg/master/thirdparty"
+	utilnet "k8s.io/kubernetes/pkg/util/net"
 
 	"k8s.io/kubernetes/pkg/registry/generic"
 	"k8s.io/kubernetes/pkg/registry/generic/registry"
@@ -84,6 +88,38 @@ type Config struct {
 	EnableUISupport   bool
 	EnableLogsSupport bool
 	ProxyTransport    http.RoundTripper
+
+	// Values to build the IP addresses used by discovery
+	// The range of IPs to be assigned to services with type=ClusterIP or greater
+	ServiceIPRange net.IPNet
+	// The IP address for the GenericAPIServer service (must be inside ServiceIPRange)
+	APIServerServiceIP net.IP
+	// Port for the apiserver service.
+	APIServerServicePort int
+
+	// TODO, we can probably group service related items into a substruct to make it easier to configure
+	// the API server items and `Extra*` fields likely fit nicely together.
+
+	// The range of ports to be assigned to services with type=NodePort or greater
+	ServiceNodePortRange utilnet.PortRange
+	// Additional ports to be exposed on the GenericAPIServer service
+	// extraServicePorts is injectable in the event that more ports
+	// (other than the default 443/tcp) are exposed on the GenericAPIServer
+	// and those ports need to be load balanced by the GenericAPIServer
+	// service because this pkg is linked by out-of-tree projects
+	// like openshift which want to use the GenericAPIServer but also do
+	// more stuff.
+	ExtraServicePorts []api.ServicePort
+	// Additional ports to be exposed on the GenericAPIServer endpoints
+	// Port names should align with ports defined in ExtraServicePorts
+	ExtraEndpointPorts []api.EndpointPort
+	// If non-zero, the "kubernetes" services uses this port as NodePort.
+	// TODO(sttts): move into master
+	KubernetesServiceNodePort int
+
+	// Number of masters running; all masters must be started with the
+	// same value for this field. (Numbers > 1 currently untested.)
+	MasterCount int
 }
 
 // EndpointReconcilerConfig holds the endpoint reconciler and endpoint reconciliation interval to be
@@ -106,6 +142,31 @@ type completedConfig struct {
 func (c *Config) Complete() completedConfig {
 	c.GenericConfig.Complete()
 
+	serviceIPRange, apiServerServiceIP, err := genericapiserver.DefaultServiceIPRange(c.ServiceIPRange)
+	if err != nil {
+		glog.Fatalf("Error determining service IP ranges: %v", err)
+	}
+	if c.ServiceIPRange.IP == nil {
+		c.ServiceIPRange = serviceIPRange
+	}
+	if c.APIServerServiceIP == nil {
+		c.APIServerServiceIP = apiServerServiceIP
+	}
+
+	discoveryAddresses := genericapiserver.DefaultDiscoveryAddresses{DefaultAddress: c.GenericConfig.ExternalAddress}
+	discoveryAddresses.DiscoveryCIDRRules = append(discoveryAddresses.DiscoveryCIDRRules,
+		genericapiserver.DiscoveryCIDRRule{IPRange: c.ServiceIPRange, Address: net.JoinHostPort(c.APIServerServiceIP.String(), strconv.Itoa(c.APIServerServicePort))})
+	c.GenericConfig.DiscoveryAddresses = discoveryAddresses
+
+	if c.ServiceNodePortRange.Size == 0 {
+		// TODO: Currently no way to specify an empty range (do we need to allow this?)
+		// We should probably allow this for clouds that don't require NodePort to do load-balancing (GCE)
+		// but then that breaks the strict nestedness of ServiceType.
+		// Review post-v1
+		c.ServiceNodePortRange = options.DefaultServiceNodePortRange
+		glog.Infof("Node port range unspecified. Defaulting to %v.", c.ServiceNodePortRange)
+	}
+
 	// enable swagger UI only if general UI support is on
 	c.GenericConfig.EnableSwaggerUI = c.GenericConfig.EnableSwaggerUI && c.EnableUISupport
 
@@ -116,7 +177,7 @@ func (c *Config) Complete() completedConfig {
 	if c.EndpointReconcilerConfig.Reconciler == nil {
 		// use a default endpoint reconciler if nothing is set
 		endpointClient := coreclient.NewForConfigOrDie(c.GenericConfig.LoopbackClientConfig)
-		c.EndpointReconcilerConfig.Reconciler = NewMasterCountEndpointReconciler(c.GenericConfig.MasterCount, endpointClient)
+		c.EndpointReconcilerConfig.Reconciler = NewMasterCountEndpointReconciler(c.MasterCount, endpointClient)
 	}
 
 	// this has always been hardcoded true in the past
@@ -174,8 +235,8 @@ func (c completedConfig) New() (*Master, error) {
 			ProxyTransport:       c.ProxyTransport,
 			KubeletClientConfig:  c.KubeletClientConfig,
 			EventTTL:             c.EventTTL,
-			ServiceClusterIPRange: c.GenericConfig.ServiceClusterIPRange,
+			ServiceIPRange:       c.ServiceIPRange,
-			ServiceNodePortRange:  c.GenericConfig.ServiceNodePortRange,
+			ServiceNodePortRange: c.ServiceNodePortRange,
 			LoopbackClientConfig: c.GenericConfig.LoopbackClientConfig,
 		}
 		m.InstallLegacyAPI(c.Config, restOptionsFactory.NewFor, legacyRESTStorageProvider)

pkg/master/master_test.go

@@ -47,7 +47,6 @@ import (
 	openapigen "k8s.io/kubernetes/pkg/generated/openapi"
 	"k8s.io/kubernetes/pkg/genericapiserver"
 	kubeletclient "k8s.io/kubernetes/pkg/kubelet/client"
-	ipallocator "k8s.io/kubernetes/pkg/registry/core/service/ipallocator"
 	"k8s.io/kubernetes/pkg/registry/registrytest"
 	"k8s.io/kubernetes/pkg/runtime"
 	etcdtesting "k8s.io/kubernetes/pkg/storage/etcd/testing"
@@ -68,6 +67,8 @@ func setUp(t *testing.T) (*Master, *etcdtesting.EtcdTestServer, Config, *assert.
 
 	config := &Config{
 		GenericConfig:        genericapiserver.NewConfig(),
+		APIServerServicePort: 443,
+		MasterCount:          1,
 	}
 
 	resourceEncoding := genericapiserver.NewDefaultResourceEncodingConfig()
@@ -143,19 +144,6 @@ func newLimitedMaster(t *testing.T) (*Master, *etcdtesting.EtcdTestServer, Confi
 	return master, etcdserver, config, assert
 }
 
-// TestNew verifies that the New function returns a Master
-// using the configuration properly.
-func TestNew(t *testing.T) {
-	master, etcdserver, _, assert := newMaster(t)
-	defer etcdserver.Terminate(t)
-
-	// these values get defaulted
-	_, serviceClusterIPRange, _ := net.ParseCIDR("10.0.0.0/24")
-	serviceReadWriteIP, _ := ipallocator.GetIndexedIP(serviceClusterIPRange, 1)
-	assert.Equal(master.GenericAPIServer.MasterCount, 1)
-	assert.Equal(master.GenericAPIServer.ServiceReadWriteIP, serviceReadWriteIP)
-}
-
 // TestVersion tests /version
 func TestVersion(t *testing.T) {
 	s, etcdserver, _, _ := newMaster(t)

pkg/registry/core/rest/storage_core.go

@@ -75,8 +75,8 @@ type LegacyRESTStorageProvider struct {
 	KubeletClientConfig kubeletclient.KubeletClientConfig
 	EventTTL            time.Duration
 
-	// ServiceClusterIPRange is used to build cluster IPs for discovery.
+	// ServiceIPRange is used to build cluster IPs for discovery.
-	ServiceClusterIPRange *net.IPNet
+	ServiceIPRange       net.IPNet
 	ServiceNodePortRange utilnet.PortRange
 
 	LoopbackClientConfig *restclient.Config
@@ -154,9 +154,9 @@ func (c LegacyRESTStorageProvider) NewLegacyRESTStorage(restOptionsGetter generi
 	restStorage.ServiceRegistry = service.NewRegistry(serviceRESTStorage)
 
 	var serviceClusterIPRegistry rangeallocation.RangeRegistry
-	serviceClusterIPRange := c.ServiceClusterIPRange
+	serviceClusterIPRange := c.ServiceIPRange
-	if serviceClusterIPRange == nil {
+	if serviceClusterIPRange.IP == nil {
-		return LegacyRESTStorage{}, genericapiserver.APIGroupInfo{}, fmt.Errorf("service clusterIPRange is nil")
+		return LegacyRESTStorage{}, genericapiserver.APIGroupInfo{}, fmt.Errorf("service clusterIPRange is missing")
 	}
 
 	serviceStorageConfig, err := c.StorageFactory.NewConfig(api.Resource("services"))
@@ -164,7 +164,7 @@ func (c LegacyRESTStorageProvider) NewLegacyRESTStorage(restOptionsGetter generi
 		return LegacyRESTStorage{}, genericapiserver.APIGroupInfo{}, err
 	}
 
-	ServiceClusterIPAllocator := ipallocator.NewAllocatorCIDRRange(serviceClusterIPRange, func(max int, rangeSpec string) allocator.Interface {
+	ServiceClusterIPAllocator := ipallocator.NewAllocatorCIDRRange(&serviceClusterIPRange, func(max int, rangeSpec string) allocator.Interface {
 		mem := allocator.NewAllocationMap(max, rangeSpec)
 		// TODO etcdallocator package to return a storage interface via the storageFactory
 		etcd := etcdallocator.NewEtcd(mem, "/ranges/serviceips", api.Resource("serviceipallocations"), serviceStorageConfig)

test/integration/framework/master_utils.go

@@ -359,6 +359,8 @@ func NewMasterConfig() *master.Config {
 		EnableCoreControllers: true,
 		EnableWatchCache:      true,
 		KubeletClientConfig:   kubeletclient.KubeletClientConfig{Port: 10250},
+		APIServerServicePort:  443,
+		MasterCount:           1,
 	}
 }
 

test/integration/master/master_test.go

@@ -425,11 +425,11 @@ func TestMasterService(t *testing.T) {
 
 func TestServiceAlloc(t *testing.T) {
 	cfg := framework.NewIntegrationTestMasterConfig()
-	_, cidr, err := net.ParseCIDR("192.168.0.0/30")
+	_, cidr, err := net.ParseCIDR("192.168.0.0/29")
 	if err != nil {
 		t.Fatalf("bad cidr: %v", err)
 	}
-	cfg.GenericConfig.ServiceClusterIPRange = cidr
+	cfg.ServiceIPRange = *cidr
 	_, s := framework.RunAMaster(cfg)
 	defer s.Close()
 
@@ -460,13 +460,15 @@ func TestServiceAlloc(t *testing.T) {
 		t.Fatalf("creating kubernetes service timed out")
 	}
 
-	// Make a service.
+	// make 5 more services to take up all IPs
-	if _, err := client.Core().Services(api.NamespaceDefault).Create(svc(1)); err != nil {
+	for i := 0; i < 5; i++ {
-		t.Fatalf("got unexpected error: %v", err)
+		if _, err := client.Core().Services(api.NamespaceDefault).Create(svc(i)); err != nil {
+			t.Error(err)
+		}
 	}
 
-	// Make a second service. It will fail because we're out of cluster IPs
+	// Make another service. It will fail because we're out of cluster IPs
-	if _, err := client.Core().Services(api.NamespaceDefault).Create(svc(2)); err != nil {
+	if _, err := client.Core().Services(api.NamespaceDefault).Create(svc(8)); err != nil {
 		if !strings.Contains(err.Error(), "range is full") {
 			t.Errorf("unexpected error text: %v", err)
 		}
@@ -488,7 +490,7 @@ func TestServiceAlloc(t *testing.T) {
 	}
 
 	// This time creating the second service should work.
-	if _, err := client.Core().Services(api.NamespaceDefault).Create(svc(2)); err != nil {
+	if _, err := client.Core().Services(api.NamespaceDefault).Create(svc(8)); err != nil {
 		t.Fatalf("got unexpected error: %v", err)
 	}
 }