Skip to content

Commit

Permalink
OCPBUGS-37220: Fix DNS for Gateway API on AWS
Browse files Browse the repository at this point in the history
  • Loading branch information
candita committed Jul 19, 2024
1 parent defe53f commit b6dc40b
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 9 deletions.
4 changes: 3 additions & 1 deletion pkg/dns/aws/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}

Expand Down
10 changes: 9 additions & 1 deletion pkg/operator/controller/dns/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -80,7 +81,14 @@ func New(mgr manager.Manager, config Config) (runtimecontroller.Controller, erro
if err != nil {
return nil, err
}
if err := c.Watch(source.Kind(operatorCache, &iov1.DNSRecord{}), &handler.EnqueueRequestForObject{}, predicate.GenerationChangedPredicate{}); err != nil {

// 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{}, skipDNSRecordForGatewayAPI); err != nil {
return nil, err
}
if err := c.Watch(source.Kind(operatorCache, &configv1.DNS{}), handler.EnqueueRequestsFromMapFunc(reconciler.ToDNSRecords)); err != nil {
Expand Down
23 changes: 16 additions & 7 deletions pkg/operator/controller/gateway-service-dns/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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{
Expand All @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 5 additions & 0 deletions pkg/resources/dnsrecord/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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)
}
}
Expand Down

0 comments on commit b6dc40b

Please sign in to comment.