Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OCPBUGS-37220: Fix DNS for Gateway API on AWS #1108

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
8 changes: 8 additions & 0 deletions 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,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
}
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