Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
35 changes: 28 additions & 7 deletions controllers/maascluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,34 +171,56 @@ func (r *MaasClusterReconciler) reconcileDNSAttachments(clusterScope *scope.Clus
return errors.Wrapf(err, "unable to find preferred subnets")
}

// Build desired IPs first (no MAAS call)
// Build desired IPs first (no MAAS call). Track running CP count separately so we can
// distinguish "machines powered off" from "machines running but IPs filtered by subnets".
var runningCPCount int
var runningIpAddresses []string
for _, m := range machines {
if !IsControlPlaneMachine(m) {
continue
}
machineIP := getExternalMachineIP(clusterScope.Logger, preferredSubnets, m)
isRunningHealthy := IsRunning(m)
if !m.DeletionTimestamp.IsZero() || !isRunningHealthy {
continue
}
runningCPCount++
machineIP := getExternalMachineIP(clusterScope.Logger, preferredSubnets, m)
if machineIP != "" {
runningIpAddresses = append(runningIpAddresses, machineIP)
}
}

// Early-exit gate using last-applied hash
desiredHash := infrautil.StableHashStringSlice(runningIpAddresses)
if clusterScope.MaasCluster.Annotations != nil && clusterScope.MaasCluster.Annotations[lastAppliedAnn] == desiredHash {
// Never wipe DNS when all CP machines are transiently unavailable (e.g. powered off during
// a flap). An empty desired set would clear the MAAS A records and the hash would be cached
// as SHA-256(""), preventing self-recovery until the controller pod itself is reachable again.
if len(runningIpAddresses) == 0 {
if runningCPCount > 0 {
clusterScope.Info("CP machines are running but no IPs match preferred subnets; preserving existing DNS records",
"runningCPs", runningCPCount)
} else {
clusterScope.Info("No running CP machines found; preserving existing DNS records")
}
return nil
}

// Only now fetch DNS resource once
// Fetch DNS resource once — needed for both drift check and potential update.
dnsResource, err := dnssvc.GetDNSResource()
if err != nil {
return errors.Wrap(err, "Unable to get the dns resource")
}

// Early-exit only when the annotation hash matches AND MAAS state agrees with desired IPs.
// Verifying MAAS state catches external drift (e.g. DNS records manually wiped while the
// desired set was unchanged, so the hash never changed).
desiredHash := infrautil.StableHashStringSlice(runningIpAddresses)
if clusterScope.MaasCluster.Annotations != nil && clusterScope.MaasCluster.Annotations[lastAppliedAnn] == desiredHash {
Comment thread
AmitSahastra marked this conversation as resolved.
if !dnssvc.IsDriftDetected(dnsResource, runningIpAddresses) {
return nil
}
clusterScope.Info("DNS drift detected: MAAS state diverges from last-applied annotation; forcing re-sync",
"desiredIPs", runningIpAddresses)
}

// Use optimized update with in-resource idempotency
updated, err := dnssvc.UpdateDNSAttachmentsWithResource(dnsResource, runningIpAddresses)
if err != nil {
Expand All @@ -211,7 +233,6 @@ func (r *MaasClusterReconciler) reconcileDNSAttachments(clusterScope *scope.Clus
}
clusterScope.MaasCluster.Annotations[lastAppliedAnn] = desiredHash

// Best-effort requeue hint remains optional; we skip computing current vs attached
if updated {
clusterScope.Info("DNS attachments updated; will continue monitoring for changes")
}
Expand Down
23 changes: 23 additions & 0 deletions pkg/maas/dns/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,24 @@ func (s *Service) ReconcileDNS() error {
return nil
}

// IsDriftDetected returns true if the MAAS DNS resource's current IP set differs from desiredIPs.
// Used to catch external drift (e.g. manual wipe) even when the annotation hash has not changed.
func (s *Service) IsDriftDetected(dnsResource maasclient.DNSResource, desiredIPs []string) bool {
desired := sets.NewString()
for _, ip := range desiredIPs {
if ip != "" {
desired.Insert(ip)
}
}
current := sets.NewString()
for _, addr := range dnsResource.IPAddresses() {
if a := addr.IP().String(); a != "" {
current.Insert(a)
}
}
return !desired.Equal(current)
}
Comment thread
AmitSahastra marked this conversation as resolved.

// UpdateDNSAttachmentsWithResource updates DNS attachments using a pre-fetched DNS resource,
// avoiding additional GET API calls. Returns true if an update was performed.
func (s *Service) UpdateDNSAttachmentsWithResource(dnsResource maasclient.DNSResource, IPs []string) (bool, error) {
Expand All @@ -83,6 +101,11 @@ func (s *Service) updateResourceIPs(dnsResource maasclient.DNSResource, IPs []st
}
}

// Refuse to wipe all A records; callers must guard before reaching here.
if desired.Len() == 0 {
return false, errors.New("refusing to PUT empty IP set to MAAS DNS resource")
}

Comment thread
AmitSahastra marked this conversation as resolved.
Outdated
// Build current set from resource
current := sets.NewString()
for _, addr := range dnsResource.IPAddresses() {
Expand Down