From 038f44d33c7a5f015552fe8e6e9ac76e8bc44290 Mon Sep 17 00:00:00 2001 From: shashidharatd Date: Tue, 23 Oct 2018 19:56:19 +0530 Subject: [PATCH] Refactored and implement new features for ServiceDNSRecord --- pkg/controller/dnsendpoint/service.go | 21 +++++-- pkg/controller/dnsendpoint/service_test.go | 67 ++++++++++++++++------ pkg/controller/servicedns/controller.go | 44 +++++++++++++- test/common/dns.go | 9 +++ test/e2e/servicedns.go | 21 +++++-- test/integration/servicedns_test.go | 20 ++++++- 6 files changed, 152 insertions(+), 30 deletions(-) diff --git a/pkg/controller/dnsendpoint/service.go b/pkg/controller/dnsendpoint/service.go index c81b623e8b..fcc63d2b03 100644 --- a/pkg/controller/dnsendpoint/service.go +++ b/pkg/controller/dnsendpoint/service.go @@ -59,15 +59,15 @@ func getServiceDNSEndpoints(obj interface{}) ([]*feddnsv1a1.Endpoint, error) { return nil, fmt.Errorf("received event for unknown object %v", obj) } - commonPrefix := strings.Join([]string{dnsObject.Name, dnsObject.Namespace, dnsObject.Spec.FederationName, "svc"}, ".") + commonPrefix := strings.Join([]string{dnsObject.Name, dnsObject.Namespace, dnsObject.Spec.Federation, "svc"}, ".") for _, clusterDNS := range dnsObject.Status.DNS { zone := clusterDNS.Zone region := clusterDNS.Region dnsNames := []string{ - strings.Join([]string{commonPrefix, zone, region, dnsObject.Spec.DNSSuffix}, "."), // zone level - strings.Join([]string{commonPrefix, region, dnsObject.Spec.DNSSuffix}, "."), // region level, one up from zone level - strings.Join([]string{commonPrefix, dnsObject.Spec.DNSSuffix}, "."), // global level, one up from region level + strings.Join([]string{commonPrefix, zone, region, dnsObject.Status.Domain}, "."), // zone level + strings.Join([]string{commonPrefix, region, dnsObject.Status.Domain}, "."), // region level, one up from zone level + strings.Join([]string{commonPrefix, dnsObject.Status.Domain}, "."), // global level, one up from region level "", // nowhere to go up from global level } @@ -101,6 +101,19 @@ func getServiceDNSEndpoints(obj interface{}) ([]*feddnsv1a1.Endpoint, error) { } endpoints = append(endpoints, endpoint) } + if dnsObject.Spec.DNSPrefix != "" && len(globalTargets) > 0 { + endpoint := &feddnsv1a1.Endpoint{ + DNSName: dnsObject.Spec.DNSPrefix + "." + dnsObject.Status.Domain, + RecordTTL: ttl, + RecordType: RecordTypeA, + } + targets, err := getResolvedTargets(globalTargets, netWrapper) + if err != nil { + return nil, err + } + endpoint.Targets = targets + endpoints = append(endpoints, endpoint) + } } return DedupeAndMergeEndpoints(endpoints), nil diff --git a/pkg/controller/dnsendpoint/service_test.go b/pkg/controller/dnsendpoint/service_test.go index 39c95d24a6..c5f4696044 100644 --- a/pkg/controller/dnsendpoint/service_test.go +++ b/pkg/controller/dnsendpoint/service_test.go @@ -59,10 +59,11 @@ func TestGetEndpointsForServiceDNSObject(t *testing.T) { "NoClusters": { dnsObject: feddnsv1a1.ServiceDNSRecord{ Spec: feddnsv1a1.ServiceDNSRecordSpec{ - FederationName: federation, - DNSSuffix: dnsZone, + Federation: federation, + }, + Status: feddnsv1a1.ServiceDNSRecordStatus{ + Domain: dnsZone, }, - Status: feddnsv1a1.ServiceDNSRecordStatus{}, }, expectEndpoints: nil, expectError: false, @@ -74,10 +75,10 @@ func TestGetEndpointsForServiceDNSObject(t *testing.T) { Namespace: namespace, }, Spec: feddnsv1a1.ServiceDNSRecordSpec{ - FederationName: federation, - DNSSuffix: dnsZone, + Federation: federation, }, Status: feddnsv1a1.ServiceDNSRecordStatus{ + Domain: dnsZone, DNS: []feddnsv1a1.ClusterDNS{ { Cluster: c1, Zone: c1Zone, Region: c1Region, @@ -100,10 +101,10 @@ func TestGetEndpointsForServiceDNSObject(t *testing.T) { Namespace: namespace, }, Spec: feddnsv1a1.ServiceDNSRecordSpec{ - FederationName: federation, - DNSSuffix: dnsZone, + Federation: federation, }, Status: feddnsv1a1.ServiceDNSRecordStatus{ + Domain: dnsZone, DNS: []feddnsv1a1.ClusterDNS{ { Cluster: c1, Zone: c1Zone, Region: c1Region, @@ -132,10 +133,10 @@ func TestGetEndpointsForServiceDNSObject(t *testing.T) { Namespace: namespace, }, Spec: feddnsv1a1.ServiceDNSRecordSpec{ - FederationName: federation, - DNSSuffix: dnsZone, + Federation: federation, }, Status: feddnsv1a1.ServiceDNSRecordStatus{ + Domain: dnsZone, DNS: []feddnsv1a1.ClusterDNS{ { Cluster: c1, Zone: c1Zone, Region: c1Region, @@ -163,10 +164,10 @@ func TestGetEndpointsForServiceDNSObject(t *testing.T) { Namespace: namespace, }, Spec: feddnsv1a1.ServiceDNSRecordSpec{ - FederationName: federation, - DNSSuffix: dnsZone, + Federation: federation, }, Status: feddnsv1a1.ServiceDNSRecordStatus{ + Domain: dnsZone, DNS: []feddnsv1a1.ClusterDNS{ { Cluster: c1, Zone: c1Zone, Region: c1Region, @@ -192,10 +193,10 @@ func TestGetEndpointsForServiceDNSObject(t *testing.T) { Namespace: namespace, }, Spec: feddnsv1a1.ServiceDNSRecordSpec{ - FederationName: federation, - DNSSuffix: dnsZone, + Federation: federation, }, Status: feddnsv1a1.ServiceDNSRecordStatus{ + Domain: dnsZone, DNS: []feddnsv1a1.ClusterDNS{ { Cluster: c1, Zone: c1Zone, Region: c1Region, @@ -224,11 +225,11 @@ func TestGetEndpointsForServiceDNSObject(t *testing.T) { Namespace: namespace, }, Spec: feddnsv1a1.ServiceDNSRecordSpec{ - FederationName: federation, - DNSSuffix: dnsZone, - RecordTTL: userConfiguredTTL, + Federation: federation, + RecordTTL: userConfiguredTTL, }, Status: feddnsv1a1.ServiceDNSRecordStatus{ + Domain: dnsZone, DNS: []feddnsv1a1.ClusterDNS{ { Cluster: c1, Zone: c1Zone, Region: c1Region, @@ -244,6 +245,40 @@ func TestGetEndpointsForServiceDNSObject(t *testing.T) { }, expectError: false, }, + "UserConfiguredDNSPrefix": { + dnsObject: feddnsv1a1.ServiceDNSRecord{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: namespace, + }, + Spec: feddnsv1a1.ServiceDNSRecordSpec{ + Federation: federation, + DNSPrefix: "foo", + }, + Status: feddnsv1a1.ServiceDNSRecordStatus{ + Domain: dnsZone, + DNS: []feddnsv1a1.ClusterDNS{ + { + Cluster: c1, Zone: c1Zone, Region: c1Region, + LoadBalancer: v1.LoadBalancerStatus{Ingress: []v1.LoadBalancerIngress{{IP: lb1}}}, + }, + { + Cluster: c2, Zone: c2Zone, Region: c2Region, + LoadBalancer: v1.LoadBalancerStatus{Ingress: []v1.LoadBalancerIngress{{IP: lb2}}}, + }, + }, + }, + }, + expectEndpoints: []*feddnsv1a1.Endpoint{ + {DNSName: globalDNSName, Targets: []string{lb1, lb2}, RecordType: RecordTypeA, RecordTTL: defaultDNSTTL}, + {DNSName: c1RegionDNSName, Targets: []string{lb1}, RecordType: RecordTypeA, RecordTTL: defaultDNSTTL}, + {DNSName: c1ZoneDNSName, Targets: []string{lb1}, RecordType: RecordTypeA, RecordTTL: defaultDNSTTL}, + {DNSName: c2RegionDNSName, Targets: []string{lb2}, RecordType: RecordTypeA, RecordTTL: defaultDNSTTL}, + {DNSName: c2ZoneDNSName, Targets: []string{lb2}, RecordType: RecordTypeA, RecordTTL: defaultDNSTTL}, + {DNSName: "foo" + "." + dnsZone, Targets: []string{lb1, lb2}, RecordType: RecordTypeA, RecordTTL: defaultDNSTTL}, + }, + expectError: false, + }, } for testName, tc := range testCases { diff --git a/pkg/controller/servicedns/controller.go b/pkg/controller/servicedns/controller.go index e47332ccab..9800f22bb7 100644 --- a/pkg/controller/servicedns/controller.go +++ b/pkg/controller/servicedns/controller.go @@ -65,11 +65,18 @@ type Controller struct { // Informer for the ServiceDNSRecord objects serviceDNSController cache.Controller + // Store for the FederationDomain objects + federationDomainStore cache.Store + // Informer for the FederationDomain objects + federationDomainController cache.Controller + worker util.ReconcileWorker clusterAvailableDelay time.Duration clusterUnavailableDelay time.Duration smallDelay time.Duration + + fedNamespace string } // StartController starts the Controller for managing ServiceDNSRecord objects. @@ -99,6 +106,7 @@ func newController(fedClient fedclientset.Interface, kubeClient kubeclientset.In clusterAvailableDelay: time.Second * 20, clusterUnavailableDelay: time.Second * 60, smallDelay: time.Second * 3, + fedNamespace: fedNamespace, } s.worker = util.NewReconcileWorker(s.reconcile, util.WorkerTiming{ @@ -123,6 +131,23 @@ func newController(fedClient fedclientset.Interface, kubeClient kubeclientset.In util.NewTriggerOnAllChanges(s.worker.EnqueueObject), ) + // Informer for the FederationDomain resource + s.federationDomainStore, s.federationDomainController = cache.NewInformer( + &cache.ListWatch{ + ListFunc: func(options metav1.ListOptions) (pkgruntime.Object, error) { + return fedClient.MulticlusterdnsV1alpha1().Domains(fedNamespace).List(options) + }, + WatchFunc: func(options metav1.ListOptions) (watch.Interface, error) { + return fedClient.MulticlusterdnsV1alpha1().Domains(fedNamespace).Watch(options) + }, + }, + &dnsv1a1.Domain{}, + util.NoResyncPeriod, + util.NewTriggerOnAllChanges(func(pkgruntime.Object) { + s.clusterDeliverer.DeliverAt(allClustersKey, nil, time.Now()) + }), + ) + // Federated serviceInformer for the service resource in members of federation. s.serviceInformer = util.NewFederatedInformer( fedClient, @@ -185,6 +210,7 @@ func (c *Controller) minimizeLatency() { // Run runs the Controller. func (c *Controller) Run(stopChan <-chan struct{}) { go c.serviceDNSController.Run(stopChan) + go c.federationDomainController.Run(stopChan) c.serviceInformer.Start() c.endpointInformer.Start() c.clusterDeliverer.StartWithHandler(func(_ *util.DelayingDelivererItem) { @@ -266,6 +292,17 @@ func (c *Controller) reconcile(qualifiedName util.QualifiedName) util.Reconcilia } cachedDNS := cachedObj.(*dnsv1a1.ServiceDNSRecord) + domainKey := util.QualifiedName{Namespace: c.fedNamespace, Name: cachedDNS.Spec.Federation}.String() + cachedDomain, exist, err := c.federationDomainStore.GetByKey(domainKey) + if err != nil { + runtime.HandleError(fmt.Errorf("Failed to query FederationDomain store for %q: %v", domainKey, err)) + return util.StatusError + } + if !exist { + return util.StatusAllOK + } + domainObj := cachedDomain.(*dnsv1a1.Domain) + fedDNS := &dnsv1a1.ServiceDNSRecord{ ObjectMeta: util.DeepCopyRelevantObjectMeta(cachedDNS.ObjectMeta), Spec: *cachedDNS.Spec.DeepCopy(), @@ -287,13 +324,13 @@ func (c *Controller) reconcile(qualifiedName util.QualifiedName) util.Reconcilia } // If there are no endpoints for the service, the service is not backed by pods - // and traffic is not routable to the service. - // We avoid such service shards while writing DNS records. + // and traffic is not routable to the service. We avoid such service shards while + // writing DNS records, except when user specified to AllowServiceWithoutEndpoints endpointsExist, err := c.serviceBackedByEndpointsInCluster(cluster.Name, key) if err != nil { return util.StatusError } - if endpointsExist { + if cachedDNS.Spec.AllowServiceWithoutEndpoints || endpointsExist { lbStatus, err := c.getServiceStatusInCluster(cluster.Name, key) if err != nil { return util.StatusError @@ -328,6 +365,7 @@ func (c *Controller) reconcile(qualifiedName util.QualifiedName) util.Reconcilia return fedDNSStatus[i].Cluster < fedDNSStatus[j].Cluster }) fedDNS.Status.DNS = fedDNSStatus + fedDNS.Status.Domain = domainObj.Domain if !reflect.DeepEqual(cachedDNS.Status, fedDNS.Status) { _, err = c.fedClient.MulticlusterdnsV1alpha1().ServiceDNSRecords(fedDNS.Namespace).UpdateStatus(fedDNS) diff --git a/test/common/dns.go b/test/common/dns.go index 68281845ed..3efd5479f3 100644 --- a/test/common/dns.go +++ b/test/common/dns.go @@ -32,6 +32,15 @@ import ( "github.com/kubernetes-sigs/federation-v2/pkg/controller/util" ) +func NewFederationDomainObject(federation, domain string) *dnsv1a1.Domain { + return &dnsv1a1.Domain{ + ObjectMeta: metav1.ObjectMeta{ + Name: federation, + }, + Domain: domain, + } +} + func NewServiceDNSObject(baseName, namespace string) *dnsv1a1.ServiceDNSRecord { return &dnsv1a1.ServiceDNSRecord{ ObjectMeta: metav1.ObjectMeta{ diff --git a/test/e2e/servicedns.go b/test/e2e/servicedns.go index 7c359b562a..68265adbf5 100644 --- a/test/e2e/servicedns.go +++ b/test/e2e/servicedns.go @@ -42,10 +42,13 @@ var _ = Describe("ServiceDNS", func() { const userAgent = "test-service-dns" const baseName = "test-service-dns-" + const federation = "galactic" + const dnsZone = "dzone.io" var fedClient fedclientset.Interface var clusterRegionZones map[string]fedv1a1.FederatedClusterStatus var namespace string + var domainClient dnsv1a1client.DomainInterface var dnsClient dnsv1a1client.ServiceDNSRecordInterface objectGetter := func(namespace, name string) (pkgruntime.Object, error) { @@ -56,6 +59,7 @@ var _ = Describe("ServiceDNS", func() { BeforeEach(func() { fedClient = f.FedClient(userAgent) namespace = f.TestNamespaceName() + domainClient = fedClient.MulticlusterdnsV1alpha1().Domains(f.FederationSystemNamespace()) dnsClient = fedClient.MulticlusterdnsV1alpha1().ServiceDNSRecords(namespace) federatedClusters, err := fedClient.CoreV1alpha1().FederatedClusters(f.FederationSystemNamespace()).List(metav1.ListOptions{}) @@ -68,15 +72,23 @@ var _ = Describe("ServiceDNS", func() { } } f.SetUpServiceDNSControllerFixture() + domainObj := common.NewFederationDomainObject(federation, dnsZone) + _, err = domainClient.Create(domainObj) + framework.ExpectNoError(err, "Error creating FederationDomain object") + }) + + AfterEach(func() { + domainClient.Delete(federation, &metav1.DeleteOptions{}) }) It("ServiceDNS object status should be updated correctly when there are no service and/or endpoint in member clusters", func() { By("Creating the ServiceDNS object") serviceDNSObj := common.NewServiceDNSObject(baseName, namespace) + serviceDNSObj.Spec.Federation = federation serviceDNS, err := dnsClient.Create(serviceDNSObj) framework.ExpectNoError(err, "Error creating ServiceDNS object: %v", serviceDNS) - serviceDNSStatus := dnsv1a1.ServiceDNSRecordStatus{DNS: []dnsv1a1.ClusterDNS{}} + serviceDNSStatus := dnsv1a1.ServiceDNSRecordStatus{Domain: dnsZone, DNS: []dnsv1a1.ClusterDNS{}} for _, clusterName := range f.ClusterNames(userAgent) { serviceDNSStatus.DNS = append(serviceDNSStatus.DNS, dnsv1a1.ClusterDNS{ Cluster: clusterName, @@ -98,20 +110,17 @@ var _ = Describe("ServiceDNS", func() { RecordTypeA = "A" RecordTTL = 300 ) - federation := "galactic" - dnsZone := "dzone.io" It("DNSEndpoint object should be created with correct status when ServiceDNS object is created", func() { By("Creating the ServiceDNS object") serviceDNSObj := common.NewServiceDNSObject(baseName, namespace) - serviceDNSObj.Spec.FederationName = federation - serviceDNSObj.Spec.DNSSuffix = dnsZone + serviceDNSObj.Spec.Federation = federation serviceDNSObj.Spec.RecordTTL = RecordTTL serviceDNS, err := dnsClient.Create(serviceDNSObj) framework.ExpectNoError(err, "Error creating ServiceDNS object %v", serviceDNS) name := serviceDNS.Name - serviceDNSStatus := &dnsv1a1.ServiceDNSRecordStatus{DNS: []dnsv1a1.ClusterDNS{}} + serviceDNSStatus := &dnsv1a1.ServiceDNSRecordStatus{Domain: dnsZone, DNS: []dnsv1a1.ClusterDNS{}} By("Creating corresponding service and endpoint for the ServiceDNS object in member clusters") serviceDNSStatus = createClusterServiceAndEndpoints(f, name, namespace, serviceDNSStatus) diff --git a/test/integration/servicedns_test.go b/test/integration/servicedns_test.go index 2f175ff9ea..b01193b266 100644 --- a/test/integration/servicedns_test.go +++ b/test/integration/servicedns_test.go @@ -68,6 +68,9 @@ var TestServiceDNS = func(t *testing.T) { }, } + const federation = "galactic" + const dnsZone = "dzone.io" + for testName, tc := range testCases { t.Run(testName, func(t *testing.T) { tl := framework.NewIntegrationLogger(t) @@ -78,14 +81,23 @@ var TestServiceDNS = func(t *testing.T) { return fixture.client.MulticlusterdnsV1alpha1().ServiceDNSRecords(namespace).Get(name, metav1.GetOptions{}) } + domain := common.NewFederationDomainObject(federation, dnsZone) + tl.Logf("Create FederationDomain Object: %s", federation) + _, err := fixture.client.MulticlusterdnsV1alpha1().Domains(FedFixture.SystemNamespace).Create(domain) + if err != nil { + tl.Fatalf("Error creating FederationDomain object %q: %v", federation, err) + } + tl.Logf("Create FederationDomain object success: %s", federation) + serviceDNS := common.NewServiceDNSObject(tc.name, namespace) + serviceDNS.Spec.Federation = federation tl.Logf("Create serviceDNSObj: %s", key) serviceDNSObj, err := fixture.client.MulticlusterdnsV1alpha1().ServiceDNSRecords(namespace).Create(serviceDNS) - name := serviceDNSObj.Name if err != nil { tl.Fatalf("Error creating ServiceDNS object %q: %v", key, err) } tl.Logf("Create ServiceDNS object success: %s", key) + name := serviceDNSObj.Name for clusterName, clusterClient := range fixture.clusterClients { if tc.createService { @@ -116,10 +128,16 @@ var TestServiceDNS = func(t *testing.T) { } serviceDNSObj.Status.DNS = tc.desiredDNSStatus + serviceDNSObj.Status.Domain = dnsZone tl.Logf("Wait for ServiceDNS object status update") common.WaitForObject(tl, namespace, name, objectGetter, serviceDNSObj, framework.DefaultWaitInterval, wait.ForeverTestTimeout) tl.Logf("ServiceDNS object status is updated successfully: %s", key) + + err = fixture.client.MulticlusterdnsV1alpha1().Domains(FedFixture.SystemNamespace).Delete(federation, &metav1.DeleteOptions{}) + if err != nil { + tl.Fatalf("Error deleting FederationDomain object %q: %v", federation, err) + } }) } }