Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implement force delete for the azure provider #7687

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

feat: implement force delete for the azure provider #7687

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a837724
feat: implement force delete
Bryce-Soghigian Jan 9, 2025
37252f4
refactor: force delete
Bryce-Soghigian Jan 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions cluster-autoscaler/cloudprovider/azure/azure_agent_pool.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,11 @@ func (as *AgentPool) DeleteNodes(nodes []*apiv1.Node) error {
return fmt.Errorf("min size reached, nodes will not be deleted")
}

return as.ForceDeleteNodes(nodes)
}

// ForceDeleteNodes deletes nodes from the group regardless of constraints.
func (as *AgentPool) ForceDeleteNodes(nodes []*apiv1.Node) error {
Bryce-Soghigian marked this conversation as resolved.
Show resolved Hide resolved
refs := make([]*azureRef, 0, len(nodes))
for _, node := range nodes {
belongs, err := as.Belongs(node)
Expand All @@ -463,19 +468,14 @@ func (as *AgentPool) DeleteNodes(nodes []*apiv1.Node) error {
refs = append(refs, ref)
}

err = as.deleteOutdatedDeployments()
err := as.deleteOutdatedDeployments()
if err != nil {
klog.Warningf("DeleteNodes: failed to cleanup outdated deployments with err: %v.", err)
klog.Warningf("ForceDeleteNodes: failed to cleanup outdated deployments with err: %v.", err)
}

return as.DeleteInstances(refs)
}

// ForceDeleteNodes deletes nodes from the group regardless of constraints.
func (as *AgentPool) ForceDeleteNodes(nodes []*apiv1.Node) error {
return cloudprovider.ErrNotImplemented
}

// Debug returns a debug string for the agent pool.
func (as *AgentPool) Debug() string {
return fmt.Sprintf("%s (%d:%d)", as.Name, as.MinSize(), as.MaxSize())
Expand Down
26 changes: 6 additions & 20 deletions cluster-autoscaler/cloudprovider/azure/azure_scale_set.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -591,12 +591,14 @@ func (scaleSet *ScaleSet) DeleteNodes(nodes []*apiv1.Node) error {
if int(size) <= scaleSet.MinSize() {
return fmt.Errorf("min size reached, nodes will not be deleted")
}
return scaleSet.ForceDeleteNodes(nodes)
}

// Distinguish between unregistered node deletion and normal node deletion
// ForceDeleteNodes deletes nodes from the group regardless of constraints.
func (scaleSet *ScaleSet) ForceDeleteNodes(nodes []*apiv1.Node) error {
klog.V(8).Infof("Delete nodes requested: %q\n", nodes)
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 {
Expand All @@ -613,28 +615,12 @@ func (scaleSet *ScaleSet) DeleteNodes(nodes []*apiv1.Node) error {
ref := &azureRef{
Name: node.Spec.ProviderID,
}

if node.Annotations[cloudprovider.FakeNodeReasonAnnotation] == cloudprovider.FakeNodeUnregistered {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the motivation/expectation of this change?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To clean up long unregistered nodes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that not the case before? I'm seeing it being deleted by return scaleSet.DeleteInstances(unregisteredRefs, true).

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)
refs = append(refs, ref)
}

return scaleSet.DeleteInstances(refs, hasUnregisteredNodes)
}

// ForceDeleteNodes deletes nodes from the group regardless of constraints.
func (scaleSet *ScaleSet) ForceDeleteNodes(nodes []*apiv1.Node) error {
return cloudprovider.ErrNotImplemented
}

// Id returns ScaleSet id.
func (scaleSet *ScaleSet) Id() string {
return scaleSet.Name
Expand Down
Loading