From 776d7311a1f6d51c619ff03cb8680ae14476caf6 Mon Sep 17 00:00:00 2001 From: Clint Fooken Date: Wed, 27 Jul 2022 11:20:17 -0700 Subject: [PATCH 1/9] Adding support for identifying nodes that have been deleted from cloud provider that are still registered within Kubernetes. Avoids misidentifying not autoscaled nodes as deleted. Simplified implementation to use apiv1.Node instead of new struct. Expanded test cases to include not autoscaled nodes and tracking deleted nodes over multiple updates. Adding check to backfill loop to confirm cloud provider node no longer exists before flagging the node as deleted. Modifying some comments to be more accurate. Replacing erroneous line deletion. --- .../cloudprovider/test/test_cloud_provider.go | 8 ++ .../clusterstate/clusterstate.go | 72 +++++++++++- .../clusterstate/clusterstate_test.go | 111 +++++++++++++++++- 3 files changed, 188 insertions(+), 3 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/test/test_cloud_provider.go b/cluster-autoscaler/cloudprovider/test/test_cloud_provider.go index 234d6853e557..af58f9e639a2 100644 --- a/cluster-autoscaler/cloudprovider/test/test_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/test/test_cloud_provider.go @@ -253,6 +253,14 @@ func (tcp *TestCloudProvider) AddNode(nodeGroupId string, node *apiv1.Node) { tcp.nodes[node.Name] = nodeGroupId } +// DeleteNode delete the given node from the provider. +func (tcp *TestCloudProvider) DeleteNode(node *apiv1.Node) { + tcp.Lock() + defer tcp.Unlock() + + delete(tcp.nodes, node.Name) +} + // GetResourceLimiter returns struct containing limits (max, min) for resources (cores, memory etc.). func (tcp *TestCloudProvider) GetResourceLimiter() (*cloudprovider.ResourceLimiter, error) { return tcp.resourceLimiter, nil diff --git a/cluster-autoscaler/clusterstate/clusterstate.go b/cluster-autoscaler/clusterstate/clusterstate.go index 3c4a52bdce28..ae89557c6a57 100644 --- a/cluster-autoscaler/clusterstate/clusterstate.go +++ b/cluster-autoscaler/clusterstate/clusterstate.go @@ -28,7 +28,6 @@ import ( "k8s.io/autoscaler/cluster-autoscaler/clusterstate/utils" "k8s.io/autoscaler/cluster-autoscaler/metrics" "k8s.io/autoscaler/cluster-autoscaler/utils/backoff" - "k8s.io/autoscaler/cluster-autoscaler/utils/deletetaint" kube_util "k8s.io/autoscaler/cluster-autoscaler/utils/kubernetes" apiv1 "k8s.io/api/core/v1" @@ -121,6 +120,7 @@ type ClusterStateRegistry struct { acceptableRanges map[string]AcceptableRange incorrectNodeGroupSizes map[string]IncorrectNodeGroupSize unregisteredNodes map[string]UnregisteredNode + deletedNodes map[string]*apiv1.Node candidatesForScaleDown map[string][]string backoff backoff.Backoff lastStatus *api.ClusterAutoscalerStatus @@ -153,6 +153,7 @@ func NewClusterStateRegistry(cloudProvider cloudprovider.CloudProvider, config C acceptableRanges: make(map[string]AcceptableRange), incorrectNodeGroupSizes: make(map[string]IncorrectNodeGroupSize), unregisteredNodes: make(map[string]UnregisteredNode), + deletedNodes: make(map[string]*apiv1.Node), candidatesForScaleDown: make(map[string][]string), backoff: backoff, lastStatus: emptyStatus, @@ -292,6 +293,7 @@ func (csr *ClusterStateRegistry) UpdateNodes(nodes []*apiv1.Node, nodeInfosForGr } cloudProviderNodeInstances, err := csr.getCloudProviderNodeInstances() + cloudProviderNodesRemoved := csr.getCloudProviderDeletedNodes(nodes, cloudProviderNodeInstances) if err != nil { return err } @@ -306,6 +308,7 @@ func (csr *ClusterStateRegistry) UpdateNodes(nodes []*apiv1.Node, nodeInfosForGr csr.cloudProviderNodeInstances = cloudProviderNodeInstances csr.updateUnregisteredNodes(notRegistered) + csr.updateCloudProviderDeletedNodes(cloudProviderNodesRemoved) csr.updateReadinessStats(currentTime) // update acceptable ranges based on requests from last loop and targetSizes @@ -541,7 +544,7 @@ func (csr *ClusterStateRegistry) updateReadinessStats(currentTime time.Time) { update := func(current Readiness, node *apiv1.Node, nr kube_util.NodeReadiness) Readiness { current.Registered++ - if deletetaint.HasToBeDeletedTaint(node) { + if _, exists := csr.deletedNodes[node.Name]; exists { current.Deleted++ } else if nr.Ready { current.Ready++ @@ -669,6 +672,30 @@ func (csr *ClusterStateRegistry) GetUnregisteredNodes() []UnregisteredNode { return result } +func (csr *ClusterStateRegistry) updateCloudProviderDeletedNodes(deletedNodes []*apiv1.Node) { + result := make(map[string]*apiv1.Node) + for _, deleted := range deletedNodes { + if prev, found := csr.deletedNodes[deleted.Name]; found { + result[deleted.Name] = prev + } else { + result[deleted.Name] = deleted + } + } + csr.deletedNodes = result +} + +//GetCloudProviderDeletedNodes returns a list of all nodes removed from cloud provider but registered in Kubernetes. +func (csr *ClusterStateRegistry) GetCloudProviderDeletedNodes() []*apiv1.Node { + csr.Lock() + defer csr.Unlock() + + result := make([]*apiv1.Node, 0, len(csr.deletedNodes)) + for _, deleted := range csr.deletedNodes { + result = append(result, deleted) + } + return result +} + // UpdateScaleDownCandidates updates scale down candidates func (csr *ClusterStateRegistry) UpdateScaleDownCandidates(nodes []*apiv1.Node, now time.Time) { result := make(map[string][]string) @@ -958,6 +985,47 @@ func getNotRegisteredNodes(allNodes []*apiv1.Node, cloudProviderNodeInstances ma return notRegistered } +// Calculates which of the registered nodes in Kubernetes that do not exist in cloud provider. +func (csr *ClusterStateRegistry) getCloudProviderDeletedNodes(allNodes []*apiv1.Node, cloudProviderNodeInstances map[string][]cloudprovider.Instance) []*apiv1.Node { + nodesRemoved := make([]*apiv1.Node, 0) + currentCloudInstances := make(map[string]string, 0) + registeredNodes := make(map[string]*apiv1.Node, 0) + for nodeGroupName, instances := range cloudProviderNodeInstances { + for _, instance := range instances { + currentCloudInstances[instance.Id] = nodeGroupName + } + } + for _, node := range allNodes { + registeredNodes[node.Name] = node + } + + // Fill previously deleted nodes, if they are still registered in Kubernetes + for nodeName, node := range csr.deletedNodes { + // Safety check to prevent flagging Kubernetes nodes as deleted + // if the Cloud Provider instance is re-discovered + _, cloudProviderFound := currentCloudInstances[node.Name] + if _, found := registeredNodes[nodeName]; found && !cloudProviderFound { + nodesRemoved = append(nodesRemoved, node) + } + } + + // Seek nodes that may have been deleted since last update + // cloudProviderNodeInstances are retrieved by nodeGroup, + // not autoscaled nodes will be excluded + for _, instances := range csr.cloudProviderNodeInstances { + for _, instance := range instances { + if _, found := currentCloudInstances[instance.Id]; !found { + // Check Kubernetes registered nodes for corresponding deleted + // Cloud Provider instance + if kubeNode, kubeNodeFound := registeredNodes[instance.Id]; kubeNodeFound { + nodesRemoved = append(nodesRemoved, kubeNode) + } + } + } + } + return nodesRemoved +} + // GetAutoscaledNodesCount calculates and returns the actual and the target number of nodes // belonging to autoscaled node groups in the cluster. func (csr *ClusterStateRegistry) GetAutoscaledNodesCount() (currentSize, targetSize int) { diff --git a/cluster-autoscaler/clusterstate/clusterstate_test.go b/cluster-autoscaler/clusterstate/clusterstate_test.go index 65d3bc43c5f5..3b7f5b663572 100644 --- a/cluster-autoscaler/clusterstate/clusterstate_test.go +++ b/cluster-autoscaler/clusterstate/clusterstate_test.go @@ -17,6 +17,8 @@ limitations under the License. package clusterstate import ( + "fmt" + "k8s.io/autoscaler/cluster-autoscaler/utils/deletetaint" "testing" "time" @@ -481,6 +483,22 @@ func TestUpcomingNodes(t *testing.T) { provider.AddNodeGroup("ng4", 1, 10, 1) provider.AddNode("ng4", ng4_1) + // One node is already there, for a second nde deletion / draining was already started. + ng5_1 := BuildTestNode("ng5-1", 1000, 1000) + SetNodeReadyState(ng5_1, true, now.Add(-time.Minute)) + ng5_2 := BuildTestNode("ng5-2", 1000, 1000) + SetNodeReadyState(ng5_2, true, now.Add(-time.Minute)) + ng5_2.Spec.Taints = []apiv1.Taint{ + { + Key: deletetaint.ToBeDeletedTaint, + Value: fmt.Sprint(time.Now().Unix()), + Effect: apiv1.TaintEffectNoSchedule, + }, + } + provider.AddNodeGroup("ng5", 1, 10, 2) + provider.AddNode("ng5", ng5_1) + provider.AddNode("ng5", ng5_2) + assert.NotNil(t, provider) fakeClient := &fake.Clientset{} fakeLogRecorder, _ := utils.NewStatusMapRecorder(fakeClient, "kube-system", kube_record.NewFakeRecorder(5), false, "my-cool-configmap") @@ -488,7 +506,7 @@ func TestUpcomingNodes(t *testing.T) { MaxTotalUnreadyPercentage: 10, OkTotalUnreadyCount: 1, }, fakeLogRecorder, newBackoff()) - err := clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, ng2_1, ng3_1, ng4_1}, nil, now) + err := clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, ng2_1, ng3_1, ng4_1, ng5_1, ng5_2}, nil, now) assert.NoError(t, err) assert.Empty(t, clusterstate.GetScaleUpFailures()) @@ -497,6 +515,7 @@ func TestUpcomingNodes(t *testing.T) { assert.Equal(t, 1, upcomingNodes["ng2"]) assert.Equal(t, 2, upcomingNodes["ng3"]) assert.NotContains(t, upcomingNodes, "ng4") + assert.NotContains(t, upcomingNodes, "ng5") } func TestIncorrectSize(t *testing.T) { @@ -570,6 +589,96 @@ func TestUnregisteredNodes(t *testing.T) { assert.Equal(t, 0, len(clusterstate.GetUnregisteredNodes())) } +func TestCloudProviderDeletedNodes(t *testing.T) { + now := time.Now() + ng1_1 := BuildTestNode("ng1-1", 1000, 1000) + SetNodeReadyState(ng1_1, true, now.Add(-time.Minute)) + ng1_1.Spec.ProviderID = "ng1-1" + ng1_2 := BuildTestNode("ng1-2", 1000, 1000) + SetNodeReadyState(ng1_2, true, now.Add(-time.Minute)) + ng1_2.Spec.ProviderID = "ng1-2" + // No Node Group - Not Autoscaled Node + noNg := BuildTestNode("no-ng", 1000, 1000) + SetNodeReadyState(noNg, true, now.Add(-time.Minute)) + noNg.Spec.ProviderID = "no-ng" + provider := testprovider.NewTestCloudProvider(nil, nil) + provider.AddNodeGroup("ng1", 1, 10, 2) + provider.AddNode("ng1", ng1_1) + provider.AddNode("ng1", ng1_2) + + fakeClient := &fake.Clientset{} + fakeLogRecorder, _ := utils.NewStatusMapRecorder(fakeClient, "kube-system", kube_record.NewFakeRecorder(5), false, "my-cool-configmap") + clusterstate := NewClusterStateRegistry(provider, ClusterStateRegistryConfig{ + MaxTotalUnreadyPercentage: 10, + OkTotalUnreadyCount: 1, + MaxNodeProvisionTime: 10 * time.Second, + }, fakeLogRecorder, newBackoff()) + now.Add(time.Minute) + err := clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, ng1_2, noNg}, nil, now) + + // Nodes are registered correctly between Kubernetes and cloud provider. + assert.NoError(t, err) + assert.Equal(t, 0, len(clusterstate.GetCloudProviderDeletedNodes())) + + // The node was removed from Cloud Provider + // should be counted as Deleted by cluster state + nodeGroup, err := provider.NodeGroupForNode(ng1_2) + assert.NoError(t, err) + provider.DeleteNode(ng1_2) + clusterstate.InvalidateNodeInstancesCacheEntry(nodeGroup) + now.Add(time.Minute) + err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, ng1_2, noNg}, nil, now) + assert.NoError(t, err) + assert.Equal(t, 1, len(clusterstate.GetCloudProviderDeletedNodes())) + assert.Equal(t, "ng1-2", clusterstate.GetCloudProviderDeletedNodes()[0].Name) + assert.Equal(t, 1, clusterstate.GetClusterReadiness().Deleted) + + // The node is removed from Kubernetes + now.Add(time.Minute) + err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNg}, nil, now) + assert.NoError(t, err) + assert.Equal(t, 0, len(clusterstate.GetCloudProviderDeletedNodes())) + + // New Node is added afterwards + ng1_3 := BuildTestNode("ng1-3", 1000, 1000) + SetNodeReadyState(ng1_3, true, now.Add(-time.Minute)) + ng1_3.Spec.ProviderID = "ng1-3" + provider.AddNode("ng1", ng1_3) + clusterstate.InvalidateNodeInstancesCacheEntry(nodeGroup) + now.Add(time.Minute) + err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, ng1_3, noNg}, nil, now) + assert.NoError(t, err) + assert.Equal(t, 0, len(clusterstate.GetCloudProviderDeletedNodes())) + + // Newly added node is removed from Cloud Provider + // should be counted as Deleted by cluster state + nodeGroup, err = provider.NodeGroupForNode(ng1_3) + assert.NoError(t, err) + provider.DeleteNode(ng1_3) + clusterstate.InvalidateNodeInstancesCacheEntry(nodeGroup) + now.Add(time.Minute) + err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNg, ng1_3}, nil, now) + assert.NoError(t, err) + assert.Equal(t, 1, len(clusterstate.GetCloudProviderDeletedNodes())) + assert.Equal(t, "ng1-3", clusterstate.GetCloudProviderDeletedNodes()[0].Name) + assert.Equal(t, 1, clusterstate.GetClusterReadiness().Deleted) + + // Confirm that previously identified deleted Cloud Provider nodes are still included + // until it is removed from Kubernetes + now.Add(time.Minute) + err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNg, ng1_3}, nil, now) + assert.NoError(t, err) + assert.Equal(t, 1, len(clusterstate.GetCloudProviderDeletedNodes())) + assert.Equal(t, "ng1-3", clusterstate.GetCloudProviderDeletedNodes()[0].Name) + assert.Equal(t, 1, clusterstate.GetClusterReadiness().Deleted) + + // The node is removed from Kubernetes + now.Add(time.Minute) + err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNg}, nil, now) + assert.NoError(t, err) + assert.Equal(t, 0, len(clusterstate.GetCloudProviderDeletedNodes())) +} + func TestUpdateLastTransitionTimes(t *testing.T) { now := metav1.Time{Time: time.Now()} later := metav1.Time{Time: now.Time.Add(10 * time.Second)} From cf67a3004ecfb9437ef04bcabbfc7ec847854ef2 Mon Sep 17 00:00:00 2001 From: Clint <103211936+fookenc@users.noreply.github.com> Date: Mon, 17 Oct 2022 14:58:38 -0700 Subject: [PATCH 2/9] Implementing new cloud provider method for node deletion detection (#1) * Adding isNodeDeleted method to CloudProvider interface. Supports detecting whether nodes are fully deleted or are not-autoscaled. Updated cloud providers to provide initial implementation of new method that will return an ErrNotImplemented to maintain existing taint-based deletion clusterstate calculation. --- .../alicloud/alicloud_cloud_provider.go | 5 ++ .../cloudprovider/aws/aws_cloud_provider.go | 5 ++ .../azure/azure_cloud_provider.go | 5 ++ .../baiducloud/baiducloud_cloud_provider.go | 5 ++ .../bizflycloud/bizflycloud_cloud_provider.go | 5 ++ .../brightbox/brightbox_cloud_provider.go | 5 ++ .../cherryservers/cherry_cloud_provider.go | 5 ++ .../cloudprovider/civo/civo_cloud_provider.go | 5 ++ .../cloudprovider/cloud_provider.go | 4 + .../cloudstack/cloudstack_cloud_provider.go | 5 ++ .../clusterapi/clusterapi_provider.go | 5 ++ .../digitalocean_cloud_provider.go | 5 ++ .../exoscale/exoscale_cloud_provider.go | 5 ++ .../externalgrpc_cloud_provider.go | 5 ++ .../cloudprovider/gce/gce_cloud_provider.go | 5 ++ .../hetzner/hetzner_cloud_provider.go | 5 ++ .../huaweicloud/huaweicloud_cloud_provider.go | 5 ++ .../ionoscloud/ionoscloud_cloud_provider.go | 5 ++ .../kamatera/kamatera_cloud_provider.go | 5 ++ .../cloudprovider/kubemark/kubemark_linux.go | 5 ++ .../cloudprovider/kubemark/kubemark_other.go | 5 ++ .../linode/linode_cloud_provider.go | 5 ++ .../magnum/magnum_cloud_provider.go | 5 ++ .../cloudprovider/mocks/CloudProvider.go | 23 +++++ .../cloudprovider/oci/oci_cloud_provider.go | 5 ++ .../ovhcloud/ovh_cloud_provider.go | 5 ++ .../packet/packet_cloud_provider.go | 5 ++ .../cloudprovider/rancher/rancher_provider.go | 5 ++ .../scaleway/scaleway_cloud_provider.go | 5 ++ .../tencentcloud_cloud_provider.go | 5 ++ .../cloudprovider/test/test_cloud_provider.go | 30 +++++++ .../vultr/vultr_cloud_provider.go | 5 ++ .../clusterstate/clusterstate.go | 84 ++++++++++++------- .../clusterstate/clusterstate_test.go | 71 +++++++++++++--- .../core/static_autoscaler_test.go | 8 ++ 35 files changed, 324 insertions(+), 41 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/alicloud/alicloud_cloud_provider.go b/cluster-autoscaler/cloudprovider/alicloud/alicloud_cloud_provider.go index c3aa703cf6b4..bd406cf108aa 100644 --- a/cluster-autoscaler/cloudprovider/alicloud/alicloud_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/alicloud/alicloud_cloud_provider.go @@ -127,6 +127,11 @@ func (ali *aliCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovider.N return ali.manager.GetAsgForInstance(instanceId) } +// NodeExists returns whether node exists in this cloud provider +func (ali *aliCloudProvider) NodeExists(*apiv1.Node) (bool, error) { + return true, cloudprovider.ErrNotImplemented +} + // Pricing returns pricing model for this cloud provider or error if not available. func (ali *aliCloudProvider) Pricing() (cloudprovider.PricingModel, errors.AutoscalerError) { return nil, cloudprovider.ErrNotImplemented diff --git a/cluster-autoscaler/cloudprovider/aws/aws_cloud_provider.go b/cluster-autoscaler/cloudprovider/aws/aws_cloud_provider.go index 339c51be90d4..e5e8e813f044 100644 --- a/cluster-autoscaler/cloudprovider/aws/aws_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/aws/aws_cloud_provider.go @@ -120,6 +120,11 @@ func (aws *awsCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovider.N }, nil } +// NodeExists returns whether node exists in this cloud provider +func (aws *awsCloudProvider) NodeExists(*apiv1.Node) (bool, error) { + return true, cloudprovider.ErrNotImplemented +} + // Pricing returns pricing model for this cloud provider or error if not available. func (aws *awsCloudProvider) Pricing() (cloudprovider.PricingModel, errors.AutoscalerError) { return nil, cloudprovider.ErrNotImplemented diff --git a/cluster-autoscaler/cloudprovider/azure/azure_cloud_provider.go b/cluster-autoscaler/cloudprovider/azure/azure_cloud_provider.go index 491c42023448..0c0142f9d509 100644 --- a/cluster-autoscaler/cloudprovider/azure/azure_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/azure/azure_cloud_provider.go @@ -106,6 +106,11 @@ func (azure *AzureCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovid return azure.azureManager.GetNodeGroupForInstance(ref) } +// NodeExists returns whether node exists in this cloud provider +func (azure *AzureCloudProvider) NodeExists(*apiv1.Node) (bool, error) { + return false, cloudprovider.ErrNotImplemented +} + // Pricing returns pricing model for this cloud provider or error if not available. func (azure *AzureCloudProvider) Pricing() (cloudprovider.PricingModel, errors.AutoscalerError) { return nil, cloudprovider.ErrNotImplemented diff --git a/cluster-autoscaler/cloudprovider/baiducloud/baiducloud_cloud_provider.go b/cluster-autoscaler/cloudprovider/baiducloud/baiducloud_cloud_provider.go index 1dc695f46e63..0c7d97f1a510 100644 --- a/cluster-autoscaler/cloudprovider/baiducloud/baiducloud_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/baiducloud/baiducloud_cloud_provider.go @@ -180,6 +180,11 @@ func (baiducloud *baiducloudCloudProvider) NodeGroupForNode(node *apiv1.Node) (c return asg, err } +// NodeExists returns whether node exists in this cloud provider +func (baiducloud *baiducloudCloudProvider) NodeExists(*apiv1.Node) (bool, error) { + return false, cloudprovider.ErrNotImplemented +} + // Pricing returns pricing model for this cloud provider or error if not available. // Implementation optional. func (baiducloud *baiducloudCloudProvider) Pricing() (cloudprovider.PricingModel, errors.AutoscalerError) { diff --git a/cluster-autoscaler/cloudprovider/bizflycloud/bizflycloud_cloud_provider.go b/cluster-autoscaler/cloudprovider/bizflycloud/bizflycloud_cloud_provider.go index 1a52f18295ef..ae1705d77454 100644 --- a/cluster-autoscaler/cloudprovider/bizflycloud/bizflycloud_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/bizflycloud/bizflycloud_cloud_provider.go @@ -104,6 +104,11 @@ func (d *bizflycloudCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprov return nil, nil } +// NodeExists returns whether node exists in this cloud provider +func (d *bizflycloudCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { + return false, cloudprovider.ErrNotImplemented +} + // Pricing returns pricing model for this cloud provider or error if not // available. Implementation optional. func (d *bizflycloudCloudProvider) Pricing() (cloudprovider.PricingModel, errors.AutoscalerError) { diff --git a/cluster-autoscaler/cloudprovider/brightbox/brightbox_cloud_provider.go b/cluster-autoscaler/cloudprovider/brightbox/brightbox_cloud_provider.go index f069f9019263..89c6ed83ce40 100644 --- a/cluster-autoscaler/cloudprovider/brightbox/brightbox_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/brightbox/brightbox_cloud_provider.go @@ -81,6 +81,11 @@ func (b *brightboxCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovid return nil, nil } +// NodeExists returns whether node exists in this cloud provider +func (b *brightboxCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { + return false, cloudprovider.ErrNotImplemented +} + // Refresh is before every main loop and can be used to dynamically // update cloud provider state. // In particular the list of node groups returned by NodeGroups can diff --git a/cluster-autoscaler/cloudprovider/cherryservers/cherry_cloud_provider.go b/cluster-autoscaler/cloudprovider/cherryservers/cherry_cloud_provider.go index b3f6b8888ce4..dce984be5a04 100644 --- a/cluster-autoscaler/cloudprovider/cherryservers/cherry_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/cherryservers/cherry_cloud_provider.go @@ -122,6 +122,11 @@ func (ccp *cherryCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovide return nil, nil } +// NodeExists returns whether node exists in this cloud provider +func (ccp *cherryCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { + return false, cloudprovider.ErrNotImplemented +} + // Pricing returns pricing model for this cloud provider or error if not available. func (ccp *cherryCloudProvider) Pricing() (cloudprovider.PricingModel, errors.AutoscalerError) { return nil, cloudprovider.ErrNotImplemented diff --git a/cluster-autoscaler/cloudprovider/civo/civo_cloud_provider.go b/cluster-autoscaler/cloudprovider/civo/civo_cloud_provider.go index 0c7245741394..c557a1609921 100644 --- a/cluster-autoscaler/cloudprovider/civo/civo_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/civo/civo_cloud_provider.go @@ -99,6 +99,11 @@ func (d *civoCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovider.No return nil, nil } +// NodeExists returns whether node exists in this cloud provider +func (d *civoCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { + return false, cloudprovider.ErrNotImplemented +} + // Pricing returns pricing model for this cloud provider or error if not // available. Implementation optional. func (d *civoCloudProvider) Pricing() (cloudprovider.PricingModel, errors.AutoscalerError) { diff --git a/cluster-autoscaler/cloudprovider/cloud_provider.go b/cluster-autoscaler/cloudprovider/cloud_provider.go index b35a7d5a8ed9..50fb0038f330 100644 --- a/cluster-autoscaler/cloudprovider/cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/cloud_provider.go @@ -100,6 +100,10 @@ type CloudProvider interface { // occurred. Must be implemented. NodeGroupForNode(*apiv1.Node) (NodeGroup, error) + // NodeExists returns whether the node exists in cloud provider, + // true if the node is available, false if it has been deleted + NodeExists(*apiv1.Node) (bool, error) + // Pricing returns pricing model for this cloud provider or error if not available. // Implementation optional. Pricing() (PricingModel, errors.AutoscalerError) diff --git a/cluster-autoscaler/cloudprovider/cloudstack/cloudstack_cloud_provider.go b/cluster-autoscaler/cloudprovider/cloudstack/cloudstack_cloud_provider.go index 63a228d8e365..1f0cd135fa81 100644 --- a/cluster-autoscaler/cloudprovider/cloudstack/cloudstack_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/cloudstack/cloudstack_cloud_provider.go @@ -68,6 +68,11 @@ func (provider *cloudStackCloudProvider) NodeGroupForNode(node *v1.Node) (cloudp return provider.manager.clusterForNode(node) } +// NodeExists returns whether node exists in this cloud provider +func (provider *cloudStackCloudProvider) NodeExists(node *v1.Node) (bool, error) { + return false, cloudprovider.ErrNotImplemented +} + // Cleanup cleans up open resources before the cloud provider is destroyed, i.e. go routines etc. func (provider *cloudStackCloudProvider) Cleanup() error { return provider.manager.cleanup() diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go index 290be55f5384..7dea49831b7d 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go @@ -81,6 +81,11 @@ func (p *provider) NodeGroupForNode(node *corev1.Node) (cloudprovider.NodeGroup, return ng, nil } +// NodeExists returns whether node exists in this cloud provider +func (p *provider) NodeExists(node *corev1.Node) (bool, error) { + return false, cloudprovider.ErrNotImplemented +} + func (*provider) Pricing() (cloudprovider.PricingModel, errors.AutoscalerError) { return nil, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/digitalocean/digitalocean_cloud_provider.go b/cluster-autoscaler/cloudprovider/digitalocean/digitalocean_cloud_provider.go index 36fb1e5e2e8a..f2bfa2d72c8b 100644 --- a/cluster-autoscaler/cloudprovider/digitalocean/digitalocean_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/digitalocean/digitalocean_cloud_provider.go @@ -101,6 +101,11 @@ func (d *digitaloceanCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudpro return nil, nil } +// NodeExists returns whether node exists in this cloud provider +func (d *digitaloceanCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { + return false, cloudprovider.ErrNotImplemented +} + // Pricing returns pricing model for this cloud provider or error if not // available. Implementation optional. func (d *digitaloceanCloudProvider) Pricing() (cloudprovider.PricingModel, errors.AutoscalerError) { diff --git a/cluster-autoscaler/cloudprovider/exoscale/exoscale_cloud_provider.go b/cluster-autoscaler/cloudprovider/exoscale/exoscale_cloud_provider.go index 82ed546896e8..2207a272e493 100644 --- a/cluster-autoscaler/cloudprovider/exoscale/exoscale_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/exoscale/exoscale_cloud_provider.go @@ -131,6 +131,11 @@ func (e *exoscaleCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovide return nodeGroup, nil } +// NodeExists returns whether node exists in this cloud provider +func (e *exoscaleCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { + return false, cloudprovider.ErrNotImplemented +} + // Pricing returns pricing model for this cloud provider or error if not available. // Implementation optional. func (e *exoscaleCloudProvider) Pricing() (cloudprovider.PricingModel, errors.AutoscalerError) { diff --git a/cluster-autoscaler/cloudprovider/externalgrpc/externalgrpc_cloud_provider.go b/cluster-autoscaler/cloudprovider/externalgrpc/externalgrpc_cloud_provider.go index d8743e918ba0..a953aea11d8c 100644 --- a/cluster-autoscaler/cloudprovider/externalgrpc/externalgrpc_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/externalgrpc/externalgrpc_cloud_provider.go @@ -134,6 +134,11 @@ func (e *externalGrpcCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudpro return ng, nil } +// NodeExists returns whether node exists in this cloud provider +func (e *externalGrpcCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { + return false, cloudprovider.ErrNotImplemented +} + // pricingModel implements cloudprovider.PricingModel interface. type pricingModel struct { client protos.CloudProviderClient diff --git a/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go b/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go index 39b62c50cabb..b80c88d26a72 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go @@ -101,6 +101,11 @@ func (gce *GceCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovider.N return mig, err } +// NodeExists returns whether node exists in this cloud provider +func (gce *GceCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { + return false, cloudprovider.ErrNotImplemented +} + // Pricing returns pricing model for this cloud provider or error if not available. func (gce *GceCloudProvider) Pricing() (cloudprovider.PricingModel, errors.AutoscalerError) { return gce.pricingModel, nil diff --git a/cluster-autoscaler/cloudprovider/hetzner/hetzner_cloud_provider.go b/cluster-autoscaler/cloudprovider/hetzner/hetzner_cloud_provider.go index 7e7f35bf5219..17deb4074dc7 100644 --- a/cluster-autoscaler/cloudprovider/hetzner/hetzner_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/hetzner/hetzner_cloud_provider.go @@ -99,6 +99,11 @@ func (d *HetznerCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovider return group, nil } +// NodeExists returns whether node exists in this cloud provider +func (d *HetznerCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { + return false, cloudprovider.ErrNotImplemented +} + // Pricing returns pricing model for this cloud provider or error if not // available. Implementation optional. func (d *HetznerCloudProvider) Pricing() (cloudprovider.PricingModel, errors.AutoscalerError) { diff --git a/cluster-autoscaler/cloudprovider/huaweicloud/huaweicloud_cloud_provider.go b/cluster-autoscaler/cloudprovider/huaweicloud/huaweicloud_cloud_provider.go index feff955034b1..0a61787dcc92 100644 --- a/cluster-autoscaler/cloudprovider/huaweicloud/huaweicloud_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/huaweicloud/huaweicloud_cloud_provider.go @@ -123,6 +123,11 @@ func (hcp *huaweicloudCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudpr return hcp.cloudServiceManager.GetAsgForInstance(instanceID) } +// NodeExists returns whether node exists in this cloud provider +func (hcp *huaweicloudCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { + return false, cloudprovider.ErrNotImplemented +} + // Pricing returns pricing model for this cloud provider or error if not available. Not implemented. func (hcp *huaweicloudCloudProvider) Pricing() (cloudprovider.PricingModel, errors.AutoscalerError) { return nil, cloudprovider.ErrNotImplemented diff --git a/cluster-autoscaler/cloudprovider/ionoscloud/ionoscloud_cloud_provider.go b/cluster-autoscaler/cloudprovider/ionoscloud/ionoscloud_cloud_provider.go index c6cd1617362d..352dc5ce36cf 100644 --- a/cluster-autoscaler/cloudprovider/ionoscloud/ionoscloud_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/ionoscloud/ionoscloud_cloud_provider.go @@ -232,6 +232,11 @@ func (ic *IonosCloudCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprov return nil, nil } +// NodeExists returns whether node exists in this cloud provider +func (ic *IonosCloudCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { + return false, cloudprovider.ErrNotImplemented +} + // Pricing returns pricing model for this cloud provider or error if not // available. Implementation optional. func (ic *IonosCloudCloudProvider) Pricing() (cloudprovider.PricingModel, errors.AutoscalerError) { diff --git a/cluster-autoscaler/cloudprovider/kamatera/kamatera_cloud_provider.go b/cluster-autoscaler/cloudprovider/kamatera/kamatera_cloud_provider.go index a77d7376a01d..64d7b20304a4 100644 --- a/cluster-autoscaler/cloudprovider/kamatera/kamatera_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/kamatera/kamatera_cloud_provider.go @@ -70,6 +70,11 @@ func (k *kamateraCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovide return nil, nil } +// NodeExists returns whether node exists in this cloud provider +func (k *kamateraCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { + return false, cloudprovider.ErrNotImplemented +} + // Pricing returns pricing model for this cloud provider or error if not available. // Implementation optional. func (k *kamateraCloudProvider) Pricing() (cloudprovider.PricingModel, errors.AutoscalerError) { diff --git a/cluster-autoscaler/cloudprovider/kubemark/kubemark_linux.go b/cluster-autoscaler/cloudprovider/kubemark/kubemark_linux.go index be0925837433..69c052586f42 100644 --- a/cluster-autoscaler/cloudprovider/kubemark/kubemark_linux.go +++ b/cluster-autoscaler/cloudprovider/kubemark/kubemark_linux.go @@ -139,6 +139,11 @@ func (kubemark *KubemarkCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloud return nil, nil } +// NodeExists returns whether node exists in this cloud provider +func (kubemark *KubemarkCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { + return false, cloudprovider.ErrNotImplemented +} + // GetAvailableMachineTypes get all machine types that can be requested from the cloud provider. // Implementation optional. func (kubemark *KubemarkCloudProvider) GetAvailableMachineTypes() ([]string, error) { diff --git a/cluster-autoscaler/cloudprovider/kubemark/kubemark_other.go b/cluster-autoscaler/cloudprovider/kubemark/kubemark_other.go index dc857e68c74c..45da92266228 100644 --- a/cluster-autoscaler/cloudprovider/kubemark/kubemark_other.go +++ b/cluster-autoscaler/cloudprovider/kubemark/kubemark_other.go @@ -80,6 +80,11 @@ func (kubemark *KubemarkCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloud return nil, cloudprovider.ErrNotImplemented } +// NodeExists returns whether node exists in this cloud provider +func (kubemark *KubemarkCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { + return true, cloudprovider.ErrNotImplemented +} + // GetAvailableMachineTypes get all machine types that can be requested from the cloud provider. // Implementation optional. func (kubemark *KubemarkCloudProvider) GetAvailableMachineTypes() ([]string, error) { diff --git a/cluster-autoscaler/cloudprovider/linode/linode_cloud_provider.go b/cluster-autoscaler/cloudprovider/linode/linode_cloud_provider.go index b14e97f151ee..8e234fa07901 100644 --- a/cluster-autoscaler/cloudprovider/linode/linode_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/linode/linode_cloud_provider.go @@ -67,6 +67,11 @@ func (l *linodeCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovider. return nil, nil } +// NodeExists returns whether node exists in this cloud provider +func (l *linodeCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { + return true, cloudprovider.ErrNotImplemented +} + // Pricing returns pricing model for this cloud provider or error if not available. // Implementation optional. func (l *linodeCloudProvider) Pricing() (cloudprovider.PricingModel, errors.AutoscalerError) { diff --git a/cluster-autoscaler/cloudprovider/magnum/magnum_cloud_provider.go b/cluster-autoscaler/cloudprovider/magnum/magnum_cloud_provider.go index c48f54560c8d..bb2c9ae53ae2 100644 --- a/cluster-autoscaler/cloudprovider/magnum/magnum_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/magnum/magnum_cloud_provider.go @@ -135,6 +135,11 @@ func (mcp *magnumCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovide return nil, nil } +// NodeExists returns whether node exists in this cloud provider +func (mcp *magnumCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { + return true, cloudprovider.ErrNotImplemented +} + // Pricing is not implemented. func (mcp *magnumCloudProvider) Pricing() (cloudprovider.PricingModel, errors.AutoscalerError) { return nil, cloudprovider.ErrNotImplemented diff --git a/cluster-autoscaler/cloudprovider/mocks/CloudProvider.go b/cluster-autoscaler/cloudprovider/mocks/CloudProvider.go index 6363f5d9d209..62a643bac834 100644 --- a/cluster-autoscaler/cloudprovider/mocks/CloudProvider.go +++ b/cluster-autoscaler/cloudprovider/mocks/CloudProvider.go @@ -177,6 +177,29 @@ func (_m *CloudProvider) NodeGroupForNode(_a0 *v1.Node) (cloudprovider.NodeGroup return r0, r1 } +// NodeExists provides a mock function with given fields: +func (_m *CloudProvider) NodeExists(_a0 *v1.Node) (bool, error) { + ret := _m.Called(_a0) + + var r0 bool + if rf, ok := ret.Get(0).(func(*v1.Node) bool); ok { + r0 = rf(_a0) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(bool) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(*v1.Node) error); ok { + r1 = rf(_a0) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // NodeGroups provides a mock function with given fields: func (_m *CloudProvider) NodeGroups() []cloudprovider.NodeGroup { ret := _m.Called() diff --git a/cluster-autoscaler/cloudprovider/oci/oci_cloud_provider.go b/cluster-autoscaler/cloudprovider/oci/oci_cloud_provider.go index 7eb9fbe704d3..a2430513c874 100644 --- a/cluster-autoscaler/cloudprovider/oci/oci_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/oci/oci_cloud_provider.go @@ -96,6 +96,11 @@ func (ocp *OciCloudProvider) NodeGroupForNode(n *apiv1.Node) (cloudprovider.Node return ng, err } +// NodeExists returns whether node exists in this cloud provider +func (ocp *OciCloudProvider) NodeExists(n *apiv1.Node) (bool, error) { + return true, cloudprovider.ErrNotImplemented +} + // Pricing returns pricing model for this cloud provider or error if not available. // Implementation optional. func (ocp *OciCloudProvider) Pricing() (cloudprovider.PricingModel, caerrors.AutoscalerError) { diff --git a/cluster-autoscaler/cloudprovider/ovhcloud/ovh_cloud_provider.go b/cluster-autoscaler/cloudprovider/ovhcloud/ovh_cloud_provider.go index 075c6cb78a15..09742563fcef 100644 --- a/cluster-autoscaler/cloudprovider/ovhcloud/ovh_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/ovhcloud/ovh_cloud_provider.go @@ -151,6 +151,11 @@ func (provider *OVHCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovi return ng, err } +// NodeExists returns whether node exists in this cloud provider +func (provider *OVHCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { + return true, cloudprovider.ErrNotImplemented +} + // findNodeGroupFromCache tries to retrieve the associated node group from an already built mapping in cache func (provider *OVHCloudProvider) findNodeGroupFromCache(providerID string) cloudprovider.NodeGroup { if ng, ok := provider.manager.NodeGroupPerProviderID[providerID]; ok { diff --git a/cluster-autoscaler/cloudprovider/packet/packet_cloud_provider.go b/cluster-autoscaler/cloudprovider/packet/packet_cloud_provider.go index ca59a84f3d23..c4b5fe090eff 100644 --- a/cluster-autoscaler/cloudprovider/packet/packet_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/packet/packet_cloud_provider.go @@ -120,6 +120,11 @@ func (pcp *packetCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovide return nil, fmt.Errorf("Could not find group for node: %s", node.Spec.ProviderID) } +// NodeExists returns whether node exists in this cloud provider +func (pcp *packetCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { + return true, cloudprovider.ErrNotImplemented +} + // Pricing returns pricing model for this cloud provider or error if not available. func (pcp *packetCloudProvider) Pricing() (cloudprovider.PricingModel, errors.AutoscalerError) { return &PacketPriceModel{}, nil diff --git a/cluster-autoscaler/cloudprovider/rancher/rancher_provider.go b/cluster-autoscaler/cloudprovider/rancher/rancher_provider.go index 6e1126b6e00f..0414bab773e1 100644 --- a/cluster-autoscaler/cloudprovider/rancher/rancher_provider.go +++ b/cluster-autoscaler/cloudprovider/rancher/rancher_provider.go @@ -171,6 +171,11 @@ func (provider *RancherCloudProvider) NodeGroupForNode(node *corev1.Node) (cloud return nil, nil } +// NodeExists returns whether node exists in this cloud provider +func (provider *RancherCloudProvider) NodeExists(node *corev1.Node) (bool, error) { + return true, cloudprovider.ErrNotImplemented +} + // GetAvailableMachineTypes get all machine types that can be requested from the cloud provider. // Implementation optional. func (provider *RancherCloudProvider) GetAvailableMachineTypes() ([]string, error) { diff --git a/cluster-autoscaler/cloudprovider/scaleway/scaleway_cloud_provider.go b/cluster-autoscaler/cloudprovider/scaleway/scaleway_cloud_provider.go index 9d1608c57121..5a8db8ade849 100644 --- a/cluster-autoscaler/cloudprovider/scaleway/scaleway_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/scaleway/scaleway_cloud_provider.go @@ -162,6 +162,11 @@ func (scw *scalewayCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovi return scw.nodeGroupForNode(node) } +// NodeExists returns whether node exists in this cloud provider +func (scw *scalewayCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { + return true, cloudprovider.ErrNotImplemented +} + func (scw *scalewayCloudProvider) NodePrice(node *apiv1.Node, startTime time.Time, endTime time.Time) (float64, error) { ng, err := scw.nodeGroupForNode(node) if err != nil { diff --git a/cluster-autoscaler/cloudprovider/tencentcloud/tencentcloud_cloud_provider.go b/cluster-autoscaler/cloudprovider/tencentcloud/tencentcloud_cloud_provider.go index 52768376dc4c..34b27437bc02 100644 --- a/cluster-autoscaler/cloudprovider/tencentcloud/tencentcloud_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/tencentcloud/tencentcloud_cloud_provider.go @@ -109,6 +109,11 @@ func (tencentcloud *tencentCloudProvider) NodeGroupForNode(node *apiv1.Node) (cl return asg, nil } +// NodeExists returns whether node exists in this cloud provider +func (tencentcloud *tencentCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { + return true, cloudprovider.ErrNotImplemented +} + // GPULabel returns the label added to nodes with GPU resource. func (tencentcloud *tencentCloudProvider) GPULabel() string { return GPULabel diff --git a/cluster-autoscaler/cloudprovider/test/test_cloud_provider.go b/cluster-autoscaler/cloudprovider/test/test_cloud_provider.go index af58f9e639a2..d27528f661af 100644 --- a/cluster-autoscaler/cloudprovider/test/test_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/test/test_cloud_provider.go @@ -41,6 +41,9 @@ type OnNodeGroupCreateFunc func(string) error // OnNodeGroupDeleteFunc is a function called when a node group is deleted. type OnNodeGroupDeleteFunc func(string) error +// NodeExists is a function called to determine if a node has been removed from the cloud provider. +type NodeExists func(string) (bool, error) + // TestCloudProvider is a dummy cloud provider to be used in tests. type TestCloudProvider struct { sync.Mutex @@ -50,6 +53,7 @@ type TestCloudProvider struct { onScaleDown func(string, string) error onNodeGroupCreate func(string) error onNodeGroupDelete func(string) error + nodeExists func(string) (bool, error) machineTypes []string machineTemplates map[string]*schedulerframework.NodeInfo priceModel cloudprovider.PricingModel @@ -84,6 +88,19 @@ func NewTestAutoprovisioningCloudProvider(onScaleUp OnScaleUpFunc, onScaleDown O } } +// NewTestNodeDeletionDetectionCloudProvider builds new TestCloudProvider with deletion detection support +func NewTestNodeDeletionDetectionCloudProvider(onScaleUp OnScaleUpFunc, onScaleDown OnScaleDownFunc, + deleted NodeExists) *TestCloudProvider { + return &TestCloudProvider{ + nodes: make(map[string]string), + groups: make(map[string]cloudprovider.NodeGroup), + onScaleUp: onScaleUp, + onScaleDown: onScaleDown, + nodeExists: deleted, + resourceLimiter: cloudprovider.NewResourceLimiter(make(map[string]int64), make(map[string]int64)), + } +} + // Name returns name of the cloud provider. func (tcp *TestCloudProvider) Name() string { return "TestCloudProvider" @@ -140,6 +157,19 @@ func (tcp *TestCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovider. return group, nil } +// NodeExists returns true if the node is available in cloud provider, +// or ErrNotImplemented to fall back to taint-based node deletion in clusterstate +// readiness calculation. +func (tcp *TestCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { + tcp.Lock() + defer tcp.Unlock() + if tcp.nodeExists != nil { + return tcp.nodeExists(node.Name) + } + _, found := tcp.nodes[node.Name] + return found, nil +} + // Pricing returns pricing model for this cloud provider or error if not available. func (tcp *TestCloudProvider) Pricing() (cloudprovider.PricingModel, errors.AutoscalerError) { if tcp.priceModel == nil { diff --git a/cluster-autoscaler/cloudprovider/vultr/vultr_cloud_provider.go b/cluster-autoscaler/cloudprovider/vultr/vultr_cloud_provider.go index de8bfaedbd48..3a0162bdbb37 100644 --- a/cluster-autoscaler/cloudprovider/vultr/vultr_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/vultr/vultr_cloud_provider.go @@ -81,6 +81,11 @@ func (v *vultrCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovider.N return nil, nil } +// NodeExists returns whether node exists in this cloud provider +func (v *vultrCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { + return true, cloudprovider.ErrNotImplemented +} + // Pricing returns pricing model for this cloud provider or error if not available. // Implementation optional. func (v *vultrCloudProvider) Pricing() (cloudprovider.PricingModel, errors.AutoscalerError) { diff --git a/cluster-autoscaler/clusterstate/clusterstate.go b/cluster-autoscaler/clusterstate/clusterstate.go index ae89557c6a57..4be85888e179 100644 --- a/cluster-autoscaler/clusterstate/clusterstate.go +++ b/cluster-autoscaler/clusterstate/clusterstate.go @@ -17,6 +17,7 @@ limitations under the License. package clusterstate import ( + "errors" "fmt" "reflect" "strings" @@ -297,6 +298,7 @@ func (csr *ClusterStateRegistry) UpdateNodes(nodes []*apiv1.Node, nodeInfosForGr if err != nil { return err } + cloudProviderNodesRemoved := csr.getCloudProviderDeletedNodes(nodes, cloudProviderNodeInstances) notRegistered := getNotRegisteredNodes(nodes, cloudProviderNodeInstances, currentTime) csr.Lock() @@ -544,7 +546,7 @@ func (csr *ClusterStateRegistry) updateReadinessStats(currentTime time.Time) { update := func(current Readiness, node *apiv1.Node, nr kube_util.NodeReadiness) Readiness { current.Registered++ - if _, exists := csr.deletedNodes[node.Name]; exists { + if _, isDeleted := csr.deletedNodes[node.Name]; isDeleted { current.Deleted++ } else if nr.Ready { current.Ready++ @@ -684,7 +686,7 @@ func (csr *ClusterStateRegistry) updateCloudProviderDeletedNodes(deletedNodes [] csr.deletedNodes = result } -//GetCloudProviderDeletedNodes returns a list of all nodes removed from cloud provider but registered in Kubernetes. +// GetCloudProviderDeletedNodes returns a list of all nodes removed from cloud provider but registered in Kubernetes. func (csr *ClusterStateRegistry) GetCloudProviderDeletedNodes() []*apiv1.Node { csr.Lock() defer csr.Unlock() @@ -990,35 +992,59 @@ func (csr *ClusterStateRegistry) getCloudProviderDeletedNodes(allNodes []*apiv1. nodesRemoved := make([]*apiv1.Node, 0) currentCloudInstances := make(map[string]string, 0) registeredNodes := make(map[string]*apiv1.Node, 0) - for nodeGroupName, instances := range cloudProviderNodeInstances { - for _, instance := range instances { - currentCloudInstances[instance.Id] = nodeGroupName - } - } - for _, node := range allNodes { - registeredNodes[node.Name] = node - } + if len(allNodes) > 0 { + _, err := csr.cloudProvider.NodeExists(allNodes[0]) + // Check if the cloud provider implements nodeExists method + nodeExistsNotImplemented := errors.Is(err, cloudprovider.ErrNotImplemented) + if nodeExistsNotImplemented { + // Fall-back to taint-based node deletion + for _, node := range allNodes { + if deletetaint.HasToBeDeletedTaint(node) { + nodesRemoved = append(nodesRemoved, node) + } + } + } else { + for nodeGroupName, instances := range cloudProviderNodeInstances { + for _, instance := range instances { + currentCloudInstances[instance.Id] = nodeGroupName + } + } + for _, node := range allNodes { + registeredNodes[node.Name] = node + } - // Fill previously deleted nodes, if they are still registered in Kubernetes - for nodeName, node := range csr.deletedNodes { - // Safety check to prevent flagging Kubernetes nodes as deleted - // if the Cloud Provider instance is re-discovered - _, cloudProviderFound := currentCloudInstances[node.Name] - if _, found := registeredNodes[nodeName]; found && !cloudProviderFound { - nodesRemoved = append(nodesRemoved, node) - } - } + // Fill previously deleted nodes, if they are still registered in Kubernetes + for nodeName, node := range csr.deletedNodes { + // Safety check to prevent flagging Kubernetes nodes as deleted + // if the Cloud Provider instance is re-discovered + _, cloudProviderFound := currentCloudInstances[node.Name] + if _, found := registeredNodes[nodeName]; found && !cloudProviderFound { + // Confirm that node is deleted by cloud provider, instead of + // a not-autoscaled node + nodeExists, existsErr := csr.cloudProvider.NodeExists(node) + if existsErr == nil && !nodeExists { + nodesRemoved = append(nodesRemoved, node) + } + } + } - // Seek nodes that may have been deleted since last update - // cloudProviderNodeInstances are retrieved by nodeGroup, - // not autoscaled nodes will be excluded - for _, instances := range csr.cloudProviderNodeInstances { - for _, instance := range instances { - if _, found := currentCloudInstances[instance.Id]; !found { - // Check Kubernetes registered nodes for corresponding deleted - // Cloud Provider instance - if kubeNode, kubeNodeFound := registeredNodes[instance.Id]; kubeNodeFound { - nodesRemoved = append(nodesRemoved, kubeNode) + // Seek nodes that may have been deleted since last update + // cloudProviderNodeInstances are retrieved by nodeGroup, + // not autoscaled nodes will be excluded + for _, instances := range csr.cloudProviderNodeInstances { + for _, instance := range instances { + if _, found := currentCloudInstances[instance.Id]; !found { + // Check Kubernetes registered nodes for corresponding deleted + // Cloud Provider instance + if kubeNode, kubeNodeFound := registeredNodes[instance.Id]; kubeNodeFound { + // Confirm that node is deleted by cloud provider, instead of + // a not-autoscaled node + nodeExists, existsErr := csr.cloudProvider.NodeExists(kubeNode) + if existsErr == nil && !nodeExists { + nodesRemoved = append(nodesRemoved, kubeNode) + } + } + } } } } diff --git a/cluster-autoscaler/clusterstate/clusterstate_test.go b/cluster-autoscaler/clusterstate/clusterstate_test.go index 3b7f5b663572..2db24ddbd807 100644 --- a/cluster-autoscaler/clusterstate/clusterstate_test.go +++ b/cluster-autoscaler/clusterstate/clusterstate_test.go @@ -18,7 +18,6 @@ package clusterstate import ( "fmt" - "k8s.io/autoscaler/cluster-autoscaler/utils/deletetaint" "testing" "time" @@ -29,6 +28,8 @@ import ( testprovider "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/test" "k8s.io/autoscaler/cluster-autoscaler/clusterstate/api" "k8s.io/autoscaler/cluster-autoscaler/clusterstate/utils" + "k8s.io/autoscaler/cluster-autoscaler/cloudprovider" + "k8s.io/autoscaler/cluster-autoscaler/utils/deletetaint" . "k8s.io/autoscaler/cluster-autoscaler/utils/test" "k8s.io/client-go/kubernetes/fake" kube_record "k8s.io/client-go/tools/record" @@ -515,7 +516,45 @@ func TestUpcomingNodes(t *testing.T) { assert.Equal(t, 1, upcomingNodes["ng2"]) assert.Equal(t, 2, upcomingNodes["ng3"]) assert.NotContains(t, upcomingNodes, "ng4") - assert.NotContains(t, upcomingNodes, "ng5") + assert.Equal(t, 0, upcomingNodes["ng5"]) +} + +func TestTaintBasedNodeDeletion(t *testing.T) { + // Create a new Cloud Provider that does not implement the NodeExists check + // it will return the ErrNotImplemented error instead. + provider := testprovider.NewTestNodeDeletionDetectionCloudProvider(nil, nil, + func(string) (bool, error) { return false, cloudprovider.ErrNotImplemented }) + now := time.Now() + + // One node is already there, for a second nde deletion / draining was already started. + ng1_1 := BuildTestNode("ng1-1", 1000, 1000) + SetNodeReadyState(ng1_1, true, now.Add(-time.Minute)) + ng1_2 := BuildTestNode("ng1-2", 1000, 1000) + SetNodeReadyState(ng1_2, true, now.Add(-time.Minute)) + ng1_2.Spec.Taints = []apiv1.Taint{ + { + Key: deletetaint.ToBeDeletedTaint, + Value: fmt.Sprint(time.Now().Unix()), + Effect: apiv1.TaintEffectNoSchedule, + }, + } + provider.AddNodeGroup("ng1", 1, 10, 2) + provider.AddNode("ng1", ng1_1) + provider.AddNode("ng1", ng1_2) + + assert.NotNil(t, provider) + fakeClient := &fake.Clientset{} + fakeLogRecorder, _ := utils.NewStatusMapRecorder(fakeClient, "kube-system", kube_record.NewFakeRecorder(5), false, "my-cool-configmap") + clusterstate := NewClusterStateRegistry(provider, ClusterStateRegistryConfig{ + MaxTotalUnreadyPercentage: 10, + OkTotalUnreadyCount: 1, + }, fakeLogRecorder, newBackoff()) + err := clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, ng1_2}, nil, now) + assert.NoError(t, err) + assert.Empty(t, clusterstate.GetScaleUpFailures()) + + upcomingNodes := clusterstate.GetUpcomingNodes() + assert.Equal(t, 1, upcomingNodes["ng1"]) } func TestIncorrectSize(t *testing.T) { @@ -598,13 +637,15 @@ func TestCloudProviderDeletedNodes(t *testing.T) { SetNodeReadyState(ng1_2, true, now.Add(-time.Minute)) ng1_2.Spec.ProviderID = "ng1-2" // No Node Group - Not Autoscaled Node - noNg := BuildTestNode("no-ng", 1000, 1000) - SetNodeReadyState(noNg, true, now.Add(-time.Minute)) - noNg.Spec.ProviderID = "no-ng" + noNgNode := BuildTestNode("no-ng", 1000, 1000) + SetNodeReadyState(noNgNode, true, now.Add(-time.Minute)) + + noNgNode.Spec.ProviderID = "no-ng" provider := testprovider.NewTestCloudProvider(nil, nil) provider.AddNodeGroup("ng1", 1, 10, 2) provider.AddNode("ng1", ng1_1) provider.AddNode("ng1", ng1_2) + provider.AddNode("no_ng", noNgNode) fakeClient := &fake.Clientset{} fakeLogRecorder, _ := utils.NewStatusMapRecorder(fakeClient, "kube-system", kube_record.NewFakeRecorder(5), false, "my-cool-configmap") @@ -614,7 +655,7 @@ func TestCloudProviderDeletedNodes(t *testing.T) { MaxNodeProvisionTime: 10 * time.Second, }, fakeLogRecorder, newBackoff()) now.Add(time.Minute) - err := clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, ng1_2, noNg}, nil, now) + err := clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, ng1_2, noNgNode}, nil, now) // Nodes are registered correctly between Kubernetes and cloud provider. assert.NoError(t, err) @@ -627,7 +668,8 @@ func TestCloudProviderDeletedNodes(t *testing.T) { provider.DeleteNode(ng1_2) clusterstate.InvalidateNodeInstancesCacheEntry(nodeGroup) now.Add(time.Minute) - err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, ng1_2, noNg}, nil, now) + + err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, ng1_2, noNgNode}, nil, now) assert.NoError(t, err) assert.Equal(t, 1, len(clusterstate.GetCloudProviderDeletedNodes())) assert.Equal(t, "ng1-2", clusterstate.GetCloudProviderDeletedNodes()[0].Name) @@ -635,7 +677,8 @@ func TestCloudProviderDeletedNodes(t *testing.T) { // The node is removed from Kubernetes now.Add(time.Minute) - err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNg}, nil, now) + + err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNgNode}, nil, now) assert.NoError(t, err) assert.Equal(t, 0, len(clusterstate.GetCloudProviderDeletedNodes())) @@ -646,7 +689,8 @@ func TestCloudProviderDeletedNodes(t *testing.T) { provider.AddNode("ng1", ng1_3) clusterstate.InvalidateNodeInstancesCacheEntry(nodeGroup) now.Add(time.Minute) - err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, ng1_3, noNg}, nil, now) + + err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, ng1_3, noNgNode}, nil, now) assert.NoError(t, err) assert.Equal(t, 0, len(clusterstate.GetCloudProviderDeletedNodes())) @@ -657,7 +701,8 @@ func TestCloudProviderDeletedNodes(t *testing.T) { provider.DeleteNode(ng1_3) clusterstate.InvalidateNodeInstancesCacheEntry(nodeGroup) now.Add(time.Minute) - err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNg, ng1_3}, nil, now) + + err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNgNode, ng1_3}, nil, now) assert.NoError(t, err) assert.Equal(t, 1, len(clusterstate.GetCloudProviderDeletedNodes())) assert.Equal(t, "ng1-3", clusterstate.GetCloudProviderDeletedNodes()[0].Name) @@ -666,7 +711,8 @@ func TestCloudProviderDeletedNodes(t *testing.T) { // Confirm that previously identified deleted Cloud Provider nodes are still included // until it is removed from Kubernetes now.Add(time.Minute) - err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNg, ng1_3}, nil, now) + + err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNgNode, ng1_3}, nil, now) assert.NoError(t, err) assert.Equal(t, 1, len(clusterstate.GetCloudProviderDeletedNodes())) assert.Equal(t, "ng1-3", clusterstate.GetCloudProviderDeletedNodes()[0].Name) @@ -674,7 +720,8 @@ func TestCloudProviderDeletedNodes(t *testing.T) { // The node is removed from Kubernetes now.Add(time.Minute) - err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNg}, nil, now) + + err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNgNode}, nil, now) assert.NoError(t, err) assert.Equal(t, 0, len(clusterstate.GetCloudProviderDeletedNodes())) } diff --git a/cluster-autoscaler/core/static_autoscaler_test.go b/cluster-autoscaler/core/static_autoscaler_test.go index 8ac72cc9ddee..de38c7f9a049 100644 --- a/cluster-autoscaler/core/static_autoscaler_test.go +++ b/cluster-autoscaler/core/static_autoscaler_test.go @@ -1083,6 +1083,10 @@ func TestStaticAutoscalerInstanceCreationErrors(t *testing.T) { } return nil }, nil) + provider.On("NodeExists", mock.Anything).Return( + func(node *apiv1.Node) bool { + return false + }, nil) now := time.Now() @@ -1211,6 +1215,10 @@ func TestStaticAutoscalerInstanceCreationErrors(t *testing.T) { provider = &mockprovider.CloudProvider{} provider.On("NodeGroups").Return([]cloudprovider.NodeGroup{nodeGroupC}) provider.On("NodeGroupForNode", mock.Anything).Return(nil, nil) + provider.On("NodeExists", mock.Anything).Return( + func(node *apiv1.Node) bool { + return false + }, nil) clusterState = clusterstate.NewClusterStateRegistry(provider, clusterStateConfig, context.LogRecorder, NewBackoff()) clusterState.RefreshCloudProviderNodeInstancesCache() From e59c0441ffaf7c41f02ff3d7b3d82fb335783966 Mon Sep 17 00:00:00 2001 From: Clint Fooken Date: Mon, 17 Oct 2022 15:17:28 -0700 Subject: [PATCH 3/9] Fixing go formatting issues with clusterstate_test --- .../clusterstate/clusterstate_test.go | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/cluster-autoscaler/clusterstate/clusterstate_test.go b/cluster-autoscaler/clusterstate/clusterstate_test.go index 2db24ddbd807..62c320c00b00 100644 --- a/cluster-autoscaler/clusterstate/clusterstate_test.go +++ b/cluster-autoscaler/clusterstate/clusterstate_test.go @@ -25,11 +25,11 @@ import ( apiv1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/autoscaler/cluster-autoscaler/cloudprovider" testprovider "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/test" "k8s.io/autoscaler/cluster-autoscaler/clusterstate/api" "k8s.io/autoscaler/cluster-autoscaler/clusterstate/utils" - "k8s.io/autoscaler/cluster-autoscaler/cloudprovider" - "k8s.io/autoscaler/cluster-autoscaler/utils/deletetaint" + "k8s.io/autoscaler/cluster-autoscaler/utils/deletetaint" . "k8s.io/autoscaler/cluster-autoscaler/utils/test" "k8s.io/client-go/kubernetes/fake" kube_record "k8s.io/client-go/tools/record" @@ -668,8 +668,8 @@ func TestCloudProviderDeletedNodes(t *testing.T) { provider.DeleteNode(ng1_2) clusterstate.InvalidateNodeInstancesCacheEntry(nodeGroup) now.Add(time.Minute) - - err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, ng1_2, noNgNode}, nil, now) + + err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, ng1_2, noNgNode}, nil, now) assert.NoError(t, err) assert.Equal(t, 1, len(clusterstate.GetCloudProviderDeletedNodes())) assert.Equal(t, "ng1-2", clusterstate.GetCloudProviderDeletedNodes()[0].Name) @@ -677,7 +677,7 @@ func TestCloudProviderDeletedNodes(t *testing.T) { // The node is removed from Kubernetes now.Add(time.Minute) - + err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNgNode}, nil, now) assert.NoError(t, err) assert.Equal(t, 0, len(clusterstate.GetCloudProviderDeletedNodes())) @@ -689,8 +689,8 @@ func TestCloudProviderDeletedNodes(t *testing.T) { provider.AddNode("ng1", ng1_3) clusterstate.InvalidateNodeInstancesCacheEntry(nodeGroup) now.Add(time.Minute) - - err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, ng1_3, noNgNode}, nil, now) + + err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, ng1_3, noNgNode}, nil, now) assert.NoError(t, err) assert.Equal(t, 0, len(clusterstate.GetCloudProviderDeletedNodes())) @@ -702,7 +702,7 @@ func TestCloudProviderDeletedNodes(t *testing.T) { clusterstate.InvalidateNodeInstancesCacheEntry(nodeGroup) now.Add(time.Minute) - err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNgNode, ng1_3}, nil, now) + err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNgNode, ng1_3}, nil, now) assert.NoError(t, err) assert.Equal(t, 1, len(clusterstate.GetCloudProviderDeletedNodes())) assert.Equal(t, "ng1-3", clusterstate.GetCloudProviderDeletedNodes()[0].Name) @@ -712,7 +712,7 @@ func TestCloudProviderDeletedNodes(t *testing.T) { // until it is removed from Kubernetes now.Add(time.Minute) - err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNgNode, ng1_3}, nil, now) + err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNgNode, ng1_3}, nil, now) assert.NoError(t, err) assert.Equal(t, 1, len(clusterstate.GetCloudProviderDeletedNodes())) assert.Equal(t, "ng1-3", clusterstate.GetCloudProviderDeletedNodes()[0].Name) @@ -720,7 +720,7 @@ func TestCloudProviderDeletedNodes(t *testing.T) { // The node is removed from Kubernetes now.Add(time.Minute) - + err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNgNode}, nil, now) assert.NoError(t, err) assert.Equal(t, 0, len(clusterstate.GetCloudProviderDeletedNodes())) From 7fc1f6be01af621b87cee4ef0710e17095d09cc4 Mon Sep 17 00:00:00 2001 From: Clint Fooken Date: Mon, 17 Oct 2022 15:45:55 -0700 Subject: [PATCH 4/9] Fixing errors due to merge on branches. --- cluster-autoscaler/clusterstate/clusterstate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cluster-autoscaler/clusterstate/clusterstate.go b/cluster-autoscaler/clusterstate/clusterstate.go index 4be85888e179..afbc6c129fe9 100644 --- a/cluster-autoscaler/clusterstate/clusterstate.go +++ b/cluster-autoscaler/clusterstate/clusterstate.go @@ -29,6 +29,7 @@ import ( "k8s.io/autoscaler/cluster-autoscaler/clusterstate/utils" "k8s.io/autoscaler/cluster-autoscaler/metrics" "k8s.io/autoscaler/cluster-autoscaler/utils/backoff" + "k8s.io/autoscaler/cluster-autoscaler/utils/deletetaint" kube_util "k8s.io/autoscaler/cluster-autoscaler/utils/kubernetes" apiv1 "k8s.io/api/core/v1" @@ -294,7 +295,6 @@ func (csr *ClusterStateRegistry) UpdateNodes(nodes []*apiv1.Node, nodeInfosForGr } cloudProviderNodeInstances, err := csr.getCloudProviderNodeInstances() - cloudProviderNodesRemoved := csr.getCloudProviderDeletedNodes(nodes, cloudProviderNodeInstances) if err != nil { return err } From ea7059f4c6d9e1640ddd7ed23d1a71958fb840a8 Mon Sep 17 00:00:00 2001 From: Clint Fooken Date: Mon, 17 Oct 2022 18:39:19 -0700 Subject: [PATCH 5/9] Adjusting initial implementation of NodeExists to be consistent among cloud providers to return true and ErrNotImplemented. --- cluster-autoscaler/cloudprovider/azure/azure_cloud_provider.go | 2 +- .../cloudprovider/baiducloud/baiducloud_cloud_provider.go | 2 +- .../cloudprovider/bizflycloud/bizflycloud_cloud_provider.go | 2 +- .../cloudprovider/brightbox/brightbox_cloud_provider.go | 2 +- .../cloudprovider/cherryservers/cherry_cloud_provider.go | 2 +- cluster-autoscaler/cloudprovider/civo/civo_cloud_provider.go | 2 +- .../cloudprovider/cloudstack/cloudstack_cloud_provider.go | 2 +- .../cloudprovider/clusterapi/clusterapi_provider.go | 2 +- .../cloudprovider/digitalocean/digitalocean_cloud_provider.go | 2 +- .../cloudprovider/exoscale/exoscale_cloud_provider.go | 2 +- .../cloudprovider/externalgrpc/externalgrpc_cloud_provider.go | 2 +- cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go | 2 +- .../cloudprovider/hetzner/hetzner_cloud_provider.go | 2 +- .../cloudprovider/huaweicloud/huaweicloud_cloud_provider.go | 2 +- .../cloudprovider/ionoscloud/ionoscloud_cloud_provider.go | 2 +- .../cloudprovider/kamatera/kamatera_cloud_provider.go | 2 +- cluster-autoscaler/cloudprovider/kubemark/kubemark_linux.go | 2 +- 17 files changed, 17 insertions(+), 17 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/azure/azure_cloud_provider.go b/cluster-autoscaler/cloudprovider/azure/azure_cloud_provider.go index 0c0142f9d509..731948f387e2 100644 --- a/cluster-autoscaler/cloudprovider/azure/azure_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/azure/azure_cloud_provider.go @@ -108,7 +108,7 @@ func (azure *AzureCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovid // NodeExists returns whether node exists in this cloud provider func (azure *AzureCloudProvider) NodeExists(*apiv1.Node) (bool, error) { - return false, cloudprovider.ErrNotImplemented + return true, cloudprovider.ErrNotImplemented } // Pricing returns pricing model for this cloud provider or error if not available. diff --git a/cluster-autoscaler/cloudprovider/baiducloud/baiducloud_cloud_provider.go b/cluster-autoscaler/cloudprovider/baiducloud/baiducloud_cloud_provider.go index 0c7d97f1a510..a5e893a42ee8 100644 --- a/cluster-autoscaler/cloudprovider/baiducloud/baiducloud_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/baiducloud/baiducloud_cloud_provider.go @@ -182,7 +182,7 @@ func (baiducloud *baiducloudCloudProvider) NodeGroupForNode(node *apiv1.Node) (c // NodeExists returns whether node exists in this cloud provider func (baiducloud *baiducloudCloudProvider) NodeExists(*apiv1.Node) (bool, error) { - return false, cloudprovider.ErrNotImplemented + return true, cloudprovider.ErrNotImplemented } // Pricing returns pricing model for this cloud provider or error if not available. diff --git a/cluster-autoscaler/cloudprovider/bizflycloud/bizflycloud_cloud_provider.go b/cluster-autoscaler/cloudprovider/bizflycloud/bizflycloud_cloud_provider.go index ae1705d77454..732636bcddb8 100644 --- a/cluster-autoscaler/cloudprovider/bizflycloud/bizflycloud_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/bizflycloud/bizflycloud_cloud_provider.go @@ -106,7 +106,7 @@ func (d *bizflycloudCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprov // NodeExists returns whether node exists in this cloud provider func (d *bizflycloudCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { - return false, cloudprovider.ErrNotImplemented + return true, cloudprovider.ErrNotImplemented } // Pricing returns pricing model for this cloud provider or error if not diff --git a/cluster-autoscaler/cloudprovider/brightbox/brightbox_cloud_provider.go b/cluster-autoscaler/cloudprovider/brightbox/brightbox_cloud_provider.go index 89c6ed83ce40..130fb6297a92 100644 --- a/cluster-autoscaler/cloudprovider/brightbox/brightbox_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/brightbox/brightbox_cloud_provider.go @@ -83,7 +83,7 @@ func (b *brightboxCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovid // NodeExists returns whether node exists in this cloud provider func (b *brightboxCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { - return false, cloudprovider.ErrNotImplemented + return true, cloudprovider.ErrNotImplemented } // Refresh is before every main loop and can be used to dynamically diff --git a/cluster-autoscaler/cloudprovider/cherryservers/cherry_cloud_provider.go b/cluster-autoscaler/cloudprovider/cherryservers/cherry_cloud_provider.go index dce984be5a04..313184549b67 100644 --- a/cluster-autoscaler/cloudprovider/cherryservers/cherry_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/cherryservers/cherry_cloud_provider.go @@ -124,7 +124,7 @@ func (ccp *cherryCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovide // NodeExists returns whether node exists in this cloud provider func (ccp *cherryCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { - return false, cloudprovider.ErrNotImplemented + return true, cloudprovider.ErrNotImplemented } // Pricing returns pricing model for this cloud provider or error if not available. diff --git a/cluster-autoscaler/cloudprovider/civo/civo_cloud_provider.go b/cluster-autoscaler/cloudprovider/civo/civo_cloud_provider.go index c557a1609921..9ceb6e701e4e 100644 --- a/cluster-autoscaler/cloudprovider/civo/civo_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/civo/civo_cloud_provider.go @@ -101,7 +101,7 @@ func (d *civoCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovider.No // NodeExists returns whether node exists in this cloud provider func (d *civoCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { - return false, cloudprovider.ErrNotImplemented + return true, cloudprovider.ErrNotImplemented } // Pricing returns pricing model for this cloud provider or error if not diff --git a/cluster-autoscaler/cloudprovider/cloudstack/cloudstack_cloud_provider.go b/cluster-autoscaler/cloudprovider/cloudstack/cloudstack_cloud_provider.go index 1f0cd135fa81..881db3f42495 100644 --- a/cluster-autoscaler/cloudprovider/cloudstack/cloudstack_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/cloudstack/cloudstack_cloud_provider.go @@ -70,7 +70,7 @@ func (provider *cloudStackCloudProvider) NodeGroupForNode(node *v1.Node) (cloudp // NodeExists returns whether node exists in this cloud provider func (provider *cloudStackCloudProvider) NodeExists(node *v1.Node) (bool, error) { - return false, cloudprovider.ErrNotImplemented + return true, cloudprovider.ErrNotImplemented } // Cleanup cleans up open resources before the cloud provider is destroyed, i.e. go routines etc. diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go index 7dea49831b7d..8d188ac78b17 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go @@ -83,7 +83,7 @@ func (p *provider) NodeGroupForNode(node *corev1.Node) (cloudprovider.NodeGroup, // NodeExists returns whether node exists in this cloud provider func (p *provider) NodeExists(node *corev1.Node) (bool, error) { - return false, cloudprovider.ErrNotImplemented + return true, cloudprovider.ErrNotImplemented } func (*provider) Pricing() (cloudprovider.PricingModel, errors.AutoscalerError) { diff --git a/cluster-autoscaler/cloudprovider/digitalocean/digitalocean_cloud_provider.go b/cluster-autoscaler/cloudprovider/digitalocean/digitalocean_cloud_provider.go index f2bfa2d72c8b..3c48d4e49394 100644 --- a/cluster-autoscaler/cloudprovider/digitalocean/digitalocean_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/digitalocean/digitalocean_cloud_provider.go @@ -103,7 +103,7 @@ func (d *digitaloceanCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudpro // NodeExists returns whether node exists in this cloud provider func (d *digitaloceanCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { - return false, cloudprovider.ErrNotImplemented + return true, cloudprovider.ErrNotImplemented } // Pricing returns pricing model for this cloud provider or error if not diff --git a/cluster-autoscaler/cloudprovider/exoscale/exoscale_cloud_provider.go b/cluster-autoscaler/cloudprovider/exoscale/exoscale_cloud_provider.go index 2207a272e493..fe371dbeccaf 100644 --- a/cluster-autoscaler/cloudprovider/exoscale/exoscale_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/exoscale/exoscale_cloud_provider.go @@ -133,7 +133,7 @@ func (e *exoscaleCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovide // NodeExists returns whether node exists in this cloud provider func (e *exoscaleCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { - return false, cloudprovider.ErrNotImplemented + return true, cloudprovider.ErrNotImplemented } // Pricing returns pricing model for this cloud provider or error if not available. diff --git a/cluster-autoscaler/cloudprovider/externalgrpc/externalgrpc_cloud_provider.go b/cluster-autoscaler/cloudprovider/externalgrpc/externalgrpc_cloud_provider.go index a953aea11d8c..c2bb83abe19a 100644 --- a/cluster-autoscaler/cloudprovider/externalgrpc/externalgrpc_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/externalgrpc/externalgrpc_cloud_provider.go @@ -136,7 +136,7 @@ func (e *externalGrpcCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudpro // NodeExists returns whether node exists in this cloud provider func (e *externalGrpcCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { - return false, cloudprovider.ErrNotImplemented + return true, cloudprovider.ErrNotImplemented } // pricingModel implements cloudprovider.PricingModel interface. diff --git a/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go b/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go index b80c88d26a72..2c9010003c37 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go @@ -103,7 +103,7 @@ func (gce *GceCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovider.N // NodeExists returns whether node exists in this cloud provider func (gce *GceCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { - return false, cloudprovider.ErrNotImplemented + return true, cloudprovider.ErrNotImplemented } // Pricing returns pricing model for this cloud provider or error if not available. diff --git a/cluster-autoscaler/cloudprovider/hetzner/hetzner_cloud_provider.go b/cluster-autoscaler/cloudprovider/hetzner/hetzner_cloud_provider.go index 17deb4074dc7..9597696fb778 100644 --- a/cluster-autoscaler/cloudprovider/hetzner/hetzner_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/hetzner/hetzner_cloud_provider.go @@ -101,7 +101,7 @@ func (d *HetznerCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovider // NodeExists returns whether node exists in this cloud provider func (d *HetznerCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { - return false, cloudprovider.ErrNotImplemented + return true, cloudprovider.ErrNotImplemented } // Pricing returns pricing model for this cloud provider or error if not diff --git a/cluster-autoscaler/cloudprovider/huaweicloud/huaweicloud_cloud_provider.go b/cluster-autoscaler/cloudprovider/huaweicloud/huaweicloud_cloud_provider.go index 0a61787dcc92..97f8350cb644 100644 --- a/cluster-autoscaler/cloudprovider/huaweicloud/huaweicloud_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/huaweicloud/huaweicloud_cloud_provider.go @@ -125,7 +125,7 @@ func (hcp *huaweicloudCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudpr // NodeExists returns whether node exists in this cloud provider func (hcp *huaweicloudCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { - return false, cloudprovider.ErrNotImplemented + return true, cloudprovider.ErrNotImplemented } // Pricing returns pricing model for this cloud provider or error if not available. Not implemented. diff --git a/cluster-autoscaler/cloudprovider/ionoscloud/ionoscloud_cloud_provider.go b/cluster-autoscaler/cloudprovider/ionoscloud/ionoscloud_cloud_provider.go index 352dc5ce36cf..7d889accd9e6 100644 --- a/cluster-autoscaler/cloudprovider/ionoscloud/ionoscloud_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/ionoscloud/ionoscloud_cloud_provider.go @@ -234,7 +234,7 @@ func (ic *IonosCloudCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprov // NodeExists returns whether node exists in this cloud provider func (ic *IonosCloudCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { - return false, cloudprovider.ErrNotImplemented + return true, cloudprovider.ErrNotImplemented } // Pricing returns pricing model for this cloud provider or error if not diff --git a/cluster-autoscaler/cloudprovider/kamatera/kamatera_cloud_provider.go b/cluster-autoscaler/cloudprovider/kamatera/kamatera_cloud_provider.go index 64d7b20304a4..fdd2f0481a57 100644 --- a/cluster-autoscaler/cloudprovider/kamatera/kamatera_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/kamatera/kamatera_cloud_provider.go @@ -72,7 +72,7 @@ func (k *kamateraCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovide // NodeExists returns whether node exists in this cloud provider func (k *kamateraCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { - return false, cloudprovider.ErrNotImplemented + return true, cloudprovider.ErrNotImplemented } // Pricing returns pricing model for this cloud provider or error if not available. diff --git a/cluster-autoscaler/cloudprovider/kubemark/kubemark_linux.go b/cluster-autoscaler/cloudprovider/kubemark/kubemark_linux.go index 69c052586f42..3169dfdc0f74 100644 --- a/cluster-autoscaler/cloudprovider/kubemark/kubemark_linux.go +++ b/cluster-autoscaler/cloudprovider/kubemark/kubemark_linux.go @@ -141,7 +141,7 @@ func (kubemark *KubemarkCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloud // NodeExists returns whether node exists in this cloud provider func (kubemark *KubemarkCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { - return false, cloudprovider.ErrNotImplemented + return true, cloudprovider.ErrNotImplemented } // GetAvailableMachineTypes get all machine types that can be requested from the cloud provider. From 08dfc7e20fcd6db5053b0f2ea28ebd1121a1ee4b Mon Sep 17 00:00:00 2001 From: Clint Fooken Date: Fri, 4 Nov 2022 17:54:05 -0700 Subject: [PATCH 6/9] Changing deletion logic to rely on a new helper method in ClusterStateRegistry, and remove old complicated logic. Adjust the naming of the method for cloud instance deletion from NodeExists to HasInstance. --- .../alicloud/alicloud_cloud_provider.go | 4 +- .../cloudprovider/aws/aws_cloud_provider.go | 4 +- .../azure/azure_cloud_provider.go | 4 +- .../baiducloud/baiducloud_cloud_provider.go | 4 +- .../bizflycloud/bizflycloud_cloud_provider.go | 4 +- .../brightbox/brightbox_cloud_provider.go | 4 +- .../cherryservers/cherry_cloud_provider.go | 4 +- .../cloudprovider/civo/civo_cloud_provider.go | 4 +- .../cloudprovider/cloud_provider.go | 6 +- .../cloudstack/cloudstack_cloud_provider.go | 6 +- .../clusterapi/clusterapi_provider.go | 4 +- .../digitalocean_cloud_provider.go | 4 +- .../exoscale/exoscale_cloud_provider.go | 4 +- .../externalgrpc_cloud_provider.go | 4 +- .../cloudprovider/gce/gce_cloud_provider.go | 4 +- .../hetzner/hetzner_cloud_provider.go | 4 +- .../huaweicloud/huaweicloud_cloud_provider.go | 4 +- .../ionoscloud/ionoscloud_cloud_provider.go | 4 +- .../kamatera/kamatera_cloud_provider.go | 4 +- .../cloudprovider/kubemark/kubemark_linux.go | 4 +- .../cloudprovider/kubemark/kubemark_other.go | 4 +- .../linode/linode_cloud_provider.go | 4 +- .../magnum/magnum_cloud_provider.go | 4 +- .../cloudprovider/mocks/CloudProvider.go | 4 +- .../cloudprovider/oci/oci_cloud_provider.go | 4 +- .../ovhcloud/ovh_cloud_provider.go | 4 +- .../packet/packet_cloud_provider.go | 4 +- .../cloudprovider/rancher/rancher_provider.go | 4 +- .../scaleway/scaleway_cloud_provider.go | 4 +- .../tencentcloud_cloud_provider.go | 4 +- .../cloudprovider/test/test_cloud_provider.go | 18 ++--- .../vultr/vultr_cloud_provider.go | 4 +- .../clusterstate/clusterstate.go | 78 +++++-------------- .../clusterstate/clusterstate_test.go | 34 +++++--- .../core/static_autoscaler_test.go | 4 +- 35 files changed, 116 insertions(+), 146 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/alicloud/alicloud_cloud_provider.go b/cluster-autoscaler/cloudprovider/alicloud/alicloud_cloud_provider.go index bd406cf108aa..17c5fdc131e8 100644 --- a/cluster-autoscaler/cloudprovider/alicloud/alicloud_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/alicloud/alicloud_cloud_provider.go @@ -127,8 +127,8 @@ func (ali *aliCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovider.N return ali.manager.GetAsgForInstance(instanceId) } -// NodeExists returns whether node exists in this cloud provider -func (ali *aliCloudProvider) NodeExists(*apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (ali *aliCloudProvider) HasInstance(*apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/aws/aws_cloud_provider.go b/cluster-autoscaler/cloudprovider/aws/aws_cloud_provider.go index e5e8e813f044..b2474ba62809 100644 --- a/cluster-autoscaler/cloudprovider/aws/aws_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/aws/aws_cloud_provider.go @@ -120,8 +120,8 @@ func (aws *awsCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovider.N }, nil } -// NodeExists returns whether node exists in this cloud provider -func (aws *awsCloudProvider) NodeExists(*apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (aws *awsCloudProvider) HasInstance(*apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/azure/azure_cloud_provider.go b/cluster-autoscaler/cloudprovider/azure/azure_cloud_provider.go index 731948f387e2..40a0205922a6 100644 --- a/cluster-autoscaler/cloudprovider/azure/azure_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/azure/azure_cloud_provider.go @@ -106,8 +106,8 @@ func (azure *AzureCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovid return azure.azureManager.GetNodeGroupForInstance(ref) } -// NodeExists returns whether node exists in this cloud provider -func (azure *AzureCloudProvider) NodeExists(*apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (azure *AzureCloudProvider) HasInstance(*apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/baiducloud/baiducloud_cloud_provider.go b/cluster-autoscaler/cloudprovider/baiducloud/baiducloud_cloud_provider.go index a5e893a42ee8..8849d9626617 100644 --- a/cluster-autoscaler/cloudprovider/baiducloud/baiducloud_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/baiducloud/baiducloud_cloud_provider.go @@ -180,8 +180,8 @@ func (baiducloud *baiducloudCloudProvider) NodeGroupForNode(node *apiv1.Node) (c return asg, err } -// NodeExists returns whether node exists in this cloud provider -func (baiducloud *baiducloudCloudProvider) NodeExists(*apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (baiducloud *baiducloudCloudProvider) HasInstance(*apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/bizflycloud/bizflycloud_cloud_provider.go b/cluster-autoscaler/cloudprovider/bizflycloud/bizflycloud_cloud_provider.go index 732636bcddb8..59f2149227ee 100644 --- a/cluster-autoscaler/cloudprovider/bizflycloud/bizflycloud_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/bizflycloud/bizflycloud_cloud_provider.go @@ -104,8 +104,8 @@ func (d *bizflycloudCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprov return nil, nil } -// NodeExists returns whether node exists in this cloud provider -func (d *bizflycloudCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (d *bizflycloudCloudProvider) HasInstance(node *apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/brightbox/brightbox_cloud_provider.go b/cluster-autoscaler/cloudprovider/brightbox/brightbox_cloud_provider.go index 130fb6297a92..322a02b2fca9 100644 --- a/cluster-autoscaler/cloudprovider/brightbox/brightbox_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/brightbox/brightbox_cloud_provider.go @@ -81,8 +81,8 @@ func (b *brightboxCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovid return nil, nil } -// NodeExists returns whether node exists in this cloud provider -func (b *brightboxCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (b *brightboxCloudProvider) HasInstance(node *apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/cherryservers/cherry_cloud_provider.go b/cluster-autoscaler/cloudprovider/cherryservers/cherry_cloud_provider.go index 313184549b67..95fabbcb30a1 100644 --- a/cluster-autoscaler/cloudprovider/cherryservers/cherry_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/cherryservers/cherry_cloud_provider.go @@ -122,8 +122,8 @@ func (ccp *cherryCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovide return nil, nil } -// NodeExists returns whether node exists in this cloud provider -func (ccp *cherryCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (ccp *cherryCloudProvider) HasInstance(node *apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/civo/civo_cloud_provider.go b/cluster-autoscaler/cloudprovider/civo/civo_cloud_provider.go index 9ceb6e701e4e..4b8e6606143a 100644 --- a/cluster-autoscaler/cloudprovider/civo/civo_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/civo/civo_cloud_provider.go @@ -99,8 +99,8 @@ func (d *civoCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovider.No return nil, nil } -// NodeExists returns whether node exists in this cloud provider -func (d *civoCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (d *civoCloudProvider) HasInstance(node *apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/cloud_provider.go b/cluster-autoscaler/cloudprovider/cloud_provider.go index 50fb0038f330..566f63b7b26f 100644 --- a/cluster-autoscaler/cloudprovider/cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/cloud_provider.go @@ -100,9 +100,9 @@ type CloudProvider interface { // occurred. Must be implemented. NodeGroupForNode(*apiv1.Node) (NodeGroup, error) - // NodeExists returns whether the node exists in cloud provider, - // true if the node is available, false if it has been deleted - NodeExists(*apiv1.Node) (bool, error) + // HasInstance returns whether the node has corresponding instance in cloud provider, + // true if the node has an instance, false if it no longer exists + HasInstance(*apiv1.Node) (bool, error) // Pricing returns pricing model for this cloud provider or error if not available. // Implementation optional. diff --git a/cluster-autoscaler/cloudprovider/cloudstack/cloudstack_cloud_provider.go b/cluster-autoscaler/cloudprovider/cloudstack/cloudstack_cloud_provider.go index 881db3f42495..cf9ef5d5c306 100644 --- a/cluster-autoscaler/cloudprovider/cloudstack/cloudstack_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/cloudstack/cloudstack_cloud_provider.go @@ -68,8 +68,8 @@ func (provider *cloudStackCloudProvider) NodeGroupForNode(node *v1.Node) (cloudp return provider.manager.clusterForNode(node) } -// NodeExists returns whether node exists in this cloud provider -func (provider *cloudStackCloudProvider) NodeExists(node *v1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (provider *cloudStackCloudProvider) HasInstance(node *v1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } @@ -155,7 +155,7 @@ func BuildCloudStack(opts config.AutoscalingOptions, do cloudprovider.NodeGroupD } return &cloudStackCloudProvider{ - manager: manager, + manager: manager, resourceLimiter: rl, } } diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go index 8d188ac78b17..e2928f9c945f 100644 --- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go +++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_provider.go @@ -81,8 +81,8 @@ func (p *provider) NodeGroupForNode(node *corev1.Node) (cloudprovider.NodeGroup, return ng, nil } -// NodeExists returns whether node exists in this cloud provider -func (p *provider) NodeExists(node *corev1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (p *provider) HasInstance(node *corev1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/digitalocean/digitalocean_cloud_provider.go b/cluster-autoscaler/cloudprovider/digitalocean/digitalocean_cloud_provider.go index 3c48d4e49394..1bbf51c3e98a 100644 --- a/cluster-autoscaler/cloudprovider/digitalocean/digitalocean_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/digitalocean/digitalocean_cloud_provider.go @@ -101,8 +101,8 @@ func (d *digitaloceanCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudpro return nil, nil } -// NodeExists returns whether node exists in this cloud provider -func (d *digitaloceanCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (d *digitaloceanCloudProvider) HasInstance(node *apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/exoscale/exoscale_cloud_provider.go b/cluster-autoscaler/cloudprovider/exoscale/exoscale_cloud_provider.go index fe371dbeccaf..021ef6316e4c 100644 --- a/cluster-autoscaler/cloudprovider/exoscale/exoscale_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/exoscale/exoscale_cloud_provider.go @@ -131,8 +131,8 @@ func (e *exoscaleCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovide return nodeGroup, nil } -// NodeExists returns whether node exists in this cloud provider -func (e *exoscaleCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (e *exoscaleCloudProvider) HasInstance(node *apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/externalgrpc/externalgrpc_cloud_provider.go b/cluster-autoscaler/cloudprovider/externalgrpc/externalgrpc_cloud_provider.go index c2bb83abe19a..0fb7917f4477 100644 --- a/cluster-autoscaler/cloudprovider/externalgrpc/externalgrpc_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/externalgrpc/externalgrpc_cloud_provider.go @@ -134,8 +134,8 @@ func (e *externalGrpcCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudpro return ng, nil } -// NodeExists returns whether node exists in this cloud provider -func (e *externalGrpcCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (e *externalGrpcCloudProvider) HasInstance(node *apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go b/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go index 2c9010003c37..83a275c25ce2 100644 --- a/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/gce/gce_cloud_provider.go @@ -101,8 +101,8 @@ func (gce *GceCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovider.N return mig, err } -// NodeExists returns whether node exists in this cloud provider -func (gce *GceCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (gce *GceCloudProvider) HasInstance(node *apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/hetzner/hetzner_cloud_provider.go b/cluster-autoscaler/cloudprovider/hetzner/hetzner_cloud_provider.go index 9597696fb778..fd8c8d1121c9 100644 --- a/cluster-autoscaler/cloudprovider/hetzner/hetzner_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/hetzner/hetzner_cloud_provider.go @@ -99,8 +99,8 @@ func (d *HetznerCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovider return group, nil } -// NodeExists returns whether node exists in this cloud provider -func (d *HetznerCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (d *HetznerCloudProvider) HasInstance(node *apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/huaweicloud/huaweicloud_cloud_provider.go b/cluster-autoscaler/cloudprovider/huaweicloud/huaweicloud_cloud_provider.go index 97f8350cb644..7a66aec2bac8 100644 --- a/cluster-autoscaler/cloudprovider/huaweicloud/huaweicloud_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/huaweicloud/huaweicloud_cloud_provider.go @@ -123,8 +123,8 @@ func (hcp *huaweicloudCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudpr return hcp.cloudServiceManager.GetAsgForInstance(instanceID) } -// NodeExists returns whether node exists in this cloud provider -func (hcp *huaweicloudCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (hcp *huaweicloudCloudProvider) HasInstance(node *apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/ionoscloud/ionoscloud_cloud_provider.go b/cluster-autoscaler/cloudprovider/ionoscloud/ionoscloud_cloud_provider.go index 7d889accd9e6..21d758c5d9c9 100644 --- a/cluster-autoscaler/cloudprovider/ionoscloud/ionoscloud_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/ionoscloud/ionoscloud_cloud_provider.go @@ -232,8 +232,8 @@ func (ic *IonosCloudCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprov return nil, nil } -// NodeExists returns whether node exists in this cloud provider -func (ic *IonosCloudCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (ic *IonosCloudCloudProvider) HasInstance(node *apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/kamatera/kamatera_cloud_provider.go b/cluster-autoscaler/cloudprovider/kamatera/kamatera_cloud_provider.go index fdd2f0481a57..2c5187ad1d39 100644 --- a/cluster-autoscaler/cloudprovider/kamatera/kamatera_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/kamatera/kamatera_cloud_provider.go @@ -70,8 +70,8 @@ func (k *kamateraCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovide return nil, nil } -// NodeExists returns whether node exists in this cloud provider -func (k *kamateraCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (k *kamateraCloudProvider) HasInstance(node *apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/kubemark/kubemark_linux.go b/cluster-autoscaler/cloudprovider/kubemark/kubemark_linux.go index 3169dfdc0f74..915109b5f963 100644 --- a/cluster-autoscaler/cloudprovider/kubemark/kubemark_linux.go +++ b/cluster-autoscaler/cloudprovider/kubemark/kubemark_linux.go @@ -139,8 +139,8 @@ func (kubemark *KubemarkCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloud return nil, nil } -// NodeExists returns whether node exists in this cloud provider -func (kubemark *KubemarkCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (kubemark *KubemarkCloudProvider) HasInstance(node *apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/kubemark/kubemark_other.go b/cluster-autoscaler/cloudprovider/kubemark/kubemark_other.go index 45da92266228..895ced6d8899 100644 --- a/cluster-autoscaler/cloudprovider/kubemark/kubemark_other.go +++ b/cluster-autoscaler/cloudprovider/kubemark/kubemark_other.go @@ -80,8 +80,8 @@ func (kubemark *KubemarkCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloud return nil, cloudprovider.ErrNotImplemented } -// NodeExists returns whether node exists in this cloud provider -func (kubemark *KubemarkCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (kubemark *KubemarkCloudProvider) HasInstance(node *apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/linode/linode_cloud_provider.go b/cluster-autoscaler/cloudprovider/linode/linode_cloud_provider.go index 8e234fa07901..a5ade015cb52 100644 --- a/cluster-autoscaler/cloudprovider/linode/linode_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/linode/linode_cloud_provider.go @@ -67,8 +67,8 @@ func (l *linodeCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovider. return nil, nil } -// NodeExists returns whether node exists in this cloud provider -func (l *linodeCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (l *linodeCloudProvider) HasInstance(node *apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/magnum/magnum_cloud_provider.go b/cluster-autoscaler/cloudprovider/magnum/magnum_cloud_provider.go index bb2c9ae53ae2..ea72859e39d5 100644 --- a/cluster-autoscaler/cloudprovider/magnum/magnum_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/magnum/magnum_cloud_provider.go @@ -135,8 +135,8 @@ func (mcp *magnumCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovide return nil, nil } -// NodeExists returns whether node exists in this cloud provider -func (mcp *magnumCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (mcp *magnumCloudProvider) HasInstance(node *apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/mocks/CloudProvider.go b/cluster-autoscaler/cloudprovider/mocks/CloudProvider.go index 62a643bac834..f07960f742de 100644 --- a/cluster-autoscaler/cloudprovider/mocks/CloudProvider.go +++ b/cluster-autoscaler/cloudprovider/mocks/CloudProvider.go @@ -177,8 +177,8 @@ func (_m *CloudProvider) NodeGroupForNode(_a0 *v1.Node) (cloudprovider.NodeGroup return r0, r1 } -// NodeExists provides a mock function with given fields: -func (_m *CloudProvider) NodeExists(_a0 *v1.Node) (bool, error) { +// HasInstance provides a mock function with given fields: +func (_m *CloudProvider) HasInstance(_a0 *v1.Node) (bool, error) { ret := _m.Called(_a0) var r0 bool diff --git a/cluster-autoscaler/cloudprovider/oci/oci_cloud_provider.go b/cluster-autoscaler/cloudprovider/oci/oci_cloud_provider.go index a2430513c874..7aa83a1dacd8 100644 --- a/cluster-autoscaler/cloudprovider/oci/oci_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/oci/oci_cloud_provider.go @@ -96,8 +96,8 @@ func (ocp *OciCloudProvider) NodeGroupForNode(n *apiv1.Node) (cloudprovider.Node return ng, err } -// NodeExists returns whether node exists in this cloud provider -func (ocp *OciCloudProvider) NodeExists(n *apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (ocp *OciCloudProvider) HasInstance(n *apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/ovhcloud/ovh_cloud_provider.go b/cluster-autoscaler/cloudprovider/ovhcloud/ovh_cloud_provider.go index 09742563fcef..c3e4fe03f08a 100644 --- a/cluster-autoscaler/cloudprovider/ovhcloud/ovh_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/ovhcloud/ovh_cloud_provider.go @@ -151,8 +151,8 @@ func (provider *OVHCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovi return ng, err } -// NodeExists returns whether node exists in this cloud provider -func (provider *OVHCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (provider *OVHCloudProvider) HasInstance(node *apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/packet/packet_cloud_provider.go b/cluster-autoscaler/cloudprovider/packet/packet_cloud_provider.go index c4b5fe090eff..4ac539b53f29 100644 --- a/cluster-autoscaler/cloudprovider/packet/packet_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/packet/packet_cloud_provider.go @@ -120,8 +120,8 @@ func (pcp *packetCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovide return nil, fmt.Errorf("Could not find group for node: %s", node.Spec.ProviderID) } -// NodeExists returns whether node exists in this cloud provider -func (pcp *packetCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (pcp *packetCloudProvider) HasInstance(node *apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/rancher/rancher_provider.go b/cluster-autoscaler/cloudprovider/rancher/rancher_provider.go index 0414bab773e1..15a72e498ac2 100644 --- a/cluster-autoscaler/cloudprovider/rancher/rancher_provider.go +++ b/cluster-autoscaler/cloudprovider/rancher/rancher_provider.go @@ -171,8 +171,8 @@ func (provider *RancherCloudProvider) NodeGroupForNode(node *corev1.Node) (cloud return nil, nil } -// NodeExists returns whether node exists in this cloud provider -func (provider *RancherCloudProvider) NodeExists(node *corev1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (provider *RancherCloudProvider) HasInstance(node *corev1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/scaleway/scaleway_cloud_provider.go b/cluster-autoscaler/cloudprovider/scaleway/scaleway_cloud_provider.go index 5a8db8ade849..6bd612db321a 100644 --- a/cluster-autoscaler/cloudprovider/scaleway/scaleway_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/scaleway/scaleway_cloud_provider.go @@ -162,8 +162,8 @@ func (scw *scalewayCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovi return scw.nodeGroupForNode(node) } -// NodeExists returns whether node exists in this cloud provider -func (scw *scalewayCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (scw *scalewayCloudProvider) HasInstance(node *apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/tencentcloud/tencentcloud_cloud_provider.go b/cluster-autoscaler/cloudprovider/tencentcloud/tencentcloud_cloud_provider.go index 34b27437bc02..4dc47eea3a23 100644 --- a/cluster-autoscaler/cloudprovider/tencentcloud/tencentcloud_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/tencentcloud/tencentcloud_cloud_provider.go @@ -109,8 +109,8 @@ func (tencentcloud *tencentCloudProvider) NodeGroupForNode(node *apiv1.Node) (cl return asg, nil } -// NodeExists returns whether node exists in this cloud provider -func (tencentcloud *tencentCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (tencentcloud *tencentCloudProvider) HasInstance(node *apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/cloudprovider/test/test_cloud_provider.go b/cluster-autoscaler/cloudprovider/test/test_cloud_provider.go index d27528f661af..d3e0d68474a7 100644 --- a/cluster-autoscaler/cloudprovider/test/test_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/test/test_cloud_provider.go @@ -41,8 +41,8 @@ type OnNodeGroupCreateFunc func(string) error // OnNodeGroupDeleteFunc is a function called when a node group is deleted. type OnNodeGroupDeleteFunc func(string) error -// NodeExists is a function called to determine if a node has been removed from the cloud provider. -type NodeExists func(string) (bool, error) +// HasInstance is a function called to determine if a node has been removed from the cloud provider. +type HasInstance func(string) (bool, error) // TestCloudProvider is a dummy cloud provider to be used in tests. type TestCloudProvider struct { @@ -53,7 +53,7 @@ type TestCloudProvider struct { onScaleDown func(string, string) error onNodeGroupCreate func(string) error onNodeGroupDelete func(string) error - nodeExists func(string) (bool, error) + hasInstance func(string) (bool, error) machineTypes []string machineTemplates map[string]*schedulerframework.NodeInfo priceModel cloudprovider.PricingModel @@ -90,13 +90,13 @@ func NewTestAutoprovisioningCloudProvider(onScaleUp OnScaleUpFunc, onScaleDown O // NewTestNodeDeletionDetectionCloudProvider builds new TestCloudProvider with deletion detection support func NewTestNodeDeletionDetectionCloudProvider(onScaleUp OnScaleUpFunc, onScaleDown OnScaleDownFunc, - deleted NodeExists) *TestCloudProvider { + hasInstance HasInstance) *TestCloudProvider { return &TestCloudProvider{ nodes: make(map[string]string), groups: make(map[string]cloudprovider.NodeGroup), onScaleUp: onScaleUp, onScaleDown: onScaleDown, - nodeExists: deleted, + hasInstance: hasInstance, resourceLimiter: cloudprovider.NewResourceLimiter(make(map[string]int64), make(map[string]int64)), } } @@ -157,14 +157,14 @@ func (tcp *TestCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovider. return group, nil } -// NodeExists returns true if the node is available in cloud provider, +// HasInstance returns true if the node has corresponding instance in cloud provider, // or ErrNotImplemented to fall back to taint-based node deletion in clusterstate // readiness calculation. -func (tcp *TestCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { +func (tcp *TestCloudProvider) HasInstance(node *apiv1.Node) (bool, error) { tcp.Lock() defer tcp.Unlock() - if tcp.nodeExists != nil { - return tcp.nodeExists(node.Name) + if tcp.hasInstance != nil { + return tcp.hasInstance(node.Name) } _, found := tcp.nodes[node.Name] return found, nil diff --git a/cluster-autoscaler/cloudprovider/vultr/vultr_cloud_provider.go b/cluster-autoscaler/cloudprovider/vultr/vultr_cloud_provider.go index 3a0162bdbb37..b02727885560 100644 --- a/cluster-autoscaler/cloudprovider/vultr/vultr_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/vultr/vultr_cloud_provider.go @@ -81,8 +81,8 @@ func (v *vultrCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovider.N return nil, nil } -// NodeExists returns whether node exists in this cloud provider -func (v *vultrCloudProvider) NodeExists(node *apiv1.Node) (bool, error) { +// HasInstance returns whether a given node has a corresponding instance in this cloud provider +func (v *vultrCloudProvider) HasInstance(node *apiv1.Node) (bool, error) { return true, cloudprovider.ErrNotImplemented } diff --git a/cluster-autoscaler/clusterstate/clusterstate.go b/cluster-autoscaler/clusterstate/clusterstate.go index afbc6c129fe9..68b4fa61732e 100644 --- a/cluster-autoscaler/clusterstate/clusterstate.go +++ b/cluster-autoscaler/clusterstate/clusterstate.go @@ -298,7 +298,7 @@ func (csr *ClusterStateRegistry) UpdateNodes(nodes []*apiv1.Node, nodeInfosForGr if err != nil { return err } - cloudProviderNodesRemoved := csr.getCloudProviderDeletedNodes(nodes, cloudProviderNodeInstances) + cloudProviderNodesRemoved := csr.getCloudProviderDeletedNodes(nodes) notRegistered := getNotRegisteredNodes(nodes, cloudProviderNodeInstances, currentTime) csr.Lock() @@ -675,7 +675,7 @@ func (csr *ClusterStateRegistry) GetUnregisteredNodes() []UnregisteredNode { } func (csr *ClusterStateRegistry) updateCloudProviderDeletedNodes(deletedNodes []*apiv1.Node) { - result := make(map[string]*apiv1.Node) + result := make(map[string]*apiv1.Node, len(deletedNodes)+len(csr.deletedNodes)) for _, deleted := range deletedNodes { if prev, found := csr.deletedNodes[deleted.Name]; found { result[deleted.Name] = prev @@ -988,70 +988,28 @@ func getNotRegisteredNodes(allNodes []*apiv1.Node, cloudProviderNodeInstances ma } // Calculates which of the registered nodes in Kubernetes that do not exist in cloud provider. -func (csr *ClusterStateRegistry) getCloudProviderDeletedNodes(allNodes []*apiv1.Node, cloudProviderNodeInstances map[string][]cloudprovider.Instance) []*apiv1.Node { +func (csr *ClusterStateRegistry) getCloudProviderDeletedNodes(allNodes []*apiv1.Node) []*apiv1.Node { nodesRemoved := make([]*apiv1.Node, 0) - currentCloudInstances := make(map[string]string, 0) - registeredNodes := make(map[string]*apiv1.Node, 0) - if len(allNodes) > 0 { - _, err := csr.cloudProvider.NodeExists(allNodes[0]) - // Check if the cloud provider implements nodeExists method - nodeExistsNotImplemented := errors.Is(err, cloudprovider.ErrNotImplemented) - if nodeExistsNotImplemented { - // Fall-back to taint-based node deletion - for _, node := range allNodes { - if deletetaint.HasToBeDeletedTaint(node) { - nodesRemoved = append(nodesRemoved, node) - } - } - } else { - for nodeGroupName, instances := range cloudProviderNodeInstances { - for _, instance := range instances { - currentCloudInstances[instance.Id] = nodeGroupName - } - } - for _, node := range allNodes { - registeredNodes[node.Name] = node - } - - // Fill previously deleted nodes, if they are still registered in Kubernetes - for nodeName, node := range csr.deletedNodes { - // Safety check to prevent flagging Kubernetes nodes as deleted - // if the Cloud Provider instance is re-discovered - _, cloudProviderFound := currentCloudInstances[node.Name] - if _, found := registeredNodes[nodeName]; found && !cloudProviderFound { - // Confirm that node is deleted by cloud provider, instead of - // a not-autoscaled node - nodeExists, existsErr := csr.cloudProvider.NodeExists(node) - if existsErr == nil && !nodeExists { - nodesRemoved = append(nodesRemoved, node) - } - } - } - - // Seek nodes that may have been deleted since last update - // cloudProviderNodeInstances are retrieved by nodeGroup, - // not autoscaled nodes will be excluded - for _, instances := range csr.cloudProviderNodeInstances { - for _, instance := range instances { - if _, found := currentCloudInstances[instance.Id]; !found { - // Check Kubernetes registered nodes for corresponding deleted - // Cloud Provider instance - if kubeNode, kubeNodeFound := registeredNodes[instance.Id]; kubeNodeFound { - // Confirm that node is deleted by cloud provider, instead of - // a not-autoscaled node - nodeExists, existsErr := csr.cloudProvider.NodeExists(kubeNode) - if existsErr == nil && !nodeExists { - nodesRemoved = append(nodesRemoved, kubeNode) - } - } - } - } - } + for _, node := range allNodes { + if !csr.hasCloudProviderInstance(node) { + nodesRemoved = append(nodesRemoved, node) } } return nodesRemoved } +func (csr *ClusterStateRegistry) hasCloudProviderInstance(node *apiv1.Node) bool { + exists, err := csr.cloudProvider.HasInstance(node) + if err == nil { + return exists + } + if !errors.Is(err, cloudprovider.ErrNotImplemented) { + klog.Warningf("Failed to check whether node has cloud instance for %s: %v", node.Name, err) + return exists + } + return !deletetaint.HasToBeDeletedTaint(node) +} + // GetAutoscaledNodesCount calculates and returns the actual and the target number of nodes // belonging to autoscaled node groups in the cluster. func (csr *ClusterStateRegistry) GetAutoscaledNodesCount() (currentSize, targetSize int) { diff --git a/cluster-autoscaler/clusterstate/clusterstate_test.go b/cluster-autoscaler/clusterstate/clusterstate_test.go index 62c320c00b00..755b61eace83 100644 --- a/cluster-autoscaler/clusterstate/clusterstate_test.go +++ b/cluster-autoscaler/clusterstate/clusterstate_test.go @@ -38,6 +38,18 @@ import ( "k8s.io/autoscaler/cluster-autoscaler/utils/backoff" ) +// GetCloudProviderDeletedNodes returns a list of all nodes removed from cloud provider but registered in Kubernetes. +func GetCloudProviderDeletedNodes(csr *ClusterStateRegistry) []*apiv1.Node { + csr.Lock() + defer csr.Unlock() + + result := make([]*apiv1.Node, 0, len(csr.deletedNodes)) + for _, deleted := range csr.deletedNodes { + result = append(result, deleted) + } + return result +} + func TestOKWithScaleUp(t *testing.T) { now := time.Now() @@ -520,7 +532,7 @@ func TestUpcomingNodes(t *testing.T) { } func TestTaintBasedNodeDeletion(t *testing.T) { - // Create a new Cloud Provider that does not implement the NodeExists check + // Create a new Cloud Provider that does not implement the HasInstance check // it will return the ErrNotImplemented error instead. provider := testprovider.NewTestNodeDeletionDetectionCloudProvider(nil, nil, func(string) (bool, error) { return false, cloudprovider.ErrNotImplemented }) @@ -659,7 +671,7 @@ func TestCloudProviderDeletedNodes(t *testing.T) { // Nodes are registered correctly between Kubernetes and cloud provider. assert.NoError(t, err) - assert.Equal(t, 0, len(clusterstate.GetCloudProviderDeletedNodes())) + assert.Equal(t, 0, len(GetCloudProviderDeletedNodes(clusterstate))) // The node was removed from Cloud Provider // should be counted as Deleted by cluster state @@ -671,8 +683,8 @@ func TestCloudProviderDeletedNodes(t *testing.T) { err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, ng1_2, noNgNode}, nil, now) assert.NoError(t, err) - assert.Equal(t, 1, len(clusterstate.GetCloudProviderDeletedNodes())) - assert.Equal(t, "ng1-2", clusterstate.GetCloudProviderDeletedNodes()[0].Name) + assert.Equal(t, 1, len(GetCloudProviderDeletedNodes(clusterstate))) + assert.Equal(t, "ng1-2", GetCloudProviderDeletedNodes(clusterstate)[0].Name) assert.Equal(t, 1, clusterstate.GetClusterReadiness().Deleted) // The node is removed from Kubernetes @@ -680,7 +692,7 @@ func TestCloudProviderDeletedNodes(t *testing.T) { err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNgNode}, nil, now) assert.NoError(t, err) - assert.Equal(t, 0, len(clusterstate.GetCloudProviderDeletedNodes())) + assert.Equal(t, 0, len(GetCloudProviderDeletedNodes(clusterstate))) // New Node is added afterwards ng1_3 := BuildTestNode("ng1-3", 1000, 1000) @@ -692,7 +704,7 @@ func TestCloudProviderDeletedNodes(t *testing.T) { err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, ng1_3, noNgNode}, nil, now) assert.NoError(t, err) - assert.Equal(t, 0, len(clusterstate.GetCloudProviderDeletedNodes())) + assert.Equal(t, 0, len(GetCloudProviderDeletedNodes(clusterstate))) // Newly added node is removed from Cloud Provider // should be counted as Deleted by cluster state @@ -704,8 +716,8 @@ func TestCloudProviderDeletedNodes(t *testing.T) { err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNgNode, ng1_3}, nil, now) assert.NoError(t, err) - assert.Equal(t, 1, len(clusterstate.GetCloudProviderDeletedNodes())) - assert.Equal(t, "ng1-3", clusterstate.GetCloudProviderDeletedNodes()[0].Name) + assert.Equal(t, 1, len(GetCloudProviderDeletedNodes(clusterstate))) + assert.Equal(t, "ng1-3", GetCloudProviderDeletedNodes(clusterstate)[0].Name) assert.Equal(t, 1, clusterstate.GetClusterReadiness().Deleted) // Confirm that previously identified deleted Cloud Provider nodes are still included @@ -714,8 +726,8 @@ func TestCloudProviderDeletedNodes(t *testing.T) { err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNgNode, ng1_3}, nil, now) assert.NoError(t, err) - assert.Equal(t, 1, len(clusterstate.GetCloudProviderDeletedNodes())) - assert.Equal(t, "ng1-3", clusterstate.GetCloudProviderDeletedNodes()[0].Name) + assert.Equal(t, 1, len(GetCloudProviderDeletedNodes(clusterstate))) + assert.Equal(t, "ng1-3", GetCloudProviderDeletedNodes(clusterstate)[0].Name) assert.Equal(t, 1, clusterstate.GetClusterReadiness().Deleted) // The node is removed from Kubernetes @@ -723,7 +735,7 @@ func TestCloudProviderDeletedNodes(t *testing.T) { err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNgNode}, nil, now) assert.NoError(t, err) - assert.Equal(t, 0, len(clusterstate.GetCloudProviderDeletedNodes())) + assert.Equal(t, 0, len(GetCloudProviderDeletedNodes(clusterstate))) } func TestUpdateLastTransitionTimes(t *testing.T) { diff --git a/cluster-autoscaler/core/static_autoscaler_test.go b/cluster-autoscaler/core/static_autoscaler_test.go index de38c7f9a049..1ab888414431 100644 --- a/cluster-autoscaler/core/static_autoscaler_test.go +++ b/cluster-autoscaler/core/static_autoscaler_test.go @@ -1083,7 +1083,7 @@ func TestStaticAutoscalerInstanceCreationErrors(t *testing.T) { } return nil }, nil) - provider.On("NodeExists", mock.Anything).Return( + provider.On("HasInstance", mock.Anything).Return( func(node *apiv1.Node) bool { return false }, nil) @@ -1215,7 +1215,7 @@ func TestStaticAutoscalerInstanceCreationErrors(t *testing.T) { provider = &mockprovider.CloudProvider{} provider.On("NodeGroups").Return([]cloudprovider.NodeGroup{nodeGroupC}) provider.On("NodeGroupForNode", mock.Anything).Return(nil, nil) - provider.On("NodeExists", mock.Anything).Return( + provider.On("HasInstance", mock.Anything).Return( func(node *apiv1.Node) bool { return false }, nil) From ab4fff63074892071af51d97f162c5d5ed5d5678 Mon Sep 17 00:00:00 2001 From: Clint Fooken Date: Mon, 7 Nov 2022 08:57:38 -0800 Subject: [PATCH 7/9] Fixing go formatting issue in cloudstack cloud provider code. --- .../cloudprovider/cloudstack/cloudstack_cloud_provider.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cluster-autoscaler/cloudprovider/cloudstack/cloudstack_cloud_provider.go b/cluster-autoscaler/cloudprovider/cloudstack/cloudstack_cloud_provider.go index cf9ef5d5c306..e4537132125e 100644 --- a/cluster-autoscaler/cloudprovider/cloudstack/cloudstack_cloud_provider.go +++ b/cluster-autoscaler/cloudprovider/cloudstack/cloudstack_cloud_provider.go @@ -155,7 +155,7 @@ func BuildCloudStack(opts config.AutoscalingOptions, do cloudprovider.NodeGroupD } return &cloudStackCloudProvider{ - manager: manager, + manager: manager, resourceLimiter: rl, } } From 1198fbcd90074d4f7d74b46ebdc7ae054367721b Mon Sep 17 00:00:00 2001 From: Clint Fooken Date: Mon, 5 Dec 2022 12:44:39 -0800 Subject: [PATCH 8/9] Updating error messaging and fallback behavior of hasCloudProviderInstance. Changing deletedNodes to store empty struct instead of node values, and modifying the helper function to utilize that information for tests. --- .../clusterstate/clusterstate.go | 27 +++------------- .../clusterstate/clusterstate_test.go | 31 ++++++++++--------- 2 files changed, 21 insertions(+), 37 deletions(-) diff --git a/cluster-autoscaler/clusterstate/clusterstate.go b/cluster-autoscaler/clusterstate/clusterstate.go index 68b4fa61732e..2141188eb03a 100644 --- a/cluster-autoscaler/clusterstate/clusterstate.go +++ b/cluster-autoscaler/clusterstate/clusterstate.go @@ -122,7 +122,7 @@ type ClusterStateRegistry struct { acceptableRanges map[string]AcceptableRange incorrectNodeGroupSizes map[string]IncorrectNodeGroupSize unregisteredNodes map[string]UnregisteredNode - deletedNodes map[string]*apiv1.Node + deletedNodes map[string]struct{} candidatesForScaleDown map[string][]string backoff backoff.Backoff lastStatus *api.ClusterAutoscalerStatus @@ -155,7 +155,7 @@ func NewClusterStateRegistry(cloudProvider cloudprovider.CloudProvider, config C acceptableRanges: make(map[string]AcceptableRange), incorrectNodeGroupSizes: make(map[string]IncorrectNodeGroupSize), unregisteredNodes: make(map[string]UnregisteredNode), - deletedNodes: make(map[string]*apiv1.Node), + deletedNodes: make(map[string]struct{}), candidatesForScaleDown: make(map[string][]string), backoff: backoff, lastStatus: emptyStatus, @@ -675,29 +675,13 @@ func (csr *ClusterStateRegistry) GetUnregisteredNodes() []UnregisteredNode { } func (csr *ClusterStateRegistry) updateCloudProviderDeletedNodes(deletedNodes []*apiv1.Node) { - result := make(map[string]*apiv1.Node, len(deletedNodes)+len(csr.deletedNodes)) + result := make(map[string]struct{}, len(deletedNodes)) for _, deleted := range deletedNodes { - if prev, found := csr.deletedNodes[deleted.Name]; found { - result[deleted.Name] = prev - } else { - result[deleted.Name] = deleted - } + result[deleted.Name] = struct{}{} } csr.deletedNodes = result } -// GetCloudProviderDeletedNodes returns a list of all nodes removed from cloud provider but registered in Kubernetes. -func (csr *ClusterStateRegistry) GetCloudProviderDeletedNodes() []*apiv1.Node { - csr.Lock() - defer csr.Unlock() - - result := make([]*apiv1.Node, 0, len(csr.deletedNodes)) - for _, deleted := range csr.deletedNodes { - result = append(result, deleted) - } - return result -} - // UpdateScaleDownCandidates updates scale down candidates func (csr *ClusterStateRegistry) UpdateScaleDownCandidates(nodes []*apiv1.Node, now time.Time) { result := make(map[string][]string) @@ -1004,8 +988,7 @@ func (csr *ClusterStateRegistry) hasCloudProviderInstance(node *apiv1.Node) bool return exists } if !errors.Is(err, cloudprovider.ErrNotImplemented) { - klog.Warningf("Failed to check whether node has cloud instance for %s: %v", node.Name, err) - return exists + klog.Warningf("Failed to check cloud provider has instance for %s: %v", node.Name, err) } return !deletetaint.HasToBeDeletedTaint(node) } diff --git a/cluster-autoscaler/clusterstate/clusterstate_test.go b/cluster-autoscaler/clusterstate/clusterstate_test.go index 755b61eace83..20b207c522b8 100644 --- a/cluster-autoscaler/clusterstate/clusterstate_test.go +++ b/cluster-autoscaler/clusterstate/clusterstate_test.go @@ -38,14 +38,15 @@ import ( "k8s.io/autoscaler/cluster-autoscaler/utils/backoff" ) -// GetCloudProviderDeletedNodes returns a list of all nodes removed from cloud provider but registered in Kubernetes. -func GetCloudProviderDeletedNodes(csr *ClusterStateRegistry) []*apiv1.Node { +// GetCloudProviderDeletedNodeNames returns a list of the names of nodes removed +// from cloud provider but registered in Kubernetes. +func GetCloudProviderDeletedNodeNames(csr *ClusterStateRegistry) []string { csr.Lock() defer csr.Unlock() - result := make([]*apiv1.Node, 0, len(csr.deletedNodes)) - for _, deleted := range csr.deletedNodes { - result = append(result, deleted) + result := make([]string, 0, len(csr.deletedNodes)) + for nodeName, _ := range csr.deletedNodes { + result = append(result, nodeName) } return result } @@ -671,7 +672,7 @@ func TestCloudProviderDeletedNodes(t *testing.T) { // Nodes are registered correctly between Kubernetes and cloud provider. assert.NoError(t, err) - assert.Equal(t, 0, len(GetCloudProviderDeletedNodes(clusterstate))) + assert.Equal(t, 0, len(GetCloudProviderDeletedNodeNames(clusterstate))) // The node was removed from Cloud Provider // should be counted as Deleted by cluster state @@ -683,8 +684,8 @@ func TestCloudProviderDeletedNodes(t *testing.T) { err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, ng1_2, noNgNode}, nil, now) assert.NoError(t, err) - assert.Equal(t, 1, len(GetCloudProviderDeletedNodes(clusterstate))) - assert.Equal(t, "ng1-2", GetCloudProviderDeletedNodes(clusterstate)[0].Name) + assert.Equal(t, 1, len(GetCloudProviderDeletedNodeNames(clusterstate))) + assert.Equal(t, "ng1-2", GetCloudProviderDeletedNodeNames(clusterstate)[0]) assert.Equal(t, 1, clusterstate.GetClusterReadiness().Deleted) // The node is removed from Kubernetes @@ -692,7 +693,7 @@ func TestCloudProviderDeletedNodes(t *testing.T) { err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNgNode}, nil, now) assert.NoError(t, err) - assert.Equal(t, 0, len(GetCloudProviderDeletedNodes(clusterstate))) + assert.Equal(t, 0, len(GetCloudProviderDeletedNodeNames(clusterstate))) // New Node is added afterwards ng1_3 := BuildTestNode("ng1-3", 1000, 1000) @@ -704,7 +705,7 @@ func TestCloudProviderDeletedNodes(t *testing.T) { err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, ng1_3, noNgNode}, nil, now) assert.NoError(t, err) - assert.Equal(t, 0, len(GetCloudProviderDeletedNodes(clusterstate))) + assert.Equal(t, 0, len(GetCloudProviderDeletedNodeNames(clusterstate))) // Newly added node is removed from Cloud Provider // should be counted as Deleted by cluster state @@ -716,8 +717,8 @@ func TestCloudProviderDeletedNodes(t *testing.T) { err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNgNode, ng1_3}, nil, now) assert.NoError(t, err) - assert.Equal(t, 1, len(GetCloudProviderDeletedNodes(clusterstate))) - assert.Equal(t, "ng1-3", GetCloudProviderDeletedNodes(clusterstate)[0].Name) + assert.Equal(t, 1, len(GetCloudProviderDeletedNodeNames(clusterstate))) + assert.Equal(t, "ng1-3", GetCloudProviderDeletedNodeNames(clusterstate)[0]) assert.Equal(t, 1, clusterstate.GetClusterReadiness().Deleted) // Confirm that previously identified deleted Cloud Provider nodes are still included @@ -726,8 +727,8 @@ func TestCloudProviderDeletedNodes(t *testing.T) { err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNgNode, ng1_3}, nil, now) assert.NoError(t, err) - assert.Equal(t, 1, len(GetCloudProviderDeletedNodes(clusterstate))) - assert.Equal(t, "ng1-3", GetCloudProviderDeletedNodes(clusterstate)[0].Name) + assert.Equal(t, 1, len(GetCloudProviderDeletedNodeNames(clusterstate))) + assert.Equal(t, "ng1-3", GetCloudProviderDeletedNodeNames(clusterstate)[0]) assert.Equal(t, 1, clusterstate.GetClusterReadiness().Deleted) // The node is removed from Kubernetes @@ -735,7 +736,7 @@ func TestCloudProviderDeletedNodes(t *testing.T) { err = clusterstate.UpdateNodes([]*apiv1.Node{ng1_1, noNgNode}, nil, now) assert.NoError(t, err) - assert.Equal(t, 0, len(GetCloudProviderDeletedNodes(clusterstate))) + assert.Equal(t, 0, len(GetCloudProviderDeletedNodeNames(clusterstate))) } func TestUpdateLastTransitionTimes(t *testing.T) { From c94740f4375e71f90b35144874a3fca52aa85168 Mon Sep 17 00:00:00 2001 From: Clint Fooken Date: Mon, 5 Dec 2022 13:11:52 -0800 Subject: [PATCH 9/9] Fixing helper function to simplify for loop to retrieve deleted node names. --- cluster-autoscaler/clusterstate/clusterstate_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cluster-autoscaler/clusterstate/clusterstate_test.go b/cluster-autoscaler/clusterstate/clusterstate_test.go index 20b207c522b8..8587c32b6dc7 100644 --- a/cluster-autoscaler/clusterstate/clusterstate_test.go +++ b/cluster-autoscaler/clusterstate/clusterstate_test.go @@ -45,7 +45,7 @@ func GetCloudProviderDeletedNodeNames(csr *ClusterStateRegistry) []string { defer csr.Unlock() result := make([]string, 0, len(csr.deletedNodes)) - for nodeName, _ := range csr.deletedNodes { + for nodeName := range csr.deletedNodes { result = append(result, nodeName) } return result