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 0f5f9a1
Show file tree
Hide file tree
Showing 11 changed files with 785 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
173 changes: 173 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,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)
})
}
Loading

0 comments on commit 0f5f9a1

Please sign in to comment.