Skip to content

[feat] Remove machines that have been marked for deletion from the dns record #787

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Jul 1, 2025
Merged

[feat] Remove machines that have been marked for deletion from the dns record #787

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2595660
Do not consider machines that have been deleted for dns
tchinmai7 Jul 1, 2025
ef0b9bf
code review
tchinmai7 Jul 1, 2025
d3da884
Merge branch 'main' into remove-deleted-nodesfrom-dns
tchinmai7 Jul 1, 2025
7df0eec
fix comment
tchinmai7 Jul 1, 2025
1d6eac1
rename
tchinmai7 Jul 1, 2025
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
58 changes: 38 additions & 20 deletions cloud/services/domains.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
"github.com/linode/linodego"
"golang.org/x/exp/slices"
"sigs.k8s.io/cluster-api/api/v1beta1"
kutil "sigs.k8s.io/cluster-api/util"

"github.com/linode/cluster-api-provider-linode/api/v1alpha2"
"github.com/linode/cluster-api-provider-linode/clients"
"github.com/linode/cluster-api-provider-linode/cloud/scope"
rutil "github.com/linode/cluster-api-provider-linode/util/reconciler"
Expand All @@ -35,7 +37,7 @@
func EnsureDNSEntries(ctx context.Context, cscope *scope.ClusterScope, operation string) error {
// Get the public IP that was assigned
var dnss DNSEntries
dnsEntries, err := dnss.getDNSEntriesToEnsure(cscope)
dnsEntries, err := dnss.getDNSEntriesToEnsure(ctx, cscope)
if err != nil {
return err
}
Expand Down Expand Up @@ -172,7 +174,6 @@
if err != nil {
return err
}

domainRecords, err := cscope.LinodeDomainsClient.ListDomainRecords(ctx, domainID, linodego.NewListOptions(0, string(filter)))
if err != nil {
return err
Expand Down Expand Up @@ -295,8 +296,38 @@
return stringList
}

func processLinodeMachine(ctx context.Context, cscope *scope.ClusterScope, machine v1alpha2.LinodeMachine, dnsTTLSec int, subdomain string) ([]DNSOptions, error) {
// Look up the corresponding CAPI machine, see if it is marked for deletion
capiMachine, err := kutil.GetOwnerMachine(ctx, cscope.Client, machine.ObjectMeta)
if err != nil {
return nil, fmt.Errorf("failed to get CAPI machine for LinodeMachine %s: %w", machine.Name, err)
}

Check warning on line 304 in cloud/services/domains.go

View check run for this annotation

Codecov / codecov/patch

cloud/services/domains.go#L303-L304 

Added lines #L303 - L304 were not covered by tests

if capiMachine == nil || capiMachine.DeletionTimestamp != nil {
Copy link
Preview

Copilot AI Jul 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Distinguish between a missing owner (nil) and a deleted CAPI Machine—silently skipping on nil may hide misconfigured resources. Consider logging or returning an explicit error for missing owners.

Copilot uses AI. Check for mistakes.

// If the CAPI machine is deleted, we don't need to create DNS entries for it.
return nil, nil
}

options := []DNSOptions{}
for _, IPs := range machine.Status.Addresses {
recordType := linodego.RecordTypeA
if IPs.Type != v1beta1.MachineExternalIP {
continue

Check warning on line 315 in cloud/services/domains.go

View check run for this annotation

Codecov / codecov/patch

cloud/services/domains.go#L315 

Added line #L315 was not covered by tests
}
addr, err := netip.ParseAddr(IPs.Address)
if err != nil {
return nil, fmt.Errorf("not a valid IP %w", err)
}

Check warning on line 320 in cloud/services/domains.go

View check run for this annotation

Codecov / codecov/patch

cloud/services/domains.go#L319-L320 

Added lines #L319 - L320 were not covered by tests
if !addr.Is4() {
recordType = linodego.RecordTypeAAAA
}
options = append(options, DNSOptions{subdomain, IPs.Address, recordType, dnsTTLSec})
}
return options, nil
}

// getDNSEntriesToEnsure return DNS entries to create/delete
func (d *DNSEntries) getDNSEntriesToEnsure(cscope *scope.ClusterScope) ([]DNSOptions, error) {
func (d *DNSEntries) getDNSEntriesToEnsure(ctx context.Context, cscope *scope.ClusterScope) ([]DNSOptions, error) {
d.mux.Lock()
defer d.mux.Unlock()
dnsTTLSec := rutil.DefaultDNSTTLSec
Expand All @@ -305,25 +336,12 @@
}

subDomain := getSubDomain(cscope)

for _, eachMachine := range cscope.LinodeMachines.Items {
for _, IPs := range eachMachine.Status.Addresses {
recordType := linodego.RecordTypeA
if IPs.Type != v1beta1.MachineExternalIP {
continue
}
addr, err := netip.ParseAddr(IPs.Address)
if err != nil {
return nil, fmt.Errorf("not a valid IP %w", err)
}
if !addr.Is4() {
recordType = linodego.RecordTypeAAAA
}
d.options = append(d.options, DNSOptions{subDomain, IPs.Address, recordType, dnsTTLSec})
}
if len(d.options) == 0 {
continue
options, err := processLinodeMachine(ctx, cscope, eachMachine, dnsTTLSec, subDomain)
if err != nil {
return nil, fmt.Errorf("failed to process LinodeMachine %s: %w", eachMachine.Name, err)

Check warning on line 342 in cloud/services/domains.go

View check run for this annotation

Codecov / codecov/patch

cloud/services/domains.go#L342 

Added line #L342 was not covered by tests
}
d.options = append(d.options, options...)
}
d.options = append(d.options, DNSOptions{subDomain, cscope.LinodeCluster.Name, linodego.RecordTypeTXT, dnsTTLSec})

Expand Down
Loading
Loading