fix(exoscale): enhance UX around SKS nodepools

In the 1.31.0 and lower version SKS nodepools where discovered through exoscale instance pools.
Whereas it works as expected, end user experience is not very convenient, especially when end users use priority based expander.
On our portal SKS nodepools are shown with their ID and the instancepool is hidden in the SKS nodepool page itself. People trying to
use priority based expander put SKS nodepool ID instead of our generic compute API InstancePool ID.
For convenience, starting with this commit we are using SKS nodepool ID and we add retro compatibility for already cluster autoscaler
managed SKS clusters. This will reduce a bit our support on it, and make priority based expander UX more convenient.

Author: nerzhul

.gitignore

@@ -16,6 +16,7 @@
 # VSCode project files
 **/.vscode
 *.code-workspace
+__debug_bin*
 
 # Emacs save files
 *~

cluster-autoscaler/cloudprovider/exoscale/exoscale_cloud_provider_test.go

@@ -197,6 +197,21 @@ func (ts *cloudProviderTestSuite) TestExoscaleCloudProvider_NodeGroupForNode_Ins
 			nil,
 		)
 
+	ts.p.manager.client.(*exoscaleClientMock).
+		On("ListSKSClusters", ts.p.manager.ctx, ts.p.manager.zone).
+		Return(
+			[]*egoscale.SKSCluster{{
+				ID:   &testSKSClusterID,
+				Name: &testSKSClusterName,
+				Nodepools: []*egoscale.SKSNodepool{{
+					ID:             &testSKSNodepoolID,
+					InstancePoolID: &testInstancePoolID,
+					Name:           &testSKSNodepoolName,
+				}},
+			}},
+			nil,
+		)
+
 	nodeGroup, err := ts.p.NodeGroupForNode(&apiv1.Node{
 		Spec: apiv1.NodeSpec{
 			ProviderID: toProviderID(testInstanceID),
@@ -280,7 +295,7 @@ func (ts *cloudProviderTestSuite) TestExoscaleCloudProvider_NodeGroupForNode_SKS
 	})
 	ts.Require().NoError(err)
 	ts.Require().NotNil(nodeGroup)
-	ts.Require().Equal(testInstancePoolID, nodeGroup.Id())
+	ts.Require().Equal(testSKSNodepoolID, nodeGroup.Id())
 	ts.Require().IsType(&sksNodepoolNodeGroup{}, nodeGroup)
 }
 
@@ -359,6 +374,21 @@ func (ts *cloudProviderTestSuite) TestExoscaleCloudProvider_NodeGroups() {
 			nil,
 		)
 
+	ts.p.manager.client.(*exoscaleClientMock).
+		On("ListSKSClusters", ts.p.manager.ctx, ts.p.manager.zone).
+		Return(
+			[]*egoscale.SKSCluster{{
+				ID:   &testSKSClusterID,
+				Name: &testSKSClusterName,
+				Nodepools: []*egoscale.SKSNodepool{{
+					ID:             &testSKSNodepoolID,
+					InstancePoolID: &sksNodepoolInstancePoolID,
+					Name:           &testSKSNodepoolName,
+				}},
+			}},
+			nil,
+		)
+
 	instancePoolNodeGroup, err := ts.p.NodeGroupForNode(&apiv1.Node{
 		Spec: apiv1.NodeSpec{
 			ProviderID: toProviderID(instancePoolInstanceID),
@@ -432,7 +462,11 @@ func (ts *cloudProviderTestSuite) TestExoscaleCloudProvider_NodeGroups() {
 
 	// ---------------------------------------------------------------
 
-	ts.Require().Len(ts.p.NodeGroups(), 2)
+	nodeGroups := ts.p.NodeGroups()
+	ts.Require().Len(nodeGroups, 2)
+	if len(nodeGroups) >= 2 {
+		ts.Require().Equal(nodeGroups[1].Id(), testSKSNodepoolID)
+	}
 }
 
 func TestSuiteExoscaleCloudProvider(t *testing.T) {

cluster-autoscaler/cloudprovider/exoscale/exoscale_manager.go

@@ -96,14 +96,46 @@ func newManager(discoveryOpts cloudprovider.NodeGroupDiscoveryOptions) (*Manager
 // based on the `--scan-interval`. By default it's 10 seconds.
 func (m *Manager) Refresh() error {
 	var nodeGroups []cloudprovider.NodeGroup
+
+	// load clusters, it's required for SKS node groups check
+	sksClusters, err := m.client.ListSKSClusters(m.ctx, m.zone)
+	if err != nil {
+		errorf("unable to list SKS clusters: %v", err)
+		return err
+	}
+
 	for _, ng := range m.nodeGroups {
-		if _, err := m.client.GetInstancePool(m.ctx, m.zone, ng.Id()); err != nil {
-			if errors.Is(err, exoapi.ErrNotFound) {
+		// Check SKS Nodepool existence first
+		found := false
+		for _, c := range sksClusters {
+			for _, np := range c.Nodepools {
+				if *np.ID == ng.Id() {
+					if _, err := m.client.GetInstancePool(m.ctx, m.zone, *np.InstancePoolID); err != nil {
+						if !errors.Is(err, exoapi.ErrNotFound) {
+							errorf("unable to retrieve SKS Instance Pool %s: %v", ng.Id(), err)
+							return err
+						}
+					} else {
+						found = true
+						break
+					}
+				}
+			}
+		}
+
+		if !found {
+			// If SKS Nodepool is not found, check the Instance Pool
+			// it was the previous behavior which was less convenient for end user UX
+			if _, err := m.client.GetInstancePool(m.ctx, m.zone, ng.Id()); err != nil {
+				if !errors.Is(err, exoapi.ErrNotFound) {
+					errorf("unable to retrieve SKS Instance Pool %s: %v", ng.Id(), err)
+					return err
+				}
+
+				// Neither SKS Nodepool nor Instance Pool found, remove it from cache
 				debugf("removing node group %s from manager cache", ng.Id())
 				continue
 			}
-			errorf("unable to retrieve Instance Pool %s: %v", ng.Id(), err)
-			return err
 		}
 
 		nodeGroups = append(nodeGroups, ng)

cluster-autoscaler/cloudprovider/exoscale/exoscale_node_group_instance_pool.go

@@ -212,9 +212,9 @@ func (n *instancePoolNodeGroup) GetOptions(_ config.NodeGroupAutoscalingOptions)
 
 func (n *instancePoolNodeGroup) waitUntilRunning(ctx context.Context) error {
 	return pollCmd(ctx, func() (bool, error) {
-		instancePool, err := n.m.client.GetInstancePool(ctx, n.m.zone, n.Id())
+		instancePool, err := n.m.client.GetInstancePool(ctx, n.m.zone, *n.instancePool.ID)
 		if err != nil {
-			errorf("unable to retrieve Instance Pool %s: %s", n.Id(), err)
+			errorf("unable to retrieve Instance Pool %s: %s", *n.instancePool.ID, err)
 			return false, err
 		}
 

cluster-autoscaler/cloudprovider/exoscale/exoscale_node_group_sks_nodepool.go

@@ -151,7 +151,7 @@ func (n *sksNodepoolNodeGroup) DecreaseTargetSize(_ int) error {
 
 // Id returns an unique identifier of the node group.
 func (n *sksNodepoolNodeGroup) Id() string {
-	return *n.sksNodepool.InstancePoolID
+	return *n.sksNodepool.ID
 }
 
 // Debug returns a string containing all information regarding this node group.
@@ -229,9 +229,9 @@ func (n *sksNodepoolNodeGroup) GetOptions(_ config.NodeGroupAutoscalingOptions)
 
 func (n *sksNodepoolNodeGroup) waitUntilRunning(ctx context.Context) error {
 	return pollCmd(ctx, func() (bool, error) {
-		instancePool, err := n.m.client.GetInstancePool(ctx, n.m.zone, n.Id())
+		instancePool, err := n.m.client.GetInstancePool(ctx, n.m.zone, *n.sksNodepool.InstancePoolID)
 		if err != nil {
-			errorf("unable to retrieve Instance Pool %s: %s", n.Id(), err)
+			errorf("unable to retrieve Instance Pool %s: %s", *n.sksNodepool.InstancePoolID, err)
 			return false, err
 		}
 

cluster-autoscaler/cloudprovider/exoscale/exoscale_node_group_sks_nodepool_test.go

@@ -206,7 +206,7 @@ func (ts *cloudProviderTestSuite) TestSKSNodepoolNodeGroup_Id() {
 		maxSize: int(testComputeInstanceQuotaLimit),
 	}
 
-	ts.Require().Equal(testInstancePoolID, nodeGroup.Id())
+	ts.Require().Equal(testSKSNodepoolID, nodeGroup.Id())
 }
 
 func (ts *cloudProviderTestSuite) TestSKSNodepoolNodeGroup_Nodes() {