Skip to content

Commit

Permalink
fix(ovhcloud): Various fixes
Browse files Browse the repository at this point in the history
Fix silent failure of upscale/downscale due to the node group status being not READY
Fix Instance.ID to the actual value of Node.Spec.ProviderID
Fix infinite looping when DecreaseTargetSize is called
Fix the way we determine to which node group a node belongs to
Fix handling of non-autoscaled nodepools (now considered as autoscaled nodepools with no margin of action)
Fix TemplateNodeInfo with correct allocatable resources from the instance flavor specs

Signed-off-by: Xavier Duthil <[email protected]>
  • Loading branch information
XavierDuthil committed May 10, 2022
1 parent a7f1984 commit 2f90257
Show file tree
Hide file tree
Showing 11 changed files with 769 additions and 335 deletions.
58 changes: 54 additions & 4 deletions cluster-autoscaler/cloudprovider/ovhcloud/ovh_cloud_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
157 changes: 157 additions & 0 deletions cluster-autoscaler/cloudprovider/ovhcloud/ovh_cloud_manager_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
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)
})
}
Loading

0 comments on commit 2f90257

Please sign in to comment.