From 3101dbc0d1c96da8b0bca5ab6ec4f073b28222cd Mon Sep 17 00:00:00 2001 From: candita Date: Fri, 19 Jul 2024 15:21:30 -0400 Subject: [PATCH] OCPBUGS-37220: Fix DNS for Gateway API on AWS --- pkg/dns/aws/dns.go | 4 +++- pkg/operator/controller/dns/controller.go | 8 +++++++ .../gateway-service-dns/controller.go | 23 +++++++++++++------ pkg/resources/dnsrecord/dns.go | 5 ++++ 4 files changed, 32 insertions(+), 8 deletions(-) diff --git a/pkg/dns/aws/dns.go b/pkg/dns/aws/dns.go index 84d017517e..9a4ab321ff 100644 --- a/pkg/dns/aws/dns.go +++ b/pkg/dns/aws/dns.go @@ -575,6 +575,8 @@ func (m *Provider) change(record *iov1.DNSRecord, zone configv1.DNSZone, action log.Info("upserted DNS record", "record", record.Spec, "zone", zone) case deleteAction: log.Info("deleted DNS record", "record", record.Spec, "zone", zone) + default: + log.Info("unrecognized action verb", "action", string(action)) } return nil } @@ -631,7 +633,7 @@ func (m *Provider) updateRecord(domain, zoneID, target, targetHostedZoneID, acti } return fmt.Errorf("couldn't update DNS record in zone %s: %v", zoneID, err) } - log.Info("updated DNS record", "zone id", zoneID, "domain", domain, "target", target, "response", resp) + log.Info("upserted DNS record", "zone id", zoneID, "domain", domain, "target", target, "response", resp) return nil } diff --git a/pkg/operator/controller/dns/controller.go b/pkg/operator/controller/dns/controller.go index 25c55dbd0e..1ac8d16f56 100644 --- a/pkg/operator/controller/dns/controller.go +++ b/pkg/operator/controller/dns/controller.go @@ -26,6 +26,7 @@ import ( logf "github.com/openshift/cluster-ingress-operator/pkg/log" "github.com/openshift/cluster-ingress-operator/pkg/manifests" "github.com/openshift/cluster-ingress-operator/pkg/operator/controller" + //gwapidns "github.com/openshift/cluster-ingress-operator/pkg/operator/controller/gateway-service-dns" oputil "github.com/openshift/cluster-ingress-operator/pkg/util" awsutil "github.com/openshift/cluster-ingress-operator/pkg/util/aws" "github.com/openshift/cluster-ingress-operator/pkg/util/slice" @@ -80,6 +81,13 @@ func New(mgr manager.Manager, config Config) (runtimecontroller.Controller, erro if err != nil { return nil, err } + + // dnsRecords for GatewayAPI are reconciled separately and should be skipped by the dnsRecord watcher. + //skipDNSRecordForGatewayAPI := predicate.NewPredicateFuncs(func(o client.Object) bool { + // _, isGatewayAPI := o.(*iov1.DNSRecord).Labels[gwapidns.ManagedByIstioLabelKey] + // return !isGatewayAPI + // }) + if err := c.Watch(source.Kind(operatorCache, &iov1.DNSRecord{}), &handler.EnqueueRequestForObject{}, predicate.GenerationChangedPredicate{}); err != nil { return nil, err } diff --git a/pkg/operator/controller/gateway-service-dns/controller.go b/pkg/operator/controller/gateway-service-dns/controller.go index 4903c2597b..1d96abd32e 100644 --- a/pkg/operator/controller/gateway-service-dns/controller.go +++ b/pkg/operator/controller/gateway-service-dns/controller.go @@ -44,9 +44,10 @@ const ( // this label in the selector of any service that it creates for a // gateway. gatewayNameLabelKey = "istio.io/gateway-name" - // managedByIstioLabelKey is the key of a label that Istio adds to - // resources that it manages. - managedByIstioLabelKey = "gateway.istio.io/managed" + // ManagedByIstioLabelKey is the key of a label that Istio adds to + // resources that it manages. Exported for use as a filter in + // dns_controller. + ManagedByIstioLabelKey = "gateway.istio.io/managed" ) var log = logf.Logger.WithName(controllerName) @@ -69,7 +70,11 @@ func NewUnmanaged(mgr manager.Manager, config Config) (controller.Controller, er scheme := mgr.GetClient().Scheme() mapper := mgr.GetClient().RESTMapper() isServiceNeedingDNS := predicate.NewPredicateFuncs(func(o client.Object) bool { - _, ok := o.(*corev1.Service).Labels[managedByIstioLabelKey] + _, ok := o.(*corev1.Service).Labels[ManagedByIstioLabelKey] + return ok + }) + isDNSRecordForGatewayAPI := predicate.NewPredicateFuncs(func(o client.Object) bool { + _, ok := o.(*iov1.DNSRecord).Labels[ManagedByIstioLabelKey] return ok }) gatewayListenersChanged := predicate.Funcs{ @@ -79,17 +84,19 @@ func NewUnmanaged(mgr manager.Manager, config Config) (controller.Controller, er // A DNSRecord CR needs to be updated if, and only if, // the hostname has changed (a listener's port and // protocol have no bearing on the DNS record). + // TODO - do backendRefs changes matter? return gatewayListenersHostnamesChanged(old, new) }, } isInOperandNamespace := predicate.NewPredicateFuncs(func(o client.Object) bool { return o.GetNamespace() == config.OperandNamespace }) + gatewayToService := func(ctx context.Context, o client.Object) []reconcile.Request { var services corev1.ServiceList listOpts := []client.ListOption{ client.MatchingLabels{gatewayNameLabelKey: o.GetName()}, - client.InNamespace(config.OperandNamespace), + // TODO - can we watch other namespaces? client.InNamespace(config.OperandNamespace), } requests := []reconcile.Request{} if err := reconciler.cache.List(ctx, &services, listOpts...); err != nil { @@ -110,10 +117,10 @@ func NewUnmanaged(mgr manager.Manager, config Config) (controller.Controller, er if err := c.Watch(source.Kind(operatorCache, &gatewayapiv1beta1.Gateway{}), handler.EnqueueRequestsFromMapFunc(gatewayToService), isInOperandNamespace, gatewayListenersChanged); err != nil { return nil, err } - if err := c.Watch(source.Kind(operatorCache, &corev1.Service{}), &handler.EnqueueRequestForObject{}, isServiceNeedingDNS, isInOperandNamespace); err != nil { + if err := c.Watch(source.Kind(operatorCache, &corev1.Service{}), &handler.EnqueueRequestForObject{}, isServiceNeedingDNS); err != nil { return nil, err } - if err := c.Watch(source.Kind(operatorCache, &iov1.DNSRecord{}), handler.EnqueueRequestForOwner(scheme, mapper, &corev1.Service{}), isInOperandNamespace); err != nil { + if err := c.Watch(source.Kind(operatorCache, &iov1.DNSRecord{}), handler.EnqueueRequestForOwner(scheme, mapper, &corev1.Service{}), isInOperandNamespace, isDNSRecordForGatewayAPI); err != nil { return nil, err } return c, nil @@ -248,6 +255,7 @@ func (r *reconciler) ensureDNSRecordsForGateway(ctx context.Context, gateway *ga if dnsrecord.ManageDNSForDomain(domain, infraConfig.Status.PlatformStatus, dnsConfig) { dnsPolicy = iov1.ManagedDNS } + log.Info("ensuring dnsRecord", "name", name, "domain", domain, "service", service.Name) _, _, err := dnsrecord.EnsureDNSRecord(r.client, name, labels, ownerRef, domain, dnsPolicy, service) errs = append(errs, err) } @@ -275,6 +283,7 @@ func (r *reconciler) deleteStaleDNSRecordsForGateway(ctx context.Context, gatewa if domains.Has(dnsrecords.Items[i].Spec.DNSName) { continue } + log.Info("deleting dnsRecord not in domains", "dnsName", dnsrecords.Items[i].Spec.DNSName, "domains", domains.List()) name := types.NamespacedName{ Namespace: dnsrecords.Items[i].Namespace, Name: dnsrecords.Items[i].Name, diff --git a/pkg/resources/dnsrecord/dns.go b/pkg/resources/dnsrecord/dns.go index 79fc6f1990..d3130c7e6c 100644 --- a/pkg/resources/dnsrecord/dns.go +++ b/pkg/resources/dnsrecord/dns.go @@ -71,6 +71,10 @@ func EnsureDNSRecord(client client.Client, name types.NamespacedName, dnsRecordL if err != nil { return false, nil, err } + if !wantWC { + log.Info("no dnsRecord desired", "domain", domain, "service", service) + return haveWC, current, nil + } switch { case wantWC && !haveWC: @@ -83,6 +87,7 @@ func EnsureDNSRecord(client client.Client, name types.NamespacedName, dnsRecordL if updated, err := updateDNSRecord(client, current, desired); err != nil { return true, current, fmt.Errorf("failed to update dnsrecord %s/%s: %v", desired.Namespace, desired.Name, err) } else if updated { + log.Info("updated dnsrecord", "olddnsrecord", current, "desireddnsrecord", desired) return CurrentDNSRecord(client, name) } }