From df4280651c272ddc6db1fe9694214058529edfec Mon Sep 17 00:00:00 2001 From: Justin Santa Barbara Date: Sun, 6 Nov 2016 23:59:01 -0500 Subject: [PATCH] Federation: allow specification of dns zone by ID If we have a public & private zone with the same name (which is common on AWS), this means we can still create records. Also tighten up some of the logic to allow for zones with duplicate names. --- .../app/controllermanager.go | 2 +- .../app/options/options.go | 3 ++ .../pkg/federation-controller/service/dns.go | 45 ++++++++++++++++--- .../service/servicecontroller.go | 22 ++++++--- hack/verify-flags/known-flags.txt | 1 + 5 files changed, 60 insertions(+), 13 deletions(-) diff --git a/federation/cmd/federation-controller-manager/app/controllermanager.go b/federation/cmd/federation-controller-manager/app/controllermanager.go index e4d94774671..b2f0781a3a2 100644 --- a/federation/cmd/federation-controller-manager/app/controllermanager.go +++ b/federation/cmd/federation-controller-manager/app/controllermanager.go @@ -190,7 +190,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.ServiceDnsSuffix, s.ZoneName) + servicecontroller := servicecontroller.New(scClientset, dns, s.FederationName, s.ServiceDnsSuffix, s.ZoneName, s.ZoneID) glog.Infof("Running service controller") if err := servicecontroller.Run(s.ConcurrentServiceSyncs, wait.NeverStop); err != nil { glog.Errorf("Failed to start service controller: %v", err) diff --git a/federation/cmd/federation-controller-manager/app/options/options.go b/federation/cmd/federation-controller-manager/app/options/options.go index fb8f52bda5e..970aa1442eb 100644 --- a/federation/cmd/federation-controller-manager/app/options/options.go +++ b/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"` + // zone ID, for use when zoneName is ambiguous. + ZoneID string `json:"zoneID"` // ServiceDnsSuffix is the dns suffix to use when publishing federated services. ServiceDnsSuffix string `json:"serviceDnsSuffix"` // dnsProvider is the provider for dns services. @@ -103,6 +105,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.ZoneID, "zone-id", s.ZoneID, "Zone ID, needed if the zone name is not unique.") 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") diff --git a/federation/pkg/federation-controller/service/dns.go b/federation/pkg/federation-controller/service/dns.go index 20366274dc2..4e50df14e7e 100644 --- a/federation/pkg/federation-controller/service/dns.go +++ b/federation/pkg/federation-controller/service/dns.go @@ -95,24 +95,55 @@ func (s *ServiceController) getServiceDnsSuffix() (string, error) { return s.serviceDnsSuffix, nil } -// getDnsZone returns the zone, as identified by zoneName -func getDnsZone(dnsZoneName string, dnsZonesInterface dnsprovider.Zones) (dnsprovider.Zone, error) { +// getDnsZones returns the DNS zones matching dnsZoneName and dnsZoneID (if specified) +func getDnsZones(dnsZoneName string, dnsZoneID 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 } + var matches []dnsprovider.Zone findName := strings.TrimSuffix(dnsZoneName, ".") for _, dnsZone := range dnsZones { - if findName != "" { - if strings.TrimSuffix(dnsZone.Name(), ".") == findName { - return dnsZone, nil + if dnsZoneID != "" { + if dnsZoneID != dnsZone.ID() { + continue } } + if findName != "" { + if strings.TrimSuffix(dnsZone.Name(), ".") != findName { + continue + } + } + matches = append(matches, dnsZone) } - return nil, fmt.Errorf("DNS zone %s not found.", dnsZoneName) + return matches, nil +} + +// getDnsZone returns the DNS zone, as identified by dnsZoneName and dnsZoneID +// This is similar to getDnsZones, but returns an error if there are zero or multiple matching zones. +func getDnsZone(dnsZoneName string, dnsZoneID string, dnsZonesInterface dnsprovider.Zones) (dnsprovider.Zone, error) { + dnsZones, err := getDnsZones(dnsZoneName, dnsZoneID, dnsZonesInterface) + if err != nil { + return nil, err + } + + if len(dnsZones) == 1 { + return dnsZones[0], nil + } + + name := dnsZoneName + if dnsZoneID != "" { + name += "/" + dnsZoneID + } + + if len(dnsZones) == 0 { + return nil, fmt.Errorf("DNS zone %s not found.", name) + } else { + return nil, fmt.Errorf("DNS zone %s is ambiguous (please specify zoneID).", name) + } } /* getRrset is a hack around the fact that dnsprovider.ResourceRecordSets interface does not yet include a Get() method, only a List() method. TODO: Fix that. @@ -320,7 +351,7 @@ func (s *ServiceController) ensureDnsRecords(clusterName string, cachedService * endpoints := [][]string{zoneEndpoints, regionEndpoints, globalEndpoints} - dnsZone, err := getDnsZone(s.zoneName, s.dnsZones) + dnsZone, err := getDnsZone(s.zoneName, s.zoneID, s.dnsZones) if err != nil { return err } diff --git a/federation/pkg/federation-controller/service/servicecontroller.go b/federation/pkg/federation-controller/service/servicecontroller.go index 69a33255cb3..8b434a7cae2 100644 --- a/federation/pkg/federation-controller/service/servicecontroller.go +++ b/federation/pkg/federation-controller/service/servicecontroller.go @@ -105,8 +105,9 @@ type ServiceController struct { federationName 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 and zoneID are used to identify the zone in which to put records zoneName string + zoneID string // each federation should be configured with a single zone (e.g. "mycompany.com") dnsZones dnsprovider.Zones serviceCache *serviceCache @@ -140,7 +141,7 @@ type ServiceController struct { // (like Kubernetes Services and DNS server records for service discovery) in sync with the registry. func New(federationClient fedclientset.Interface, dns dnsprovider.Interface, - federationName, serviceDnsSuffix, zoneName string) *ServiceController { + federationName, serviceDnsSuffix, zoneName string, zoneID string) *ServiceController { broadcaster := record.NewBroadcaster() // federationClient event is not supported yet // broadcaster.StartRecordingToSink(&unversioned_core.EventSinkImpl{Interface: kubeClient.Core().Events("")}) @@ -152,6 +153,7 @@ func New(federationClient fedclientset.Interface, dns dnsprovider.Interface, federationName: federationName, serviceDnsSuffix: serviceDnsSuffix, zoneName: zoneName, + zoneID: zoneID, serviceCache: &serviceCache{fedServiceMap: make(map[string]*cachedService)}, clusterCache: &clusterClientCache{ rwlock: sync.Mutex{}, @@ -279,8 +281,8 @@ func (s *ServiceController) init() error { if s.federationName == "" { return fmt.Errorf("ServiceController should not be run without federationName.") } - if s.zoneName == "" { - return fmt.Errorf("ServiceController should not be run without zoneName.") + if s.zoneName == "" && s.zoneID == "" { + return fmt.Errorf("ServiceController must be run with either zoneName or zoneID.") } if s.serviceDnsSuffix == "" { // TODO: Is this the right place to do defaulting? @@ -297,7 +299,14 @@ func (s *ServiceController) init() error { return fmt.Errorf("the dns provider does not support zone enumeration, which is required for creating dns records.") } s.dnsZones = zones - if _, err := getDnsZone(s.zoneName, s.dnsZones); err != nil { + matchingZones, err := getDnsZones(s.zoneName, s.zoneID, s.dnsZones) + if err != nil { + return fmt.Errorf("error querying for DNS zones: %v", err) + } + if len(matchingZones) == 0 { + if s.zoneName == "" { + return fmt.Errorf("ServiceController must be run with zoneName to create zone automatically.") + } glog.Infof("DNS zone %q not found. Creating DNS zone %q.", s.zoneName, s.zoneName) managedZone, err := s.dnsZones.New(s.zoneName) if err != nil { @@ -310,6 +319,9 @@ func (s *ServiceController) init() error { glog.Infof("DNS zone %q successfully created. Note that DNS resolution will not work until you have registered this name with "+ "a DNS registrar and they have changed the authoritative name servers for your domain to point to your DNS provider.", zone.Name()) } + if len(matchingZones) > 1 { + return fmt.Errorf("Multiple matching DNS zones found for %q; please specify zoneID", s.zoneName) + } return nil } diff --git a/hack/verify-flags/known-flags.txt b/hack/verify-flags/known-flags.txt index 3bd944cab3a..1424433b8f3 100644 --- a/hack/verify-flags/known-flags.txt +++ b/hack/verify-flags/known-flags.txt @@ -592,6 +592,7 @@ watch-only whitelist-override-label windows-line-endings www-prefix +zone-id zone-name garbage-collector-enabled viper-config