Skip to content

Commit 8cbe826

Browse files
steffen-karlssonsteffen-karlsson
committed
feat(instance): avoid missing Node Refs for Private Network Instances without pnNIC
Signed-off-by: Steffen Karlsson <steffen.karlsson@gmail.com>
1 parent fda1045 commit 8cbe826

2 files changed

Lines changed: 257 additions & 29 deletions

File tree

‎scaleway/instances.go‎

Lines changed: 69 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -178,49 +178,90 @@ func (i *instances) instanceAddresses(server *scwinstance.Server) ([]v1.NodeAddr
178178
}
179179
}
180180

181-
var pnNIC *scwinstance.PrivateNIC
181+
// Try to get private network IPs
182+
privateAddresses, err := i.getPrivateNetworkAddresses(server)
183+
if err != nil {
184+
klog.Warningf("error getting private network addresses for node %s: %v", server.Name, err)
185+
}
186+
187+
if len(privateAddresses) > 0 {
188+
addresses = append(addresses, privateAddresses...)
189+
return addresses, nil
190+
}
191+
192+
// fallback to legacy private ip
193+
if server.PrivateIP != nil && *server.PrivateIP != "" {
194+
addresses = append(
195+
addresses,
196+
v1.NodeAddress{Type: v1.NodeInternalIP, Address: *server.PrivateIP},
197+
v1.NodeAddress{Type: v1.NodeInternalDNS, Address: fmt.Sprintf("%s.priv.instances.scw.cloud", server.ID)},
198+
)
199+
}
200+
201+
return addresses, nil
202+
}
203+
204+
// getPrivateNetworkAddresses returns private network IPs for the server.
205+
// If pnID is configured, it only looks up IPs for that specific private network.
206+
// If pnID is not configured, it iterates through all private NICs and returns all found IPs.
207+
func (i *instances) getPrivateNetworkAddresses(server *scwinstance.Server) ([]v1.NodeAddress, error) {
208+
if len(server.PrivateNics) == 0 {
209+
return nil, nil
210+
}
211+
212+
region, err := server.Zone.Region()
213+
if err != nil {
214+
return nil, fmt.Errorf("unable to get region from zone %s: %v", server.Zone, err)
215+
}
216+
217+
var addresses []v1.NodeAddress
218+
219+
// If a specific private network ID is configured, only look for that one
182220
if i.pnID != "" {
183221
for _, pNIC := range server.PrivateNics {
184222
if pNIC.PrivateNetworkID == i.pnID {
185-
pnNIC = pNIC
186-
break
223+
nicAddresses, err := i.getIPsForPrivateNIC(server, pNIC, region)
224+
if err != nil {
225+
return nil, err
226+
}
227+
return nicAddresses, nil
187228
}
188229
}
230+
// Configured pnID not found in server's private NICs
231+
return nil, nil
189232
}
190233

191-
if pnNIC != nil {
192-
region, _ := server.Zone.Region()
193-
ips, err := i.ipam.ListIPs(&scwipam.ListIPsRequest{
194-
ProjectID: &server.Project,
195-
ResourceType: scwipam.ResourceTypeInstancePrivateNic,
196-
ResourceID: &pnNIC.ID,
197-
IsIPv6: scw.BoolPtr(false),
198-
Region: region,
199-
})
234+
// No specific pnID configured - iterate through all private NICs
235+
for _, pNIC := range server.PrivateNics {
236+
nicAddresses, err := i.getIPsForPrivateNIC(server, pNIC, region)
200237
if err != nil {
201-
return addresses, fmt.Errorf("unable to query ipam for node %s: %v", server.Name, err)
202-
}
203-
204-
if len(ips.IPs) == 0 {
205-
return addresses, fmt.Errorf("no private network ip for node %s", server.Name)
238+
klog.Warningf("error getting IPs for private NIC %s on node %s: %v", pNIC.ID, server.Name, err)
239+
continue
206240
}
241+
addresses = append(addresses, nicAddresses...)
242+
}
207243

208-
for _, nicIP := range ips.IPs {
209-
addresses = append(
210-
addresses,
211-
v1.NodeAddress{Type: v1.NodeInternalIP, Address: nicIP.Address.IP.String()},
212-
)
213-
}
244+
return addresses, nil
245+
}
214246

215-
return addresses, nil
247+
// getIPsForPrivateNIC queries IPAM for IPs assigned to a specific private NIC
248+
func (i *instances) getIPsForPrivateNIC(server *scwinstance.Server, pNIC *scwinstance.PrivateNIC, region scw.Region) ([]v1.NodeAddress, error) {
249+
ips, err := i.ipam.ListIPs(&scwipam.ListIPsRequest{
250+
ProjectID: &server.Project,
251+
ResourceType: scwipam.ResourceTypeInstancePrivateNic,
252+
ResourceID: &pNIC.ID,
253+
IsIPv6: scw.BoolPtr(false),
254+
Region: region,
255+
})
256+
if err != nil {
257+
return nil, fmt.Errorf("unable to query ipam for node %s NIC %s: %v", server.Name, pNIC.ID, err)
216258
}
217259

218-
// fallback to legacy private ip
219-
if server.PrivateIP != nil && *server.PrivateIP != "" {
260+
var addresses []v1.NodeAddress
261+
for _, nicIP := range ips.IPs {
220262
addresses = append(
221263
addresses,
222-
v1.NodeAddress{Type: v1.NodeInternalIP, Address: *server.PrivateIP},
223-
v1.NodeAddress{Type: v1.NodeInternalDNS, Address: fmt.Sprintf("%s.priv.instances.scw.cloud", server.ID)},
264+
v1.NodeAddress{Type: v1.NodeInternalIP, Address: nicIP.Address.IP.String()},
224265
)
225266
}
226267

‎scaleway/instances_test.go‎

Lines changed: 188 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"testing"
2424

2525
scwinstance "github.com/scaleway/scaleway-sdk-go/api/instance/v1"
26+
scwipam "github.com/scaleway/scaleway-sdk-go/api/ipam/v1"
2627
"github.com/scaleway/scaleway-sdk-go/scw"
2728
v1 "k8s.io/api/core/v1"
2829
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -134,6 +135,59 @@ func newFakeInstanceAPI() *fakeInstanceAPI {
134135
CommercialType: "GP1-XS",
135136
State: scwinstance.ServerStateStarting,
136137
},
138+
// a server with private network only (no public IP)
139+
"private-only-server-id": {
140+
Zone: "fr-par-2",
141+
Name: "scw-private-only",
142+
CommercialType: "PLAY2-NANO",
143+
State: scwinstance.ServerStateRunning,
144+
Project: "test-project",
145+
PrivateNics: []*scwinstance.PrivateNIC{
146+
{
147+
ID: "nic-private-only",
148+
PrivateNetworkID: "pn-12345",
149+
},
150+
},
151+
},
152+
// a server with both public and private network
153+
"both-networks-server-id": {
154+
Zone: "fr-par-2",
155+
Name: "scw-both-networks",
156+
CommercialType: "PLAY2-NANO",
157+
State: scwinstance.ServerStateRunning,
158+
Project: "test-project",
159+
PublicIPs: []*scwinstance.ServerIP{
160+
{
161+
Address: net.ParseIP("51.159.100.1"),
162+
ProvisioningMode: scwinstance.ServerIPProvisioningModeDHCP,
163+
Family: scwinstance.ServerIPIPFamilyInet,
164+
},
165+
},
166+
PrivateNics: []*scwinstance.PrivateNIC{
167+
{
168+
ID: "nic-both-networks",
169+
PrivateNetworkID: "pn-12345",
170+
},
171+
},
172+
},
173+
// a server with multiple private networks
174+
"multi-pn-server-id": {
175+
Zone: "fr-par-2",
176+
Name: "scw-multi-pn",
177+
CommercialType: "PLAY2-NANO",
178+
State: scwinstance.ServerStateRunning,
179+
Project: "test-project",
180+
PrivateNics: []*scwinstance.PrivateNIC{
181+
{
182+
ID: "nic-private-only",
183+
PrivateNetworkID: "pn-12345",
184+
},
185+
{
186+
ID: "nic-other-pn",
187+
PrivateNetworkID: "pn-67890",
188+
},
189+
},
190+
},
137191
},
138192
}
139193
}
@@ -171,9 +225,65 @@ func (f *fakeInstanceAPI) GetServer(req *scwinstance.GetServerRequest, opts ...s
171225
}, nil
172226
}
173227

228+
// fakeIPAMAPI implements IPAMAPI for testing
229+
type fakeIPAMAPI struct {
230+
// IPs maps NIC ID to list of IPs
231+
IPs map[string][]*scwipam.IP
232+
}
233+
234+
func newFakeIPAMAPI() *fakeIPAMAPI {
235+
return &fakeIPAMAPI{
236+
IPs: map[string][]*scwipam.IP{
237+
// IPs for private NIC on server with private network only
238+
"nic-private-only": {
239+
{
240+
Address: scw.IPNet{IPNet: net.IPNet{IP: net.ParseIP("10.200.0.5"), Mask: net.CIDRMask(20, 32)}},
241+
},
242+
},
243+
// IPs for private NIC on server with both public and private
244+
"nic-both-networks": {
245+
{
246+
Address: scw.IPNet{IPNet: net.IPNet{IP: net.ParseIP("10.200.0.10"), Mask: net.CIDRMask(20, 32)}},
247+
},
248+
},
249+
// IPs for another private network
250+
"nic-other-pn": {
251+
{
252+
Address: scw.IPNet{IPNet: net.IPNet{IP: net.ParseIP("192.168.1.5"), Mask: net.CIDRMask(24, 32)}},
253+
},
254+
},
255+
},
256+
}
257+
}
258+
259+
func (f *fakeIPAMAPI) ListIPs(req *scwipam.ListIPsRequest, opts ...scw.RequestOption) (*scwipam.ListIPsResponse, error) {
260+
if req.ResourceID == nil {
261+
return &scwipam.ListIPsResponse{IPs: []*scwipam.IP{}}, nil
262+
}
263+
264+
ips, ok := f.IPs[*req.ResourceID]
265+
if !ok {
266+
return &scwipam.ListIPsResponse{IPs: []*scwipam.IP{}}, nil
267+
}
268+
269+
return &scwipam.ListIPsResponse{
270+
IPs: ips,
271+
TotalCount: uint64(len(ips)),
272+
}, nil
273+
}
274+
174275
func newFakeInstances() *instances {
175276
return &instances{
176-
api: newFakeInstanceAPI(),
277+
api: newFakeInstanceAPI(),
278+
ipam: newFakeIPAMAPI(),
279+
}
280+
}
281+
282+
func newFakeInstancesWithPNID(pnID string) *instances {
283+
return &instances{
284+
api: newFakeInstanceAPI(),
285+
ipam: newFakeIPAMAPI(),
286+
pnID: pnID,
177287
}
178288
}
179289

@@ -577,3 +687,80 @@ func TestInstances_InstanceMetadata(t *testing.T) {
577687

578688
})
579689
}
690+
691+
func TestInstances_PrivateNetworkAddresses(t *testing.T) {
692+
t.Run("PrivateNetworkOnly_NoPNID", func(t *testing.T) {
693+
// When no pnID is configured, the CCM should discover private IPs from all NICs
694+
instance := newFakeInstances()
695+
696+
expectedAddresses := []v1.NodeAddress{
697+
{Type: v1.NodeHostName, Address: "scw-private-only"},
698+
{Type: v1.NodeInternalIP, Address: "10.200.0.5"},
699+
}
700+
701+
returnedAddresses, err := instance.NodeAddressesByProviderID(context.TODO(), "scaleway://instance/fr-par-2/private-only-server-id")
702+
AssertNoError(t, err)
703+
Equals(t, expectedAddresses, returnedAddresses)
704+
})
705+
706+
t.Run("PrivateNetworkOnly_WithPNID", func(t *testing.T) {
707+
// When pnID is configured, only look for that specific private network
708+
instance := newFakeInstancesWithPNID("pn-12345")
709+
710+
expectedAddresses := []v1.NodeAddress{
711+
{Type: v1.NodeHostName, Address: "scw-private-only"},
712+
{Type: v1.NodeInternalIP, Address: "10.200.0.5"},
713+
}
714+
715+
returnedAddresses, err := instance.NodeAddressesByProviderID(context.TODO(), "scaleway://instance/fr-par-2/private-only-server-id")
716+
AssertNoError(t, err)
717+
Equals(t, expectedAddresses, returnedAddresses)
718+
})
719+
720+
t.Run("BothNetworks_NoPNID", func(t *testing.T) {
721+
// Server with both public and private network, no specific pnID configured
722+
instance := newFakeInstances()
723+
724+
expectedAddresses := []v1.NodeAddress{
725+
{Type: v1.NodeHostName, Address: "scw-both-networks"},
726+
{Type: v1.NodeExternalIP, Address: "51.159.100.1"},
727+
{Type: v1.NodeExternalDNS, Address: "both-networks-server-id.pub.instances.scw.cloud"},
728+
{Type: v1.NodeInternalIP, Address: "10.200.0.10"},
729+
}
730+
731+
returnedAddresses, err := instance.NodeAddressesByProviderID(context.TODO(), "scaleway://instance/fr-par-2/both-networks-server-id")
732+
AssertNoError(t, err)
733+
Equals(t, expectedAddresses, returnedAddresses)
734+
})
735+
736+
t.Run("MultiplePrivateNetworks_NoPNID", func(t *testing.T) {
737+
// Server with multiple private networks, no specific pnID configured
738+
// Should return IPs from all private networks
739+
instance := newFakeInstances()
740+
741+
expectedAddresses := []v1.NodeAddress{
742+
{Type: v1.NodeHostName, Address: "scw-multi-pn"},
743+
{Type: v1.NodeInternalIP, Address: "10.200.0.5"},
744+
{Type: v1.NodeInternalIP, Address: "192.168.1.5"},
745+
}
746+
747+
returnedAddresses, err := instance.NodeAddressesByProviderID(context.TODO(), "scaleway://instance/fr-par-2/multi-pn-server-id")
748+
AssertNoError(t, err)
749+
Equals(t, expectedAddresses, returnedAddresses)
750+
})
751+
752+
t.Run("MultiplePrivateNetworks_WithPNID", func(t *testing.T) {
753+
// Server with multiple private networks, specific pnID configured
754+
// Should only return IPs from the configured private network
755+
instance := newFakeInstancesWithPNID("pn-67890")
756+
757+
expectedAddresses := []v1.NodeAddress{
758+
{Type: v1.NodeHostName, Address: "scw-multi-pn"},
759+
{Type: v1.NodeInternalIP, Address: "192.168.1.5"},
760+
}
761+
762+
returnedAddresses, err := instance.NodeAddressesByProviderID(context.TODO(), "scaleway://instance/fr-par-2/multi-pn-server-id")
763+
AssertNoError(t, err)
764+
Equals(t, expectedAddresses, returnedAddresses)
765+
})
766+
}

0 commit comments

Comments
 (0)