diff --git a/cloud/linode/common.go b/cloud/linode/common.go index 221816bf..c4562c3c 100644 --- a/cloud/linode/common.go +++ b/cloud/linode/common.go @@ -1,14 +1,9 @@ package linode import ( - "context" "fmt" "strconv" "strings" - - "github.com/linode/linodego" - "k8s.io/apimachinery/pkg/types" - cloudprovider "k8s.io/cloud-provider" ) const providerIDPrefix = "linode://" @@ -31,34 +26,3 @@ func parseProviderID(providerID string) (int, error) { } return id, nil } - -func linodeFilterListOptions(targetLabel string) *linodego.ListOptions { - jsonFilter := fmt.Sprintf(`{"label":%q}`, targetLabel) - return linodego.NewListOptions(0, jsonFilter) -} - -func linodeByName(ctx context.Context, client Client, nodeName types.NodeName) (*linodego.Instance, error) { - linodes, err := client.ListInstances(ctx, linodeFilterListOptions(string(nodeName))) - if err != nil { - return nil, err - } - - if len(linodes) == 0 { - return nil, cloudprovider.InstanceNotFound - } else if len(linodes) > 1 { - return nil, fmt.Errorf("Multiple instances found with name %v", nodeName) - } - - return &linodes[0], nil -} - -func linodeByID(ctx context.Context, client Client, id int) (*linodego.Instance, error) { - instance, err := client.GetInstance(ctx, id) - if err != nil { - return nil, err - } - if instance == nil { - return nil, fmt.Errorf("linode not found with id %v", id) - } - return instance, nil -} diff --git a/cloud/linode/instances.go b/cloud/linode/instances.go index 3402159b..c803b32c 100644 --- a/cloud/linode/instances.go +++ b/cloud/linode/instances.go @@ -3,8 +3,10 @@ package linode import ( "context" "fmt" - "net/http" + "os" "strconv" + "sync" + "time" "github.com/linode/linode-cloud-controller-manager/sentry" "github.com/linode/linodego" @@ -13,12 +15,56 @@ import ( cloudprovider "k8s.io/cloud-provider" ) +type nodeCache struct { + sync.RWMutex + nodes map[int]*linodego.Instance + lastUpdate time.Time + ttl time.Duration +} + +// refreshInstances conditionally loads all instances from the Linode API and caches them. +// It does not refresh if the last update happened less than `nodeCache.ttl` ago. +func (nc *nodeCache) refreshInstances(ctx context.Context, client Client) error { + nc.Lock() + defer nc.Unlock() + + if time.Since(nc.lastUpdate) < nc.ttl { + return nil + } + + instances, err := client.ListInstances(ctx, nil) + if err != nil { + return err + } + nc.nodes = make(map[int]*linodego.Instance) + for _, instance := range instances { + instance := instance + nc.nodes[instance.ID] = &instance + } + nc.lastUpdate = time.Now() + + return nil +} + type instances struct { client Client + + nodeCache *nodeCache } func newInstances(client Client) cloudprovider.InstancesV2 { - return &instances{client} + var timeout int + if raw, ok := os.LookupEnv("LINODE_INSTANCE_CACHE_TTL"); ok { + timeout, _ = strconv.Atoi(raw) + } + if timeout == 0 { + timeout = 15 + } + + return &instances{client, &nodeCache{ + nodes: make(map[int]*linodego.Instance), + ttl: time.Duration(timeout) * time.Second, + }} } type instanceNoIPAddressesError struct { @@ -29,7 +75,33 @@ func (e instanceNoIPAddressesError) Error() string { return fmt.Sprintf("instance %d has no IP addresses", e.id) } +func (i *instances) linodeByName(nodeName types.NodeName) (*linodego.Instance, error) { + i.nodeCache.RLock() + defer i.nodeCache.RUnlock() + for _, node := range i.nodeCache.nodes { + if node.Label == string(nodeName) { + return node, nil + } + } + + return nil, cloudprovider.InstanceNotFound +} + +func (i *instances) linodeByID(id int) (*linodego.Instance, error) { + i.nodeCache.RLock() + defer i.nodeCache.RUnlock() + instance, ok := i.nodeCache.nodes[id] + if !ok { + return nil, cloudprovider.InstanceNotFound + } + return instance, nil +} + func (i *instances) lookupLinode(ctx context.Context, node *v1.Node) (*linodego.Instance, error) { + if err := i.nodeCache.refreshInstances(ctx, i.client); err != nil { + return nil, err + } + providerID := node.Spec.ProviderID nodeName := types.NodeName(node.Name) @@ -44,16 +116,16 @@ func (i *instances) lookupLinode(ctx context.Context, node *v1.Node) (*linodego. } sentry.SetTag(ctx, "linode_id", strconv.Itoa(id)) - return linodeByID(ctx, i.client, id) + return i.linodeByID(id) } - return linodeByName(ctx, i.client, nodeName) + return i.linodeByName(nodeName) } func (i *instances) InstanceExists(ctx context.Context, node *v1.Node) (bool, error) { ctx = sentry.SetHubOnContext(ctx) if _, err := i.lookupLinode(ctx, node); err != nil { - if apiError, ok := err.(*linodego.Error); ok && apiError.Code == http.StatusNotFound { + if err == cloudprovider.InstanceNotFound { return false, nil } sentry.CaptureError(ctx, err) diff --git a/cloud/linode/instances_test.go b/cloud/linode/instances_test.go index f42068a4..acfba0ab 100644 --- a/cloud/linode/instances_test.go +++ b/cloud/linode/instances_test.go @@ -2,10 +2,8 @@ package linode import ( "context" - "errors" "fmt" "net" - "net/http" "strconv" "testing" @@ -33,23 +31,11 @@ func TestInstanceExists(t *testing.T) { defer ctrl.Finish() client := NewMockClient(ctrl) - instances := newInstances(client) - - t.Run("should propagate generic api error", func(t *testing.T) { - node := nodeWithProviderID(providerIDPrefix + "123") - expectedErr := errors.New("some error") - client.EXPECT().GetInstance(gomock.Any(), 123).Times(1).Return(nil, expectedErr) - - exists, err := instances.InstanceExists(ctx, node) - assert.ErrorIs(t, err, expectedErr) - assert.False(t, exists) - }) t.Run("should return false if linode does not exist (by providerID)", func(t *testing.T) { + instances := newInstances(client) node := nodeWithProviderID(providerIDPrefix + "123") - client.EXPECT().GetInstance(gomock.Any(), 123).Times(1).Return(nil, &linodego.Error{ - Code: http.StatusNotFound, - }) + client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{}, nil) exists, err := instances.InstanceExists(ctx, node) assert.NoError(t, err) @@ -57,13 +43,10 @@ func TestInstanceExists(t *testing.T) { }) t.Run("should return false if linode does not exist (by name)", func(t *testing.T) { + instances := newInstances(client) name := "some-name" node := nodeWithName(name) - notFound := &linodego.Error{ - Code: http.StatusNotFound, - } - filter := linodeFilterListOptions(name) - client.EXPECT().ListInstances(gomock.Any(), filter).Times(1).Return(nil, notFound) + client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{}, nil) exists, err := instances.InstanceExists(ctx, node) assert.NoError(t, err) @@ -71,12 +54,13 @@ func TestInstanceExists(t *testing.T) { }) t.Run("should return true if linode exists (by provider)", func(t *testing.T) { + instances := newInstances(client) node := nodeWithProviderID(providerIDPrefix + "123") - client.EXPECT().GetInstance(gomock.Any(), 123).Times(1).Return(&linodego.Instance{ - ID: 123, - Label: "mock", - Region: "us-east", - Type: "g6-standard-2", + client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{ + {ID: 123, + Label: "mock", + Region: "us-east", + Type: "g6-standard-2"}, }, nil) exists, err := instances.InstanceExists(ctx, node) @@ -85,10 +69,11 @@ func TestInstanceExists(t *testing.T) { }) t.Run("should return true if linode exists (by name)", func(t *testing.T) { + instances := newInstances(client) name := "some-name" node := nodeWithName(name) - client.EXPECT().ListInstances(gomock.Any(), linodeFilterListOptions(name)).Times(1).Return([]linodego.Instance{ + client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{ {ID: 123, Label: name}, }, nil) @@ -104,13 +89,12 @@ func TestMetadataRetrieval(t *testing.T) { defer ctrl.Finish() client := NewMockClient(ctrl) - instances := newInstances(client) t.Run("errors when linode does not exist (by name)", func(t *testing.T) { + instances := newInstances(client) name := "does-not-exist" node := nodeWithName(name) - filter := linodeFilterListOptions(name) - client.EXPECT().ListInstances(gomock.Any(), filter).Times(1).Return([]linodego.Instance{}, nil) + client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{}, nil) meta, err := instances.InstanceMetadata(ctx, node) assert.ErrorIs(t, err, cloudprovider.InstanceNotFound) @@ -118,18 +102,19 @@ func TestMetadataRetrieval(t *testing.T) { }) t.Run("fails when linode does not exist (by provider)", func(t *testing.T) { + instances := newInstances(client) id := 456302 providerID := providerIDPrefix + strconv.Itoa(id) node := nodeWithProviderID(providerID) - getInstanceErr := &linodego.Error{Code: http.StatusNotFound} - client.EXPECT().GetInstance(gomock.Any(), id).Times(1).Return(nil, getInstanceErr) + client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{}, nil) meta, err := instances.InstanceMetadata(ctx, node) - assert.ErrorIs(t, err, getInstanceErr) + assert.ErrorIs(t, err, cloudprovider.InstanceNotFound) assert.Nil(t, meta) }) t.Run("should return data when linode is found (by name)", func(t *testing.T) { + instances := newInstances(client) id := 123 name := "mock-instance" node := nodeWithName(name) @@ -137,8 +122,7 @@ func TestMetadataRetrieval(t *testing.T) { privateIPv4 := net.ParseIP("192.168.133.65") linodeType := "g6-standard-1" region := "us-east" - filter := linodeFilterListOptions(name) - client.EXPECT().ListInstances(gomock.Any(), filter).Times(1).Return([]linodego.Instance{ + client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{ {ID: id, Label: name, Type: linodeType, Region: region, IPv4: []*net.IP{&publicIPv4, &privateIPv4}}, }, nil) @@ -184,6 +168,7 @@ func TestMetadataRetrieval(t *testing.T) { for _, test := range ipTests { t.Run(fmt.Sprintf("addresses are retrieved - %s", test.name), func(t *testing.T) { + instances := newInstances(client) id := 192910 name := "my-instance" providerID := providerIDPrefix + strconv.Itoa(id) @@ -200,8 +185,8 @@ func TestMetadataRetrieval(t *testing.T) { linodeType := "g6-standard-1" region := "us-east" - client.EXPECT().GetInstance(gomock.Any(), id).Times(1).Return(&linodego.Instance{ - ID: id, Label: name, Type: linodeType, Region: region, IPv4: ips, + client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{ + {ID: id, Label: name, Type: linodeType, Region: region, IPv4: ips}, }, nil) meta, err := instances.InstanceMetadata(ctx, node) @@ -225,19 +210,23 @@ func TestMalformedProviders(t *testing.T) { defer ctrl.Finish() client := NewMockClient(ctrl) - instances := newInstances(client) t.Run("fails on malformed providerID", func(t *testing.T) { + instances := newInstances(client) providerID := "bogus://bogus" node := nodeWithProviderID(providerID) + client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{}, nil) + meta, err := instances.InstanceMetadata(ctx, node) assert.ErrorIs(t, err, invalidProviderIDError{providerID}) assert.Nil(t, meta) }) t.Run("fails on non-numeric providerID", func(t *testing.T) { + instances := newInstances(client) providerID := providerIDPrefix + "abc" node := nodeWithProviderID(providerID) + client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{}, nil) meta, err := instances.InstanceMetadata(ctx, node) assert.ErrorIs(t, err, invalidProviderIDError{providerID}) @@ -251,12 +240,12 @@ func TestInstanceShutdown(t *testing.T) { defer ctrl.Finish() client := NewMockClient(ctrl) - instances := newInstances(client) t.Run("fails when instance not found (by provider)", func(t *testing.T) { + instances := newInstances(client) id := 12345 node := nodeWithProviderID(providerIDPrefix + strconv.Itoa(id)) - client.EXPECT().GetInstance(gomock.Any(), id).Times(1).Return(nil, linodego.Error{Code: http.StatusNotFound}) + client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{}, nil) shutdown, err := instances.InstanceShutdown(ctx, node) assert.Error(t, err) @@ -264,13 +253,10 @@ func TestInstanceShutdown(t *testing.T) { }) t.Run("fails when instance not found (by name)", func(t *testing.T) { + instances := newInstances(client) name := "some-name" node := nodeWithName(name) - notFound := &linodego.Error{ - Code: http.StatusNotFound, - } - filter := linodeFilterListOptions(name) - client.EXPECT().ListInstances(gomock.Any(), filter).Times(1).Return(nil, notFound) + client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{}, nil) shutdown, err := instances.InstanceShutdown(ctx, node) assert.Error(t, err) @@ -278,10 +264,11 @@ func TestInstanceShutdown(t *testing.T) { }) t.Run("returns true when instance is shut down", func(t *testing.T) { + instances := newInstances(client) id := 12345 node := nodeWithProviderID(providerIDPrefix + strconv.Itoa(id)) - client.EXPECT().GetInstance(gomock.Any(), id).Times(1).Return(&linodego.Instance{ - ID: id, Label: "offline-linode", Status: linodego.InstanceOffline, + client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{ + {ID: id, Label: "offline-linode", Status: linodego.InstanceOffline}, }, nil) shutdown, err := instances.InstanceShutdown(ctx, node) @@ -290,11 +277,11 @@ func TestInstanceShutdown(t *testing.T) { }) t.Run("returns true when instance is shutting down", func(t *testing.T) { + instances := newInstances(client) id := 12345 node := nodeWithProviderID(providerIDPrefix + strconv.Itoa(id)) - client.EXPECT().GetInstance(gomock.Any(), id).Times(1).Return(&linodego.Instance{ - ID: id, Label: "shutting-down-linode", Status: linodego.InstanceShuttingDown, - }, nil) + client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{ + {ID: id, Label: "shutting-down-linode", Status: linodego.InstanceShuttingDown}}, nil) shutdown, err := instances.InstanceShutdown(ctx, node) assert.NoError(t, err) @@ -302,10 +289,11 @@ func TestInstanceShutdown(t *testing.T) { }) t.Run("returns false when instance is running", func(t *testing.T) { + instances := newInstances(client) id := 12345 node := nodeWithProviderID(providerIDPrefix + strconv.Itoa(id)) - client.EXPECT().GetInstance(gomock.Any(), id).Times(1).Return(&linodego.Instance{ - ID: id, Label: "running-linode", Status: linodego.InstanceRunning, + client.EXPECT().ListInstances(gomock.Any(), nil).Times(1).Return([]linodego.Instance{ + {ID: id, Label: "running-linode", Status: linodego.InstanceRunning}, }, nil) shutdown, err := instances.InstanceShutdown(ctx, node)