Skip to content

agent: don't discard network status on IPv4-only route lookup failure - #233

Open
sebastian-pf9 wants to merge 1 commit into
mainfrom
claude/byoh-ipv6-gaps-0a5ef2
Open

agent: don't discard network status on IPv4-only route lookup failure#233
sebastian-pf9 wants to merge 1 commit into
mainfrom
claude/byoh-ipv6-gaps-0a5ef2

Conversation

@sebastian-pf9

Copy link
Copy Markdown
Collaborator

GetNetworkStatus() returned empty whenever the IPv4-only gateway lookup failed, blanking IPv6-only hosts entirely. Now checks both DiscoverInterface() and DiscoverInterfaceIPv6() and enumerates interfaces regardless of either's result.

Getting towards full IPv6 support is still some way to go

GetNetworkStatus() returned empty whenever the IPv4-only gateway lookup
failed, blanking IPv6-only hosts entirely. Now checks both
DiscoverInterface() and DiscoverInterfaceIPv6() and enumerates interfaces
regardless of either's result.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.29412% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
agent/registration/host_registrar.go 85.29% 3 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

@indradhanush indradhanush left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approve to unblock. Please see comments.


Expect(status).To(BeEmpty())
})
})

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can we please rewrite this as table driven non-ginkgo unit tests? I find this format time consuming in terms of evaluating all the test cases.


// GetNetworkStatus returns the network interface(s) status for the host
func (hr *HostRegistrar) GetNetworkStatus() []infrastructurev1beta1.NetworkStatus {
return hr.getNetworkStatus(gateway.DiscoverInterface, gateway.DiscoverInterfaceIPv6, systemInterfaces)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This feels like an anti pattern. I think GetNetowrkStatus should accept an interface (Go, not network interface). And the implementations of the interface are something like:

  • RealNet (the existing methods that are being passed here instead)
  • FakeNet (the fake methods in the test file)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants