From f3849df93b397de3d94332616b1b34284bfee86d Mon Sep 17 00:00:00 2001 From: Maskym Vavilov Date: Thu, 14 Sep 2023 17:13:51 +0100 Subject: [PATCH] adressing comments p2 --- .../dnspolicy/dnspolicy_dnsrecords.go | 16 +-- test/integration/dnspolicy_controller_test.go | 108 +++++++++++++----- 2 files changed, 87 insertions(+), 37 deletions(-) diff --git a/pkg/controllers/dnspolicy/dnspolicy_dnsrecords.go b/pkg/controllers/dnspolicy/dnspolicy_dnsrecords.go index 7b76bab25..e5e8b0876 100644 --- a/pkg/controllers/dnspolicy/dnspolicy_dnsrecords.go +++ b/pkg/controllers/dnspolicy/dnspolicy_dnsrecords.go @@ -138,21 +138,26 @@ func (r *DNSPolicyReconciler) deleteGatewayDNSRecords(ctx context.Context, gatew func (r *DNSPolicyReconciler) buildClusterGateway(ctx context.Context, downstreamClusterName string, clusterAddress []gatewayv1beta1.GatewayAddress) (dns.ClusterGateway, error) { var target dns.ClusterGateway + singleClusterAddresses := make([]gatewayv1beta1.GatewayAddress, len(clusterAddress)) mc := &clusterv1.ManagedCluster{} if err := r.Client().Get(ctx, client.ObjectKey{Name: downstreamClusterName}, mc, &client.GetOptions{}); err != nil { return target, err } - for _, addr := range clusterAddress { + for i, addr := range clusterAddress { addrType := gatewayv1beta1.IPAddressType if *addr.Type == gateway.MultiClusterHostnameAddressType { addrType = gatewayv1beta1.HostnameAddressType } - addr.Type = &addrType + + singleClusterAddresses[i] = gatewayv1beta1.GatewayAddress{ + Type: &addrType, + Value: addr.Value, + } } - target = *dns.NewClusterGateway(mc, clusterAddress) + target = *dns.NewClusterGateway(mc, singleClusterAddresses) return target, nil } @@ -163,11 +168,6 @@ func getClusterGatewayAddresses(gw *gatewayv1beta1.Gateway) map[string][]gateway for _, address := range gw.Status.Addresses { var gatewayAddresses []gatewayv1beta1.GatewayAddress - //addressType := gatewayv1beta1.IPAddressType - //if *address.Type == gateway.MultiClusterHostnameAddressType { - // addressType = gatewayv1beta1.HostnameAddressType - //} - //Default to Single Cluster (Normal Gateway Status) cluster := "none" addressValue := address.Value diff --git a/test/integration/dnspolicy_controller_test.go b/test/integration/dnspolicy_controller_test.go index 8f9ed5131..77a8d4e14 100644 --- a/test/integration/dnspolicy_controller_test.go +++ b/test/integration/dnspolicy_controller_test.go @@ -362,7 +362,13 @@ var _ = Describe("DNSPolicy", Ordered, func() { }, } Eventually(func() error { // DNS record exists - return k8sClient.Get(ctx, client.ObjectKey{Name: dnsRecordName, Namespace: testNamespace}, createdDNSRecord) + if err := k8sClient.Get(ctx, client.ObjectKey{Name: dnsRecordName, Namespace: testNamespace}, createdDNSRecord); err != nil { + return err + } + if len(createdDNSRecord.Spec.Endpoints) != len(expectedEndpoints) { + return fmt.Errorf("expected %v endpoints in DNSRecord, got %v", len(expectedEndpoints), len(createdDNSRecord.Spec.Endpoints)) + } + return nil }, TestTimeoutMedium, TestRetryIntervalMedium).Should(BeNil()) Expect(createdDNSRecord.Spec.ManagedZoneRef.Name).To(Equal("example.com")) Expect(createdDNSRecord.Spec.Endpoints).To(HaveLen(len(expectedEndpoints))) @@ -445,7 +451,13 @@ var _ = Describe("DNSPolicy", Ordered, func() { }, } Eventually(func() error { // DNS record exists - return k8sClient.Get(ctx, client.ObjectKey{Name: wildcardDNSRecordName, Namespace: testNamespace}, wildcardDNSRecord) + if err := k8sClient.Get(ctx, client.ObjectKey{Name: wildcardDNSRecordName, Namespace: testNamespace}, wildcardDNSRecord); err != nil { + return err + } + if len(wildcardDNSRecord.Spec.Endpoints) != len(expectedEndpoints) { + return fmt.Errorf("expected %v wildcard endpoints in DNSRecord, got %v", len(expectedEndpoints), len(wildcardDNSRecord.Spec.Endpoints)) + } + return nil }, TestTimeoutMedium, TestRetryIntervalMedium).Should(BeNil()) Expect(wildcardDNSRecord.Spec.ManagedZoneRef.Name).To(Equal("example.com")) Expect(wildcardDNSRecord.Spec.Endpoints).To(HaveLen(len(expectedEndpoints))) @@ -698,7 +710,13 @@ var _ = Describe("DNSPolicy", Ordered, func() { }, } Eventually(func() error { // DNS record exists - return k8sClient.Get(ctx, client.ObjectKey{Name: dnsRecordName, Namespace: dnsPolicy.Namespace}, createdDNSRecord) + if err := k8sClient.Get(ctx, client.ObjectKey{Name: dnsRecordName, Namespace: dnsPolicy.Namespace}, createdDNSRecord); err != nil { + return err + } + if len(createdDNSRecord.Spec.Endpoints) != len(expectedEndpoints) { + return fmt.Errorf("expected %v endpoints in DNSRecord, got %v", len(expectedEndpoints), len(createdDNSRecord.Spec.Endpoints)) + } + return nil }, TestTimeoutMedium, TestRetryIntervalMedium).Should(BeNil()) Expect(createdDNSRecord.Spec.ManagedZoneRef.Name).To(Equal("example.com")) Expect(createdDNSRecord.Spec.Endpoints).To(HaveLen(len(expectedEndpoints))) @@ -709,7 +727,7 @@ var _ = Describe("DNSPolicy", Ordered, func() { It("should create a wildcard dns record", func() { wildcardDNSRecord := &v1alpha1.DNSRecord{} - _ = []*v1alpha1.Endpoint{ + expectedEndpoints := []*v1alpha1.Endpoint{ { DNSName: "*.example.com", Targets: []string{ @@ -800,12 +818,18 @@ var _ = Describe("DNSPolicy", Ordered, func() { }, } Eventually(func() error { // DNS record exists - return k8sClient.Get(ctx, client.ObjectKey{Name: wildcardDNSRecordName, Namespace: dnsPolicy.Namespace}, wildcardDNSRecord) + if err := k8sClient.Get(ctx, client.ObjectKey{Name: wildcardDNSRecordName, Namespace: dnsPolicy.Namespace}, wildcardDNSRecord); err != nil { + return err + } + if len(wildcardDNSRecord.Spec.Endpoints) != len(expectedEndpoints) { + return fmt.Errorf("expected %v endpoints in DNSRecord, got %v", len(expectedEndpoints), len(wildcardDNSRecord.Spec.Endpoints)) + } + return nil }, TestTimeoutMedium, TestRetryIntervalMedium).Should(BeNil()) Expect(wildcardDNSRecord.Spec.ManagedZoneRef.Name).To(Equal("example.com")) - //Expect(wildcardDNSRecord.Spec.Endpoints).To(HaveLen(len(expectedEndpoints))) - //Expect(wildcardDNSRecord.Spec.Endpoints).Should(ContainElements(expectedEndpoints)) - //Expect(expectedEndpoints).Should(ContainElements(wildcardDNSRecord.Spec.Endpoints)) + Expect(wildcardDNSRecord.Spec.Endpoints).To(HaveLen(len(expectedEndpoints))) + Expect(wildcardDNSRecord.Spec.Endpoints).Should(ContainElements(expectedEndpoints)) + Expect(expectedEndpoints).Should(ContainElements(wildcardDNSRecord.Spec.Endpoints)) }) }) Context("probes status impact DNS records", func() { @@ -823,16 +847,26 @@ var _ = Describe("DNSPolicy", Ordered, func() { It("should create a dns record", func() { createdDNSRecord := &v1alpha1.DNSRecord{} Eventually(func() error { // DNS record exists - return k8sClient.Get(ctx, client.ObjectKey{Name: dnsRecordName, Namespace: testNamespace}, createdDNSRecord) + if err := k8sClient.Get(ctx, client.ObjectKey{Name: dnsRecordName, Namespace: testNamespace}, createdDNSRecord); err != nil { + return err + } + if len(createdDNSRecord.Spec.Endpoints) != 6 { + return fmt.Errorf("expected %v endpoints in DNSRecord, got %v", 6, len(createdDNSRecord.Spec.Endpoints)) + } + return nil }, TestTimeoutMedium, TestRetryIntervalMedium).Should(BeNil()) Expect(createdDNSRecord.Spec.Endpoints).To(HaveLen(6)) }) It("should have probes that are healthy", func() { probeList := &v1alpha1.DNSHealthCheckProbeList{} Eventually(func() error { - return k8sClient.List(ctx, probeList, &client.ListOptions{Namespace: testNamespace}) + Expect(k8sClient.List(ctx, probeList, &client.ListOptions{Namespace: testNamespace})).To(BeNil()) + if len(probeList.Items) != 2 { + return fmt.Errorf("expected %v probes, got %v", 2, len(probeList.Items)) + } + return nil }, TestTimeoutMedium, TestRetryIntervalMedium).Should(BeNil()) - Expect(probeList.Items).To(HaveLen(2)) + Expect(len(probeList.Items)).To(Equal(2)) }) Context("all unhealthy probes", func() { @@ -953,6 +987,9 @@ var _ = Describe("DNSPolicy", Ordered, func() { if err != nil && k8serrors.IsNotFound(err) { return err } + if len(createdDNSRecord.Spec.Endpoints) != len(expectedEndpoints) { + return fmt.Errorf("expected %v endpoints in DNSRecord, got %v", len(expectedEndpoints), len(createdDNSRecord.Spec.Endpoints)) + } return nil }, TestTimeoutLong, TestRetryIntervalMedium).Should(BeNil()) Expect(createdDNSRecord.Spec.Endpoints).To(HaveLen(len(expectedEndpoints))) @@ -975,6 +1012,15 @@ var _ = Describe("DNSPolicy", Ordered, func() { SetIdentifier: "", RecordTTL: 60, }, + { + DNSName: "s07c46.lb-" + lbHash + ".test.example.com", + Targets: []string{ + TestAttachedRouteAddressOne, + }, + RecordType: "A", + SetIdentifier: "", + RecordTTL: 60, + }, { DNSName: "default.lb-" + lbHash + ".test.example.com", Targets: []string{ @@ -990,6 +1036,22 @@ var _ = Describe("DNSPolicy", Ordered, func() { }, }, }, + { + DNSName: "default.lb-" + lbHash + ".test.example.com", + Targets: []string{ + "s07c46.lb-" + lbHash + ".test.example.com", + }, + RecordType: "CNAME", + SetIdentifier: "s07c46.lb-" + lbHash + ".test.example.com", + RecordTTL: 60, + Labels: nil, + ProviderSpecific: v1alpha1.ProviderSpecific{ + { + Name: "weight", + Value: "120", + }, + }, + }, { DNSName: "lb-" + lbHash + ".test.example.com", Targets: []string{ @@ -1018,13 +1080,17 @@ var _ = Describe("DNSPolicy", Ordered, func() { probeList := &v1alpha1.DNSHealthCheckProbeList{} Eventually(func() error { - return k8sClient.List(ctx, probeList, &client.ListOptions{Namespace: testNamespace}) + Expect(k8sClient.List(ctx, probeList, &client.ListOptions{Namespace: testNamespace})).To(BeNil()) + if len(probeList.Items) != 2 { + return fmt.Errorf("expected %v probes, got %v", 2, len(probeList.Items)) + } + return nil }, TestTimeoutLong, TestRetryIntervalMedium).Should(BeNil()) Expect(probeList.Items).To(HaveLen(2)) Eventually(func() error { getProbe := &v1alpha1.DNSHealthCheckProbe{} - if err := k8sClient.Get(ctx, client.ObjectKey{Name: fmt.Sprintf("%s-%s-%s", TestAttachedRouteAddressOne, TestPlacedGatewayName, "api"), Namespace: testNamespace}, getProbe); err != nil { + if err := k8sClient.Get(ctx, client.ObjectKey{Name: fmt.Sprintf("%s-%s-%s", TestAttachedRouteAddressOne, TestPlacedGatewayName, TestAttachedRouteName), Namespace: testNamespace}, getProbe); err != nil { return err } patch := client.MergeFrom(getProbe.DeepCopy()) @@ -1136,22 +1202,6 @@ var _ = Describe("DNSPolicy", Ordered, func() { err := k8sClient.Get(ctx, client.ObjectKey{Name: gateway.Name, Namespace: gateway.Namespace}, gateway) Expect(err).NotTo(HaveOccurred()) - patch := client.MergeFrom(gateway.DeepCopy()) - addressType := mgcgateway.MultiClusterIPAddressType - gateway.Status.Addresses = []gatewayv1beta1.GatewayAddress{ - { - Type: &addressType, - Value: fmt.Sprintf("%s/%s", "kind-mgc-control-plane", TestAttachedRouteAddressOne), - }, - { - Type: &addressType, - Value: fmt.Sprintf("%s/%s", "kind-mgc-control-plane", TestAttachedRouteAddressTwo), - }, - } - Expect(k8sClient.Status().Patch(ctx, gateway, patch)).To(BeNil()) - - err = k8sClient.Get(ctx, client.ObjectKey{Name: gateway.Name, Namespace: gateway.Namespace}, gateway) - Expect(err).NotTo(HaveOccurred()) Expect(gateway.Spec.Listeners).NotTo(BeNil()) // add another listener, should result in 4 probes typedHostname := gatewayv1beta1.Hostname(OtherAttachedRouteName)