From a83772486a811e3f22872bf7101c4d09c5c7bd7d Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Thu, 9 Jan 2025 15:13:53 -0800 Subject: [PATCH 1/2] feat: implement force delete --- .../cloudprovider/azure/azure_agent_pool.go | 25 ++++++++++++++++++- .../cloudprovider/azure/azure_scale_set.go | 24 +++++++++++++++++- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/azure/azure_agent_pool.go b/cluster-autoscaler/cloudprovider/azure/azure_agent_pool.go index 9d3731f5d1b9..6bda444937d9 100644 --- a/cluster-autoscaler/cloudprovider/azure/azure_agent_pool.go +++ b/cluster-autoscaler/cloudprovider/azure/azure_agent_pool.go @@ -473,7 +473,30 @@ func (as *AgentPool) DeleteNodes(nodes []*apiv1.Node) error { // ForceDeleteNodes deletes nodes from the group regardless of constraints. func (as *AgentPool) ForceDeleteNodes(nodes []*apiv1.Node) error { - return cloudprovider.ErrNotImplemented + klog.V(6).Infof("Delete nodes requested: %v\n", nodes) + refs := make([]*azureRef, 0, len(nodes)) + for _, node := range nodes { + belongs, err := as.Belongs(node) + if err != nil { + return err + } + + if belongs != true { + return fmt.Errorf("%s belongs to a different asg than %s", node.Name, as.Name) + } + + ref := &azureRef{ + Name: node.Spec.ProviderID, + } + refs = append(refs, ref) + } + + err := as.deleteOutdatedDeployments() + if err != nil { + klog.Warningf("ForceDeleteNodes: failed to cleanup outdated deployments with err: %v.", err) + } + + return as.DeleteInstances(refs) } // Debug returns a debug string for the agent pool. diff --git a/cluster-autoscaler/cloudprovider/azure/azure_scale_set.go b/cluster-autoscaler/cloudprovider/azure/azure_scale_set.go index 0593f7bc8b07..8b3b0d10c2c9 100644 --- a/cluster-autoscaler/cloudprovider/azure/azure_scale_set.go +++ b/cluster-autoscaler/cloudprovider/azure/azure_scale_set.go @@ -632,7 +632,29 @@ func (scaleSet *ScaleSet) DeleteNodes(nodes []*apiv1.Node) error { // ForceDeleteNodes deletes nodes from the group regardless of constraints. func (scaleSet *ScaleSet) ForceDeleteNodes(nodes []*apiv1.Node) error { - return cloudprovider.ErrNotImplemented + klog.V(8).Infof("Delete nodes requested: %q\n", nodes) + refs := make([]*azureRef, 0, len(nodes)) + hasUnregisteredNodes := false + for _, node := range nodes { + belongs, err := scaleSet.Belongs(node) + if err != nil { + return err + } + + if belongs != true { + return fmt.Errorf("%s belongs to a different asg than %s", node.Name, scaleSet.Id()) + } + + if node.Annotations[cloudprovider.FakeNodeReasonAnnotation] == cloudprovider.FakeNodeUnregistered { + hasUnregisteredNodes = true + } + ref := &azureRef{ + Name: node.Spec.ProviderID, + } + refs = append(refs, ref) + } + + return scaleSet.DeleteInstances(refs, hasUnregisteredNodes) } // Id returns ScaleSet id. From 37252f4553f8a36d608c555bdeaa144e1fe4bedb Mon Sep 17 00:00:00 2001 From: Bryce Soghigian Date: Fri, 10 Jan 2025 10:47:10 -0800 Subject: [PATCH 2/2] refactor: force delete --- .../cloudprovider/azure/azure_agent_pool.go | 25 +----------- .../cloudprovider/azure/azure_scale_set.go | 38 +------------------ 2 files changed, 2 insertions(+), 61 deletions(-) diff --git a/cluster-autoscaler/cloudprovider/azure/azure_agent_pool.go b/cluster-autoscaler/cloudprovider/azure/azure_agent_pool.go index 6bda444937d9..cdf925592ca6 100644 --- a/cluster-autoscaler/cloudprovider/azure/azure_agent_pool.go +++ b/cluster-autoscaler/cloudprovider/azure/azure_agent_pool.go @@ -446,34 +446,11 @@ func (as *AgentPool) DeleteNodes(nodes []*apiv1.Node) error { return fmt.Errorf("min size reached, nodes will not be deleted") } - refs := make([]*azureRef, 0, len(nodes)) - for _, node := range nodes { - belongs, err := as.Belongs(node) - if err != nil { - return err - } - - if belongs != true { - return fmt.Errorf("%s belongs to a different asg than %s", node.Name, as.Name) - } - - ref := &azureRef{ - Name: node.Spec.ProviderID, - } - refs = append(refs, ref) - } - - err = as.deleteOutdatedDeployments() - if err != nil { - klog.Warningf("DeleteNodes: failed to cleanup outdated deployments with err: %v.", err) - } - - return as.DeleteInstances(refs) + return as.ForceDeleteNodes(nodes) } // ForceDeleteNodes deletes nodes from the group regardless of constraints. func (as *AgentPool) ForceDeleteNodes(nodes []*apiv1.Node) error { - klog.V(6).Infof("Delete nodes requested: %v\n", nodes) refs := make([]*azureRef, 0, len(nodes)) for _, node := range nodes { belongs, err := as.Belongs(node) diff --git a/cluster-autoscaler/cloudprovider/azure/azure_scale_set.go b/cluster-autoscaler/cloudprovider/azure/azure_scale_set.go index 8b3b0d10c2c9..2dbca3424a72 100644 --- a/cluster-autoscaler/cloudprovider/azure/azure_scale_set.go +++ b/cluster-autoscaler/cloudprovider/azure/azure_scale_set.go @@ -591,43 +591,7 @@ func (scaleSet *ScaleSet) DeleteNodes(nodes []*apiv1.Node) error { if int(size) <= scaleSet.MinSize() { return fmt.Errorf("min size reached, nodes will not be deleted") } - - // Distinguish between unregistered node deletion and normal node deletion - refs := make([]*azureRef, 0, len(nodes)) - hasUnregisteredNodes := false - unregisteredRefs := make([]*azureRef, 0, len(nodes)) - - for _, node := range nodes { - belongs, err := scaleSet.Belongs(node) - if err != nil { - return err - } - - if belongs != true { - return fmt.Errorf("%s belongs to a different asg than %s", node.Name, scaleSet.Id()) - } - - if node.Annotations[cloudprovider.FakeNodeReasonAnnotation] == cloudprovider.FakeNodeUnregistered { - hasUnregisteredNodes = true - } - ref := &azureRef{ - Name: node.Spec.ProviderID, - } - - if node.Annotations[cloudprovider.FakeNodeReasonAnnotation] == cloudprovider.FakeNodeUnregistered { - klog.V(5).Infof("Node: %s type is unregistered..Appending to the unregistered list", node.Name) - unregisteredRefs = append(unregisteredRefs, ref) - } else { - refs = append(refs, ref) - } - } - - if len(unregisteredRefs) > 0 { - klog.V(3).Infof("Removing unregisteredNodes: %v", unregisteredRefs) - return scaleSet.DeleteInstances(unregisteredRefs, true) - } - - return scaleSet.DeleteInstances(refs, hasUnregisteredNodes) + return scaleSet.ForceDeleteNodes(nodes) } // ForceDeleteNodes deletes nodes from the group regardless of constraints.