Federation: separate notion of zone-name & dns-suffix

federation/cmd/federation-controller-manager/app/controllermanager.go

@@ -179,7 +179,7 @@ func StartControllers(s *options.CMServer, restClientCfg *restclient.Config) err
 
 	glog.Infof("Loading client config for service controller %q", servicecontroller.UserAgentName)
 	scClientset := federationclientset.NewForConfigOrDie(restclient.AddUserAgent(restClientCfg, servicecontroller.UserAgentName))
-	servicecontroller := servicecontroller.New(scClientset, dns, s.FederationName, s.ZoneName)
+	servicecontroller := servicecontroller.New(scClientset, dns, s.FederationName, s.ServiceDnsSuffix, s.ZoneName)
 	glog.Infof("Running service controller")
 	if err := servicecontroller.Run(s.ConcurrentServiceSyncs, wait.NeverStop); err != nil {
 		glog.Errorf("Failed to start service controller: %v", err)

federation/cmd/federation-controller-manager/app/options/options.go

@@ -39,6 +39,8 @@ type ControllerManagerConfiguration struct {
 	FederationName string `json:"federationName"`
 	// zone name, like example.com.
 	ZoneName string `json:"zoneName"`
+	// ServiceDnsSuffix is the dns suffix to use when publishing federated services.
+	ServiceDnsSuffix string `json:"serviceDnsSuffix"`
 	// dnsProvider is the provider for dns services.
 	DnsProvider string `json:"dnsProvider"`
 	// dnsConfigFile is the path to the dns provider configuration file.
@@ -101,6 +103,7 @@ func (s *CMServer) AddFlags(fs *pflag.FlagSet) {
 	fs.Var(componentconfig.IPVar{Val: &s.Address}, "address", "The IP address to serve on (set to 0.0.0.0 for all interfaces)")
 	fs.StringVar(&s.FederationName, "federation-name", s.FederationName, "Federation name.")
 	fs.StringVar(&s.ZoneName, "zone-name", s.ZoneName, "Zone name, like example.com.")
+	fs.StringVar(&s.ServiceDnsSuffix, "service-dns-suffix", s.ServiceDnsSuffix, "DNS Suffix to use when publishing federated service names.  Defaults to zone-name")
 	fs.IntVar(&s.ConcurrentServiceSyncs, "concurrent-service-syncs", s.ConcurrentServiceSyncs, "The number of service syncing operations that will be done concurrently. Larger number = faster endpoint updating, but more CPU (and network) load")
 	fs.IntVar(&s.ConcurrentReplicaSetSyncs, "concurrent-replicaset-syncs", s.ConcurrentReplicaSetSyncs, "The number of ReplicaSets syncing operations that will be done concurrently. Larger number = faster endpoint updating, but more CPU (and network) load")
 	fs.DurationVar(&s.ClusterMonitorPeriod.Duration, "cluster-monitor-period", s.ClusterMonitorPeriod.Duration, "The period for syncing ClusterStatus in ClusterController.")

federation/pkg/federation-controller/service/BUILD

@@ -50,6 +50,7 @@ go_library(
 go_test(
     name = "go_default_test",
     srcs = [
+        "dns_test.go",
         "endpoint_helper_test.go",
         "service_helper_test.go",
         "servicecontroller_test.go",

federation/pkg/federation-controller/service/dns.go

@@ -24,6 +24,7 @@ import (
 
 	"k8s.io/kubernetes/federation/pkg/dnsprovider"
 	"k8s.io/kubernetes/federation/pkg/dnsprovider/rrstype"
+	"strings"
 )
 
 const (
@@ -89,22 +90,28 @@ func (s *ServiceController) getClusterZoneNames(clusterName string) (zones []str
 	return client.cluster.Status.Zones, client.cluster.Status.Region, nil
 }
 
-// getFederationDNSZoneName returns the name of the managed DNS Zone configured for this federation
-func (s *ServiceController) getFederationDNSZoneName() (string, error) {
-	return s.zoneName, nil
+// getServiceDnsSuffix returns the DNS suffix to use when creating federated-service DNS records
+func (s *ServiceController) getServiceDnsSuffix() (string, error) {
+	return s.serviceDnsSuffix, nil
 }
 
-// getDnsZone is a hack around the fact that dnsprovider does not yet support a Get() method, only a List() method.  TODO:  Fix that.
+// getDnsZone returns the zone, as identified by zoneName
 func getDnsZone(dnsZoneName string, dnsZonesInterface dnsprovider.Zones) (dnsprovider.Zone, error) {
+	// TODO: We need query-by-name and query-by-id functions
 	dnsZones, err := dnsZonesInterface.List()
 	if err != nil {
 		return nil, err
 	}
+
+	findName := strings.TrimSuffix(dnsZoneName, ".")
 	for _, dnsZone := range dnsZones {
-		if dnsZone.Name() == dnsZoneName {
-			return dnsZone, nil
+		if findName != "" {
+			if strings.TrimSuffix(dnsZone.Name(), ".") == findName {
+				return dnsZone, nil
+			}
 		}
 	}
+
 	return nil, fmt.Errorf("DNS zone %s not found.", dnsZoneName)
 }
 
@@ -152,11 +159,7 @@ func getResolvedEndpoints(endpoints []string) ([]string, error) {
 /* ensureDnsRrsets ensures (idempotently, and with minimum mutations) that all of the DNS resource record sets for dnsName are consistent with endpoints.
    if endpoints is nil or empty, a CNAME record to uplevelCname is ensured.
 */
-func (s *ServiceController) ensureDnsRrsets(dnsZoneName, dnsName string, endpoints []string, uplevelCname string) error {
-	dnsZone, err := getDnsZone(dnsZoneName, s.dnsZones)
-	if err != nil {
-		return err
-	}
+func (s *ServiceController) ensureDnsRrsets(dnsZone dnsprovider.Zone, dnsName string, endpoints []string, uplevelCname string) error {
 	rrsets, supported := dnsZone.ResourceRecordSets()
 	if !supported {
 		return fmt.Errorf("Failed to ensure DNS records for %s. DNS provider does not support the ResourceRecordSets interface.", dnsName)
@@ -258,7 +261,7 @@ func (s *ServiceController) ensureDnsRrsets(dnsZoneName, dnsName string, endpoin
 
 /* ensureDnsRecords ensures (idempotently, and with minimum mutations) that all of the DNS records for a service in a given cluster are correct,
 given the current state of that service in that cluster.  This should be called every time the state of a service might have changed
-(either w.r.t. it's loadblancer address, or if the number of healthy backend endpoints for that service transitioned from zero to non-zero
+(either w.r.t. it's loadbalancer address, or if the number of healthy backend endpoints for that service transitioned from zero to non-zero
 (or vice verse).  Only shards of the service which have both a loadbalancer ingress IP address or hostname AND at least one healthy backend endpoint
 are included in DNS records for that service (at all of zone, region and global levels). All other addresses are removed.  Also, if no shards exist
 in the zone or region of the cluster, a CNAME reference to the next higher level is ensured to exist. */
@@ -298,7 +301,7 @@ func (s *ServiceController) ensureDnsRecords(clusterName string, cachedService *
 	if zoneNames == nil {
 		return fmt.Errorf("failed to get cluster zone names")
 	}
-	dnsZoneName, err := s.getFederationDNSZoneName()
+	serviceDnsSuffix, err := s.getServiceDnsSuffix()
 	if err != nil {
 		return err
 	}
@@ -309,16 +312,21 @@ func (s *ServiceController) ensureDnsRecords(clusterName string, cachedService *
 	commonPrefix := serviceName + "." + namespaceName + "." + s.federationName + ".svc"
 	// dnsNames is the path up the DNS search tree, starting at the leaf
 	dnsNames := []string{
-		commonPrefix + "." + zoneNames[0] + "." + regionName + "." + dnsZoneName, // zone level - TODO might need other zone names for multi-zone clusters
-		commonPrefix + "." + regionName + "." + dnsZoneName,                      // region level, one up from zone level
-		commonPrefix + "." + dnsZoneName,                                         // global level, one up from region level
+		commonPrefix + "." + zoneNames[0] + "." + regionName + "." + serviceDnsSuffix, // zone level - TODO might need other zone names for multi-zone clusters
+		commonPrefix + "." + regionName + "." + serviceDnsSuffix,                      // region level, one up from zone level
+		commonPrefix + "." + serviceDnsSuffix,                                         // global level, one up from region level
 		"", // nowhere to go up from global level
 	}
 
 	endpoints := [][]string{zoneEndpoints, regionEndpoints, globalEndpoints}
 
+	dnsZone, err := getDnsZone(s.zoneName, s.dnsZones)
+	if err != nil {
+		return err
+	}
+
 	for i, endpoint := range endpoints {
-		if err = s.ensureDnsRrsets(dnsZoneName, dnsNames[i], endpoint, dnsNames[i+1]); err != nil {
+		if err = s.ensureDnsRrsets(dnsZone, dnsNames[i], endpoint, dnsNames[i+1]); err != nil {
 			return err
 		}
 	}

federation/pkg/federation-controller/service/dns_test.go

@@ -0,0 +1,170 @@
+/*
+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 service
+
+import (
+	"sync"
+	"testing"
+
+	"fmt"
+	"k8s.io/kubernetes/federation/apis/federation/v1beta1"
+	"k8s.io/kubernetes/federation/pkg/dnsprovider/providers/google/clouddns" // Only for unit testing purposes.
+	"k8s.io/kubernetes/pkg/api/v1"
+	"k8s.io/kubernetes/pkg/util/sets"
+	"reflect"
+	"sort"
+)
+
+func TestServiceController_ensureDnsRecords(t *testing.T) {
+	tests := []struct {
+		name          string
+		service       v1.Service
+		expected      []string
+		serviceStatus v1.LoadBalancerStatus
+	}{
+		{
+			name: "withip",
+			service: v1.Service{
+				ObjectMeta: v1.ObjectMeta{
+					Name:      "servicename",
+					Namespace: "servicenamespace",
+				},
+			},
+			serviceStatus: buildServiceStatus([][]string{{"198.51.100.1", ""}}),
+			expected: []string{
+				"example.com:servicename.servicenamespace.myfederation.svc.federation.example.com:A:180:[198.51.100.1]",
+				"example.com:servicename.servicenamespace.myfederation.svc.fooregion.federation.example.com:A:180:[198.51.100.1]",
+				"example.com:servicename.servicenamespace.myfederation.svc.foozone.fooregion.federation.example.com:A:180:[198.51.100.1]",
+			},
+		},
+		/*
+			TODO: getResolvedEndpoints preforms DNS lookup.
+			Mock and maybe look at error handling when some endpoints resolve, but also caching?
+			{
+				name: "withname",
+				service: v1.Service{
+					ObjectMeta: v1.ObjectMeta{
+						Name: "servicename",
+						Namespace: "servicenamespace",
+					},
+				},
+				serviceStatus: buildServiceStatus([][]string{{"", "randomstring.amazonelb.example.com"}}),
+				expected: []string{
+					"example.com:servicename.servicenamespace.myfederation.svc.federation.example.com:A:180:[198.51.100.1]",
+					"example.com:servicename.servicenamespace.myfederation.svc.fooregion.federation.example.com:A:180:[198.51.100.1]",
+					"example.com:servicename.servicenamespace.myfederation.svc.foozone.fooregion.federation.example.com:A:180:[198.51.100.1]",
+				},
+			},
+		*/
+		{
+			name: "noendpoints",
+			service: v1.Service{
+				ObjectMeta: v1.ObjectMeta{
+					Name:      "servicename",
+					Namespace: "servicenamespace",
+				},
+			},
+			expected: []string{
+				"example.com:servicename.servicenamespace.myfederation.svc.fooregion.federation.example.com:CNAME:180:[servicename.servicenamespace.myfederation.svc.federation.example.com]",
+				"example.com:servicename.servicenamespace.myfederation.svc.foozone.fooregion.federation.example.com:CNAME:180:[servicename.servicenamespace.myfederation.svc.fooregion.federation.example.com]",
+			},
+		},
+	}
+	for _, test := range tests {
+		fakedns, _ := clouddns.NewFakeInterface()
+		fakednsZones, ok := fakedns.Zones()
+		if !ok {
+			t.Error("Unable to fetch zones")
+		}
+		serviceController := ServiceController{
+			dns:              fakedns,
+			dnsZones:         fakednsZones,
+			serviceDnsSuffix: "federation.example.com",
+			zoneName:         "example.com",
+			federationName:   "myfederation",
+			serviceCache:     &serviceCache{fedServiceMap: make(map[string]*cachedService)},
+			clusterCache: &clusterClientCache{
+				rwlock:    sync.Mutex{},
+				clientMap: make(map[string]*clusterCache),
+			},
+			knownClusterSet: make(sets.String),
+		}
+
+		clusterName := "testcluster"
+
+		serviceController.clusterCache.clientMap[clusterName] = &clusterCache{
+			cluster: &v1beta1.Cluster{
+				Status: v1beta1.ClusterStatus{
+					Zones:  []string{"foozone"},
+					Region: "fooregion",
+				},
+			},
+		}
+
+		cachedService := &cachedService{
+			lastState:        &test.service,
+			endpointMap:      make(map[string]int),
+			serviceStatusMap: make(map[string]v1.LoadBalancerStatus),
+		}
+		cachedService.endpointMap[clusterName] = 1
+		if !reflect.DeepEqual(&test.serviceStatus, &v1.LoadBalancerStatus{}) {
+			cachedService.serviceStatusMap[clusterName] = test.serviceStatus
+		}
+
+		err := serviceController.ensureDnsRecords(clusterName, cachedService)
+		if err != nil {
+			t.Errorf("Test failed for %s, unexpected error %v", test.name, err)
+		}
+
+		zones, err := fakednsZones.List()
+		if err != nil {
+			t.Errorf("error querying zones: %v", err)
+		}
+
+		// Dump every record to a testable-by-string-comparison form
+		var records []string
+		for _, z := range zones {
+			zoneName := z.Name()
+
+			rrs, ok := z.ResourceRecordSets()
+			if !ok {
+				t.Errorf("cannot get rrs for zone %q", zoneName)
+			}
+
+			rrList, err := rrs.List()
+			if err != nil {
+				t.Errorf("error querying rr for zone %q: %v", zoneName, err)
+			}
+			for _, rr := range rrList {
+				rrdatas := rr.Rrdatas()
+
+				// Put in consistent (testable-by-string-comparison) order
+				sort.Strings(rrdatas)
+				records = append(records, fmt.Sprintf("%s:%s:%s:%d:%s", zoneName, rr.Name(), rr.Type(), rr.Ttl(), rrdatas))
+			}
+		}
+
+		// Ignore order of records
+		sort.Strings(records)
+		sort.Strings(test.expected)
+
+		if !reflect.DeepEqual(records, test.expected) {
+			t.Errorf("Test %q failed.  Actual=%v, Expected=%v", test.name, records, test.expected)
+		}
+
+	}
+}

federation/pkg/federation-controller/service/endpoint_helper_test.go

@@ -29,11 +29,12 @@ var fakeDns, _ = clouddns.NewFakeInterface() // No need to check for unsupported
 var fakeDnsZones, _ = fakeDns.Zones()
 
 var fakeServiceController = ServiceController{
-	dns:            fakeDns,
-	dnsZones:       fakeDnsZones,
-	federationName: "fed1",
-	zoneName:       "example.com",
-	serviceCache:   &serviceCache{fedServiceMap: make(map[string]*cachedService)},
+	dns:              fakeDns,
+	dnsZones:         fakeDnsZones,
+	federationName:   "fed1",
+	zoneName:         "example.com",
+	serviceDnsSuffix: "federation.example.com",
+	serviceCache:     &serviceCache{fedServiceMap: make(map[string]*cachedService)},
 	clusterCache: &clusterClientCache{
 		clientMap: make(map[string]*clusterCache),
 	},

federation/pkg/federation-controller/service/servicecontroller.go

@@ -103,7 +103,10 @@ type ServiceController struct {
 	dns              dnsprovider.Interface
 	federationClient fedclientset.Interface
 	federationName   string
-	zoneName         string
+	// serviceDnsSuffix is the DNS suffix we use when publishing service DNS names
+	serviceDnsSuffix string
+	// zoneName is used to identify the zone in which to put records
+	zoneName string
 	// each federation should be configured with a single zone (e.g. "mycompany.com")
 	dnsZones     dnsprovider.Zones
 	serviceCache *serviceCache
@@ -136,7 +139,8 @@ type ServiceController struct {
 // New returns a new service controller to keep DNS provider service resources
 // (like Kubernetes Services and DNS server records for service discovery) in sync with the registry.
 
-func New(federationClient fedclientset.Interface, dns dnsprovider.Interface, federationName, zoneName string) *ServiceController {
+func New(federationClient fedclientset.Interface, dns dnsprovider.Interface,
+	federationName, serviceDnsSuffix, zoneName string) *ServiceController {
 	broadcaster := record.NewBroadcaster()
 	// federationClient event is not supported yet
 	// broadcaster.StartRecordingToSink(&unversioned_core.EventSinkImpl{Interface: kubeClient.Core().Events("")})
@@ -146,6 +150,7 @@ func New(federationClient fedclientset.Interface, dns dnsprovider.Interface, fed
 		dns:              dns,
 		federationClient: federationClient,
 		federationName:   federationName,
+		serviceDnsSuffix: serviceDnsSuffix,
 		zoneName:         zoneName,
 		serviceCache:     &serviceCache{fedServiceMap: make(map[string]*cachedService)},
 		clusterCache: &clusterClientCache{
@@ -277,6 +282,13 @@ func (s *ServiceController) init() error {
 	if s.zoneName == "" {
 		return fmt.Errorf("ServiceController should not be run without zoneName.")
 	}
+	if s.serviceDnsSuffix == "" {
+		// TODO: Is this the right place to do defaulting?
+		if s.zoneName == "" {
+			return fmt.Errorf("ServiceController must be run with zoneName, if serviceDnsSuffix is not set.")
+		}
+		s.serviceDnsSuffix = s.zoneName
+	}
 	if s.dns == nil {
 		return fmt.Errorf("ServiceController should not be run without a dnsprovider.")
 	}

hack/verify-flags/known-flags.txt

@@ -505,6 +505,7 @@ service-address
 service-cidr
 service-cluster-ip-range
 service-dns-domain
+service-dns-suffix
 service-generator
 service-node-port-range
 service-node-ports