diff --git a/cloud/linode/services/instances.go b/cloud/linode/services/instances.go index 4faccc06..9d4de001 100644 --- a/cloud/linode/services/instances.go +++ b/cloud/linode/services/instances.go @@ -103,8 +103,7 @@ func (nc *nodeCache) refreshInstances(ctx context.Context, client client.Client) } resp, err := GetVPCIPAddresses(ctx, client, vpcName) if err != nil { - klog.Errorf("failed updating instances cache for VPC %s. Error: %s", vpcName, err.Error()) - continue + return fmt.Errorf("failed updating instances cache for VPC %s: %w", vpcName, err) } for _, vpcip := range resp { if vpcip.Address == nil { @@ -115,8 +114,7 @@ func (nc *nodeCache) refreshInstances(ctx context.Context, client client.Client) resp, err = GetVPCIPv6Addresses(ctx, client, vpcName) if err != nil { - klog.Errorf("failed updating instances cache for VPC %s. Error: %s", vpcName, err.Error()) - continue + return fmt.Errorf("failed updating instances cache for VPC %s: %w", vpcName, err) } for _, vpcip := range resp { if len(vpcip.IPv6Addresses) == 0 { diff --git a/cloud/linode/services/instances_test.go b/cloud/linode/services/instances_test.go index 4d6a5c1a..7c6bab58 100644 --- a/cloud/linode/services/instances_test.go +++ b/cloud/linode/services/instances_test.go @@ -456,6 +456,116 @@ func TestMalformedProviders(t *testing.T) { }) } +func TestRefreshInstancesVPCErrors(t *testing.T) { + ctx := t.Context() + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + c := mocks.NewMockClient(ctrl) + + t.Run("returns error when GetVPCIPAddresses fails", func(t *testing.T) { + instances := NewInstances(c) + node := nodeWithProviderID(ccmUtils.ProviderIDPrefix + "123") + + options.Options.VPCNames = []string{"test"} + VpcIDs["test"] = 1 + defer func() { + options.Options.VPCNames = []string{} + delete(VpcIDs, "test") + }() + + apiErr := fmt.Errorf("VPC API error") + c.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{ + {ID: 123, Label: "test-instance"}, + }, nil) + c.EXPECT().ListVPCIPAddresses(gomock.Any(), 1, gomock.Any()).Return(nil, apiErr) + + _, err := instances.InstanceExists(ctx, node) + require.Error(t, err) + assert.ErrorContains(t, err, "failed updating instances cache for VPC test") + }) + + t.Run("returns error when GetVPCIPv6Addresses fails", func(t *testing.T) { + instances := NewInstances(c) + node := nodeWithProviderID(ccmUtils.ProviderIDPrefix + "123") + + options.Options.VPCNames = []string{"test"} + VpcIDs["test"] = 1 + defer func() { + options.Options.VPCNames = []string{} + delete(VpcIDs, "test") + }() + + vpcIP := "10.0.0.2" + apiErr := fmt.Errorf("VPC IPv6 API error") + c.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{ + {ID: 123, Label: "test-instance"}, + }, nil) + c.EXPECT().ListVPCIPAddresses(gomock.Any(), 1, gomock.Any()).Return([]linodego.VPCIP{ + {Address: &vpcIP, LinodeID: 123, VPCID: 1}, + }, nil) + c.EXPECT().ListVPCIPv6Addresses(gomock.Any(), 1, gomock.Any()).Return(nil, apiErr) + + _, err := instances.InstanceExists(ctx, node) + require.Error(t, err) + assert.ErrorContains(t, err, "failed updating instances cache for VPC test") + }) + + t.Run("does not update cache when GetVPCIPAddresses fails", func(t *testing.T) { + instances := NewInstances(c) + node := nodeWithProviderID(ccmUtils.ProviderIDPrefix + "123") + + options.Options.VPCNames = []string{"test"} + VpcIDs["test"] = 1 + defer func() { + options.Options.VPCNames = []string{} + delete(VpcIDs, "test") + }() + + apiErr := fmt.Errorf("VPC API error") + c.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{ + {ID: 123, Label: "test-instance"}, + }, nil) + c.EXPECT().ListVPCIPAddresses(gomock.Any(), 1, gomock.Any()).Return(nil, apiErr) + + _, err := instances.InstanceExists(ctx, node) + require.Error(t, err) + + // Cache must not be stamped so the next call retries instead of + // serving the empty/stale data that caused the original node-deletion bug. + assert.True(t, instances.nodeCache.lastUpdate.IsZero(), "cache lastUpdate should remain zero after a failed refresh") + }) + + t.Run("does not update cache when GetVPCIPv6Addresses fails", func(t *testing.T) { + instances := NewInstances(c) + node := nodeWithProviderID(ccmUtils.ProviderIDPrefix + "123") + + options.Options.VPCNames = []string{"test"} + VpcIDs["test"] = 1 + defer func() { + options.Options.VPCNames = []string{} + delete(VpcIDs, "test") + }() + + vpcIP := "10.0.0.2" + apiErr := fmt.Errorf("VPC IPv6 API error") + c.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{ + {ID: 123, Label: "test-instance"}, + }, nil) + c.EXPECT().ListVPCIPAddresses(gomock.Any(), 1, gomock.Any()).Return([]linodego.VPCIP{ + {Address: &vpcIP, LinodeID: 123, VPCID: 1}, + }, nil) + c.EXPECT().ListVPCIPv6Addresses(gomock.Any(), 1, gomock.Any()).Return(nil, apiErr) + + _, err := instances.InstanceExists(ctx, node) + require.Error(t, err) + + // Cache must not be stamped so the next call retries instead of + // serving the empty/stale data that caused the original node-deletion bug. + assert.True(t, instances.nodeCache.lastUpdate.IsZero(), "cache lastUpdate should remain zero after a failed refresh") + }) +} + func TestInstanceShutdown(t *testing.T) { ctx := t.Context() ctrl := gomock.NewController(t)