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] 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()