Various fixes to the OVHcloud provider

cluster-autoscaler/cloudprovider/ovhcloud/ovh_cloud_manager.go

@@ -23,10 +23,14 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
+	"time"
 
 	"k8s.io/autoscaler/cluster-autoscaler/cloudprovider/ovhcloud/sdk"
+	"k8s.io/klog/v2"
 )
 
+const flavorCacheDuration = time.Hour
+
 // ClientInterface defines all mandatory methods to be exposed as a client (mock or API)
 type ClientInterface interface {
 	// ListNodePools lists all the node pools found in a Kubernetes cluster.
@@ -44,8 +48,8 @@ type ClientInterface interface {
 	// DeleteNodePool deletes a specific pool.
 	DeleteNodePool(ctx context.Context, projectID string, clusterID string, poolID string) (*sdk.NodePool, error)
 
-	// ListFlavors list all available flavors usable in a Kubernetes cluster.
-	ListFlavors(ctx context.Context, projectID string, clusterID string) ([]sdk.Flavor, error)
+	// ListClusterFlavors list all available flavors usable in a Kubernetes cluster.
+	ListClusterFlavors(ctx context.Context, projectID string, clusterID string) ([]sdk.Flavor, error)
 }
 
 // OvhCloudManager defines current application context manager to interact
@@ -57,7 +61,11 @@ type OvhCloudManager struct {
 	ClusterID string
 	ProjectID string
 
-	NodePools []sdk.NodePool
+	NodePools              []sdk.NodePool
+	NodeGroupPerProviderID map[string]*NodeGroup
+
+	FlavorsCache               map[string]sdk.Flavor
+	FlavorsCacheExpirationTime time.Time
 }
 
 // Config is the configuration file content of OVHcloud provider
@@ -138,10 +146,52 @@ func NewManager(configFile io.Reader) (*OvhCloudManager, error) {
 		ProjectID: cfg.ProjectID,
 		ClusterID: cfg.ClusterID,
 
-		NodePools: make([]sdk.NodePool, 0),
+		NodePools:              make([]sdk.NodePool, 0),
+		NodeGroupPerProviderID: make(map[string]*NodeGroup),
+
+		FlavorsCache:               make(map[string]sdk.Flavor),
+		FlavorsCacheExpirationTime: time.Time{},
 	}, nil
 }
 
+// getFlavorsByName lists available flavors from cache or from OVHCloud APIs if the cache is outdated
+func (m *OvhCloudManager) getFlavorsByName() (map[string]sdk.Flavor, error) {
+	// Update the flavors cache if expired
+	if m.FlavorsCacheExpirationTime.Before(time.Now()) {
+		newFlavorCacheExpirationTime := time.Now().Add(flavorCacheDuration)
+		klog.V(4).Infof("Listing flavors to update flavors cache (will expire at %s)", newFlavorCacheExpirationTime)
+
+		// Fetch all flavors in API
+		flavors, err := m.Client.ListClusterFlavors(context.Background(), m.ProjectID, m.ClusterID)
+		if err != nil {
+			return nil, fmt.Errorf("failed to list available flavors: %w", err)
+		}
+
+		// Update the flavors cache
+		m.FlavorsCache = make(map[string]sdk.Flavor)
+		for _, flavor := range flavors {
+			m.FlavorsCache[flavor.Name] = flavor
+			m.FlavorsCacheExpirationTime = newFlavorCacheExpirationTime
+		}
+	}
+
+	return m.FlavorsCache, nil
+}
+
+// getFlavorByName returns the given flavor from cache or API
+func (m *OvhCloudManager) getFlavorByName(flavorName string) (sdk.Flavor, error) {
+	flavorsByName, err := m.getFlavorsByName()
+	if err != nil {
+		return sdk.Flavor{}, err
+	}
+
+	if flavor, ok := flavorsByName[flavorName]; ok {
+		return flavor, nil
+	}
+
+	return sdk.Flavor{}, fmt.Errorf("flavor %s not found in available flavors", flavorName)
+}
+
 // ReAuthenticate allows OpenStack keystone token to be revoked and re-created to call API
 func (m *OvhCloudManager) ReAuthenticate() error {
 	if m.OpenStackProvider != nil {

cluster-autoscaler/cloudprovider/ovhcloud/ovh_cloud_manager_test.go

@@ -0,0 +1,173 @@
+/*
+Copyright 2020 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package ovhcloud
+
+import (
+	"bytes"
+	"context"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"k8s.io/autoscaler/cluster-autoscaler/cloudprovider/ovhcloud/sdk"
+)
+
+func newTestManager(t *testing.T) *OvhCloudManager {
+	cfg := `{
+		"project_id": "projectID",
+		"cluster_id": "clusterID",
+		"authentication_type": "consumer",
+		"application_endpoint": "ovh-eu",
+		"application_key": "key",
+		"application_secret": "secret",
+		"application_consumer_key": "consumer_key"
+	}`
+
+	manager, err := NewManager(bytes.NewBufferString(cfg))
+	if err != nil {
+		assert.FailNow(t, "failed to create manager", err)
+	}
+
+	client := &sdk.ClientMock{}
+	ctx := context.Background()
+
+	client.On("ListClusterFlavors", ctx, "projectID", "clusterID").Return(
+		[]sdk.Flavor{
+			{
+				Name:     "b2-7",
+				Category: "b",
+				State:    "available",
+				VCPUs:    2,
+				GPUs:     0,
+				RAM:      7,
+			},
+			{
+				Name:     "t1-45",
+				Category: "t",
+				State:    "available",
+				VCPUs:    8,
+				GPUs:     1,
+				RAM:      45,
+			},
+			{
+				Name:     "unknown",
+				Category: "",
+				State:    "unavailable",
+				VCPUs:    2,
+				GPUs:     0,
+				RAM:      7,
+			},
+		}, nil,
+	)
+	manager.Client = client
+
+	return manager
+}
+
+func TestOvhCloudManager_getFlavorsByName(t *testing.T) {
+	expectedFlavorsByNameFromAPICall := map[string]sdk.Flavor{
+		"b2-7": {
+			Name:     "b2-7",
+			Category: "b",
+			State:    "available",
+			VCPUs:    2,
+			GPUs:     0,
+			RAM:      7,
+		},
+		"t1-45": {
+			Name:     "t1-45",
+			Category: "t",
+			State:    "available",
+			VCPUs:    8,
+			GPUs:     1,
+			RAM:      45,
+		},
+		"unknown": {
+			Name:     "unknown",
+			Category: "",
+			State:    "unavailable",
+			VCPUs:    2,
+			GPUs:     0,
+			RAM:      7,
+		},
+	}
+
+	t.Run("brand new manager: list from api", func(t *testing.T) {
+		ng := newTestManager(t)
+		flavorsByName, err := ng.getFlavorsByName()
+
+		ng.Client.(*sdk.ClientMock).AssertCalled(t, "ListClusterFlavors", context.Background(), "projectID", "clusterID")
+		assert.NoError(t, err)
+		assert.Equal(t, expectedFlavorsByNameFromAPICall, flavorsByName)
+		assert.Equal(t, expectedFlavorsByNameFromAPICall, ng.FlavorsCache)
+	})
+
+	t.Run("flavors cache expired: renew and list from api", func(t *testing.T) {
+		initialFlavorsCache := map[string]sdk.Flavor{
+			"custom": {
+				Name: "custom",
+			},
+		}
+
+		ng := newTestManager(t)
+		ng.FlavorsCache = initialFlavorsCache
+		ng.FlavorsCacheExpirationTime = time.Now()
+
+		flavorsByName, err := ng.getFlavorsByName()
+
+		ng.Client.(*sdk.ClientMock).AssertCalled(t, "ListClusterFlavors", context.Background(), "projectID", "clusterID")
+		assert.NoError(t, err)
+		assert.Equal(t, expectedFlavorsByNameFromAPICall, flavorsByName)
+		assert.Equal(t, expectedFlavorsByNameFromAPICall, ng.FlavorsCache)
+	})
+
+	t.Run("flavors cache still valid: list from cache", func(t *testing.T) {
+		initialFlavorsCache := map[string]sdk.Flavor{
+			"custom": {
+				Name: "custom",
+			},
+		}
+
+		ng := newTestManager(t)
+		ng.FlavorsCache = initialFlavorsCache
+		ng.FlavorsCacheExpirationTime = time.Now().Add(time.Minute)
+
+		flavorsByName, err := ng.getFlavorsByName()
+
+		ng.Client.(*sdk.ClientMock).AssertNotCalled(t, "ListClusterFlavors", context.Background(), "projectID", "clusterID")
+		assert.NoError(t, err)
+		assert.Equal(t, initialFlavorsCache, flavorsByName)
+		assert.Equal(t, initialFlavorsCache, ng.FlavorsCache)
+	})
+}
+
+func TestOvhCloudManager_getFlavorByName(t *testing.T) {
+	ng := newTestManager(t)
+
+	t.Run("check default node group max size", func(t *testing.T) {
+		flavor, err := ng.getFlavorByName("b2-7")
+		assert.NoError(t, err)
+		assert.Equal(t, sdk.Flavor{
+			Name:     "b2-7",
+			Category: "b",
+			State:    "available",
+			VCPUs:    2,
+			GPUs:     0,
+			RAM:      7,
+		}, flavor)
+	})
+}