Skip to content

Commit

Permalink
Merge pull request #7613 from walidghallab/err
Browse files Browse the repository at this point in the history
Refactor NewAutoscalerError function.
  • Loading branch information
k8s-ci-robot authored Dec 17, 2024
2 parents e4898a9 + 720f594 commit 7b64836
Show file tree
Hide file tree
Showing 14 changed files with 44 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ func (client *autoscalingGceClientV1) FetchMigTargetSize(migRef GceRef) (int64,
if err != nil {
if err, ok := err.(*googleapi.Error); ok {
if err.Code == http.StatusNotFound {
return 0, errors.NewAutoscalerError(errors.NodeGroupDoesNotExistError, "%s", err.Error())
return 0, errors.NewAutoscalerError(errors.NodeGroupDoesNotExistError, err.Error())
}
}
return 0, err
Expand All @@ -262,7 +262,7 @@ func (client *autoscalingGceClientV1) FetchMigBasename(migRef GceRef) (string, e
igm, err := client.gceService.InstanceGroupManagers.Get(migRef.Project, migRef.Zone, migRef.Name).Context(ctx).Do()
if err != nil {
if err, ok := err.(*googleapi.Error); ok && err.Code == http.StatusNotFound {
return "", errors.NewAutoscalerError(errors.NodeGroupDoesNotExistError, "%s", err.Error())
return "", errors.NewAutoscalerError(errors.NodeGroupDoesNotExistError, err.Error())
}
return "", err
}
Expand All @@ -277,7 +277,7 @@ func (client *autoscalingGceClientV1) FetchListManagedInstancesResults(migRef Gc
if err != nil {
if err, ok := err.(*googleapi.Error); ok {
if err.Code == http.StatusNotFound {
return "", errors.NewAutoscalerError(errors.NodeGroupDoesNotExistError, "%s", err.Error())
return "", errors.NewAutoscalerError(errors.NodeGroupDoesNotExistError, err.Error())
}
}
return "", err
Expand Down Expand Up @@ -785,7 +785,7 @@ func (client *autoscalingGceClientV1) FetchMigTemplateName(migRef GceRef) (Insta
if err != nil {
if err, ok := err.(*googleapi.Error); ok {
if err.Code == http.StatusNotFound {
return InstanceTemplateName{}, errors.NewAutoscalerError(errors.NodeGroupDoesNotExistError, "%s", err.Error())
return InstanceTemplateName{}, errors.NewAutoscalerError(errors.NodeGroupDoesNotExistError, err.Error())
}
}
return InstanceTemplateName{}, err
Expand Down
12 changes: 6 additions & 6 deletions cluster-autoscaler/core/scaledown/actuation/actuator.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ func (a *Actuator) taintNodesSync(NodeGroupViews []*budgets.NodeGroupView) (time
if a.ctx.AutoscalingOptions.DynamicNodeDeleteDelayAfterTaintEnabled {
close(updateLatencyTracker.AwaitOrStopChan)
}
return nodeDeleteDelayAfterTaint, errors.NewAutoscalerError(errors.ApiCallError, "couldn't taint %d nodes with ToBeDeleted", len(failedTaintedNodes))
return nodeDeleteDelayAfterTaint, errors.NewAutoscalerErrorf(errors.ApiCallError, "couldn't taint %d nodes with ToBeDeleted", len(failedTaintedNodes))
}

if a.ctx.AutoscalingOptions.DynamicNodeDeleteDelayAfterTaintEnabled {
Expand Down Expand Up @@ -287,7 +287,7 @@ func (a *Actuator) deleteNodesAsync(nodes []*apiv1.Node, nodeGroup cloudprovider
clusterSnapshot, err := a.createSnapshot(nodes)
if err != nil {
klog.Errorf("Scale-down: couldn't create delete snapshot, err: %v", err)
nodeDeleteResult := status.NodeDeleteResult{ResultType: status.NodeDeleteErrorInternal, Err: errors.NewAutoscalerError(errors.InternalError, "createSnapshot returned error %v", err)}
nodeDeleteResult := status.NodeDeleteResult{ResultType: status.NodeDeleteErrorInternal, Err: errors.NewAutoscalerErrorf(errors.InternalError, "createSnapshot returned error %v", err)}
for _, node := range nodes {
a.nodeDeletionScheduler.AbortNodeDeletion(node, nodeGroup.Id(), drain, "failed to create delete snapshot", nodeDeleteResult)
}
Expand All @@ -298,7 +298,7 @@ func (a *Actuator) deleteNodesAsync(nodes []*apiv1.Node, nodeGroup cloudprovider
pdbs, err := a.ctx.PodDisruptionBudgetLister().List()
if err != nil {
klog.Errorf("Scale-down: couldn't fetch pod disruption budgets, err: %v", err)
nodeDeleteResult := status.NodeDeleteResult{ResultType: status.NodeDeleteErrorInternal, Err: errors.NewAutoscalerError(errors.InternalError, "podDisruptionBudgetLister.List returned error %v", err)}
nodeDeleteResult := status.NodeDeleteResult{ResultType: status.NodeDeleteErrorInternal, Err: errors.NewAutoscalerErrorf(errors.InternalError, "podDisruptionBudgetLister.List returned error %v", err)}
for _, node := range nodes {
a.nodeDeletionScheduler.AbortNodeDeletion(node, nodeGroup.Id(), drain, "failed to fetch pod disruption budgets", nodeDeleteResult)
}
Expand All @@ -317,22 +317,22 @@ func (a *Actuator) deleteNodesAsync(nodes []*apiv1.Node, nodeGroup cloudprovider
nodeInfo, err := clusterSnapshot.GetNodeInfo(node.Name)
if err != nil {
klog.Errorf("Scale-down: can't retrieve node %q from snapshot, err: %v", node.Name, err)
nodeDeleteResult := status.NodeDeleteResult{ResultType: status.NodeDeleteErrorInternal, Err: errors.NewAutoscalerError(errors.InternalError, "nodeInfos.Get for %q returned error: %v", node.Name, err)}
nodeDeleteResult := status.NodeDeleteResult{ResultType: status.NodeDeleteErrorInternal, Err: errors.NewAutoscalerErrorf(errors.InternalError, "nodeInfos.Get for %q returned error: %v", node.Name, err)}
a.nodeDeletionScheduler.AbortNodeDeletion(node, nodeGroup.Id(), drain, "failed to get node info", nodeDeleteResult)
continue
}

podsToRemove, _, _, err := simulator.GetPodsToMove(nodeInfo, a.deleteOptions, a.drainabilityRules, registry, remainingPdbTracker, time.Now())
if err != nil {
klog.Errorf("Scale-down: couldn't delete node %q, err: %v", node.Name, err)
nodeDeleteResult := status.NodeDeleteResult{ResultType: status.NodeDeleteErrorInternal, Err: errors.NewAutoscalerError(errors.InternalError, "GetPodsToMove for %q returned error: %v", node.Name, err)}
nodeDeleteResult := status.NodeDeleteResult{ResultType: status.NodeDeleteErrorInternal, Err: errors.NewAutoscalerErrorf(errors.InternalError, "GetPodsToMove for %q returned error: %v", node.Name, err)}
a.nodeDeletionScheduler.AbortNodeDeletion(node, nodeGroup.Id(), drain, "failed to get pods to move on node", nodeDeleteResult)
continue
}

if !drain && len(podsToRemove) != 0 {
klog.Errorf("Scale-down: couldn't delete empty node %q, new pods got scheduled", node.Name)
nodeDeleteResult := status.NodeDeleteResult{ResultType: status.NodeDeleteErrorInternal, Err: errors.NewAutoscalerError(errors.InternalError, "failed to delete empty node %q, new pods scheduled", node.Name)}
nodeDeleteResult := status.NodeDeleteResult{ResultType: status.NodeDeleteErrorInternal, Err: errors.NewAutoscalerErrorf(errors.InternalError, "failed to delete empty node %q, new pods scheduled", node.Name)}
a.nodeDeletionScheduler.AbortNodeDeletion(node, nodeGroup.Id(), drain, "node is not empty", nodeDeleteResult)
continue
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,16 +149,16 @@ func (d *NodeDeletionBatcher) remove(nodeGroupId string) error {
func deleteNodesFromCloudProvider(ctx *context.AutoscalingContext, scaleStateNotifier nodegroupchange.NodeGroupChangeObserver, nodes []*apiv1.Node) (cloudprovider.NodeGroup, error) {
nodeGroup, err := ctx.CloudProvider.NodeGroupForNode(nodes[0])
if err != nil {
return nodeGroup, errors.NewAutoscalerError(errors.CloudProviderError, "failed to find node group for %s: %v", nodes[0].Name, err)
return nodeGroup, errors.NewAutoscalerErrorf(errors.CloudProviderError, "failed to find node group for %s: %v", nodes[0].Name, err)
}
if nodeGroup == nil || reflect.ValueOf(nodeGroup).IsNil() {
return nil, errors.NewAutoscalerError(errors.InternalError, "picked node that doesn't belong to a node group: %s", nodes[0].Name)
return nil, errors.NewAutoscalerErrorf(errors.InternalError, "picked node that doesn't belong to a node group: %s", nodes[0].Name)
}
if err := nodeGroup.DeleteNodes(nodes); err != nil {
scaleStateNotifier.RegisterFailedScaleDown(nodeGroup,
string(errors.CloudProviderError),
time.Now())
return nodeGroup, errors.NewAutoscalerError(errors.CloudProviderError, "failed to delete nodes from group %s: %v", nodeGroup.Id(), err)
return nodeGroup, errors.NewAutoscalerErrorf(errors.CloudProviderError, "failed to delete nodes from group %s: %v", nodeGroup.Id(), err)
}
return nodeGroup, nil
}
Expand Down
6 changes: 3 additions & 3 deletions cluster-autoscaler/core/scaledown/actuation/drain.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func (e Evictor) drainNodeWithPodsBasedOnPodPriority(ctx *acontext.AutoscalingCo
for _, group := range groups {
for _, pod := range group.FullEvictionPods {
evictionResults[pod.Name] = status.PodEvictionResult{Pod: pod, TimedOut: false,
Err: errors.NewAutoscalerError(errors.UnexpectedScaleDownStateError, "Eviction did not attempted for the pod %s because some of the previous evictions failed", pod.Name)}
Err: errors.NewAutoscalerErrorf(errors.UnexpectedScaleDownStateError, "Eviction did not attempted for the pod %s because some of the previous evictions failed", pod.Name)}
}
}

Expand Down Expand Up @@ -163,7 +163,7 @@ func (e Evictor) waitPodsToDisappear(ctx *acontext.AutoscalingContext, node *api
}
}

return evictionResults, errors.NewAutoscalerError(errors.TransientError, "Failed to drain node %s/%s: pods remaining after timeout", node.Namespace, node.Name)
return evictionResults, errors.NewAutoscalerErrorf(errors.TransientError, "Failed to drain node %s/%s: pods remaining after timeout", node.Namespace, node.Name)
}

func (e Evictor) initiateEviction(ctx *acontext.AutoscalingContext, node *apiv1.Node, fullEvictionPods, bestEffortEvictionPods []*apiv1.Pod, evictionResults map[string]status.PodEvictionResult,
Expand Down Expand Up @@ -207,7 +207,7 @@ func (e Evictor) initiateEviction(ctx *acontext.AutoscalingContext, node *apiv1.
}
}
if len(evictionErrs) != 0 {
return evictionResults, errors.NewAutoscalerError(errors.ApiCallError, "Failed to drain node %s/%s, due to following errors: %v", node.Namespace, node.Name, evictionErrs)
return evictionResults, errors.NewAutoscalerErrorf(errors.ApiCallError, "Failed to drain node %s/%s, due to following errors: %v", node.Namespace, node.Name, evictionErrs)
}
return evictionResults, nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func (ds *GroupDeletionScheduler) ResetAndReportMetrics() {
func (ds *GroupDeletionScheduler) ScheduleDeletion(nodeInfo *framework.NodeInfo, nodeGroup cloudprovider.NodeGroup, batchSize int, drain bool) {
opts, err := nodeGroup.GetOptions(ds.ctx.NodeGroupDefaults)
if err != nil && err != cloudprovider.ErrNotImplemented {
nodeDeleteResult := status.NodeDeleteResult{ResultType: status.NodeDeleteErrorInternal, Err: errors.NewAutoscalerError(errors.InternalError, "GetOptions returned error %v", err)}
nodeDeleteResult := status.NodeDeleteResult{ResultType: status.NodeDeleteErrorInternal, Err: errors.NewAutoscalerErrorf(errors.InternalError, "GetOptions returned error %v", err)}
ds.AbortNodeDeletion(nodeInfo.Node(), nodeGroup.Id(), drain, "failed to get autoscaling options for a node group", nodeDeleteResult)
return
}
Expand Down
2 changes: 1 addition & 1 deletion cluster-autoscaler/core/scaleup/orchestrator/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ func checkUniqueNodeGroups(scaleUpInfos []nodegroupset.ScaleUpInfo) errors.Autos
uniqueGroups := make(map[string]bool)
for _, info := range scaleUpInfos {
if uniqueGroups[info.Group.Id()] {
return errors.NewAutoscalerError(
return errors.NewAutoscalerErrorf(
errors.InternalError,
"assertion failure: detected group double scaling: %s", info.Group.Id(),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ func (o *ScaleUpOrchestrator) UpcomingNodes(nodeInfos map[string]*framework.Node
for nodeGroup, numberOfNodes := range upcomingCounts {
nodeTemplate, found := nodeInfos[nodeGroup]
if !found {
return nil, errors.NewAutoscalerError(errors.InternalError, "failed to find template node for node group %s", nodeGroup)
return nil, errors.NewAutoscalerErrorf(errors.InternalError, "failed to find template node for node group %s", nodeGroup)
}
for i := 0; i < numberOfNodes; i++ {
upcomingNodes = append(upcomingNodes, nodeTemplate)
Expand Down
4 changes: 2 additions & 2 deletions cluster-autoscaler/core/scaleup/resource/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ func (m *Manager) coresMemoryTotal(ctx *context.AutoscalingContext, nodeInfos ma

nodeInfo, found := nodeInfos[nodeGroup.Id()]
if !found {
return 0, 0, errors.NewAutoscalerError(errors.CloudProviderError, "No node info for: %s", nodeGroup.Id())
return 0, 0, errors.NewAutoscalerErrorf(errors.CloudProviderError, "No node info for: %s", nodeGroup.Id())
}

if currentSize > 0 {
Expand Down Expand Up @@ -243,7 +243,7 @@ func (m *Manager) customResourcesTotal(ctx *context.AutoscalingContext, nodeInfo

nodeInfo, found := nodeInfos[nodeGroup.Id()]
if !found {
return nil, errors.NewAutoscalerError(errors.CloudProviderError, "No node info for: %s", nodeGroup.Id())
return nil, errors.NewAutoscalerErrorf(errors.CloudProviderError, "No node info for: %s", nodeGroup.Id())
}

if currentSize > 0 {
Expand Down
6 changes: 3 additions & 3 deletions cluster-autoscaler/expander/factory/expander_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,18 +58,18 @@ func (f *Factory) Build(names []string) (expander.Strategy, errors.AutoscalerErr
strategySeen := false
for i, name := range names {
if _, ok := seenExpanders[name]; ok {
return nil, errors.NewAutoscalerError(errors.InternalError, "Expander %s was specified multiple times, each expander must not be specified more than once", name)
return nil, errors.NewAutoscalerErrorf(errors.InternalError, "Expander %s was specified multiple times, each expander must not be specified more than once", name)
}
if strategySeen {
return nil, errors.NewAutoscalerError(errors.InternalError, "Expander %s came after an expander %s that will always return only one result, this is not allowed since %s will never be used", name, names[i-1], name)
return nil, errors.NewAutoscalerErrorf(errors.InternalError, "Expander %s came after an expander %s that will always return only one result, this is not allowed since %s will never be used", name, names[i-1], name)
}
seenExpanders[name] = struct{}{}

create, known := f.createFunc[name]
if known {
filters = append(filters, create())
} else {
return nil, errors.NewAutoscalerError(errors.InternalError, "Expander %s not supported", name)
return nil, errors.NewAutoscalerErrorf(errors.InternalError, "Expander %s not supported", name)
}
if _, ok := filters[len(filters)-1].(expander.Strategy); ok {
strategySeen = true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (b *BalancingNodeGroupSetProcessor) FindSimilarNodeGroups(context *context.
nodeGroupId := nodeGroup.Id()
nodeInfo, found := nodeInfosForGroups[nodeGroupId]
if !found {
return []cloudprovider.NodeGroup{}, errors.NewAutoscalerError(
return []cloudprovider.NodeGroup{}, errors.NewAutoscalerErrorf(
errors.InternalError,
"failed to find template node for node group %s",
nodeGroupId)
Expand Down Expand Up @@ -88,7 +88,7 @@ func (b *BalancingNodeGroupSetProcessor) BalanceScaleUpBetweenGroups(context *co
for _, ng := range groups {
currentSize, err := ng.TargetSize()
if err != nil {
return []ScaleUpInfo{}, errors.NewAutoscalerError(
return []ScaleUpInfo{}, errors.NewAutoscalerErrorf(
errors.CloudProviderError,
"failed to get node group size: %v", err)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func (p *MixedTemplateNodeInfoProvider) Process(ctx *context.AutoscalingContext,
if _, found := result[id]; !found {
nodeInfo, err := ctx.ClusterSnapshot.GetNodeInfo(node.Name)
if err != nil {
return false, "", caerror.NewAutoscalerError(caerror.InternalError, "error while retrieving node %s from cluster snapshot - this shouldn't happen: %v", node.Name, err)
return false, "", caerror.NewAutoscalerErrorf(caerror.InternalError, "error while retrieving node %s from cluster snapshot - this shouldn't happen: %v", node.Name, err)
}
templateNodeInfo, caErr := simulator.SanitizedTemplateNodeInfoFromNodeInfo(nodeInfo, id, daemonsets, p.forceDaemonSets, taintConfig)
if caErr != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
"k8s.io/autoscaler/cluster-autoscaler/simulator/framework"
"k8s.io/klog/v2"

"k8s.io/autoscaler/cluster-autoscaler/apis/provisioningrequest/autoscaling.x-k8s.io/v1"
v1 "k8s.io/autoscaler/cluster-autoscaler/apis/provisioningrequest/autoscaling.x-k8s.io/v1"
"k8s.io/autoscaler/cluster-autoscaler/clusterstate"
"k8s.io/autoscaler/cluster-autoscaler/context"
"k8s.io/autoscaler/cluster-autoscaler/core/scaleup"
Expand Down Expand Up @@ -99,15 +99,15 @@ func (o *bestEffortAtomicProvClass) Provision(
if _, updateErr := o.client.UpdateProvisioningRequest(pr.ProvisioningRequest); updateErr != nil {
klog.Errorf("failed to add Provisioned=false condition to ProvReq %s/%s, err: %v", pr.Namespace, pr.Name, updateErr)
}
return status.UpdateScaleUpError(&status.ScaleUpStatus{}, errors.NewAutoscalerError(errors.InternalError, "error during ScaleUp: %s", err.Error()))
return status.UpdateScaleUpError(&status.ScaleUpStatus{}, errors.NewAutoscalerErrorf(errors.InternalError, "error during ScaleUp: %s", err.Error()))
}

if len(actuallyUnschedulablePods) == 0 {
// Nothing to do here - everything fits without scale-up.
conditions.AddOrUpdateCondition(pr, v1.Provisioned, metav1.ConditionTrue, conditions.CapacityIsFoundReason, conditions.CapacityIsFoundMsg, metav1.Now())
if _, updateErr := o.client.UpdateProvisioningRequest(pr.ProvisioningRequest); updateErr != nil {
klog.Errorf("failed to add Provisioned=true condition to ProvReq %s/%s, err: %v", pr.Namespace, pr.Name, updateErr)
return status.UpdateScaleUpError(&status.ScaleUpStatus{}, errors.NewAutoscalerError(errors.InternalError, "capacity available, but failed to admit workload: %s", updateErr.Error()))
return status.UpdateScaleUpError(&status.ScaleUpStatus{}, errors.NewAutoscalerErrorf(errors.InternalError, "capacity available, but failed to admit workload: %s", updateErr.Error()))
}
return &status.ScaleUpStatus{Result: status.ScaleUpNotNeeded}, nil
}
Expand All @@ -118,7 +118,7 @@ func (o *bestEffortAtomicProvClass) Provision(
conditions.AddOrUpdateCondition(pr, v1.Provisioned, metav1.ConditionTrue, conditions.CapacityIsProvisionedReason, conditions.CapacityIsProvisionedMsg, metav1.Now())
if _, updateErr := o.client.UpdateProvisioningRequest(pr.ProvisioningRequest); updateErr != nil {
klog.Errorf("failed to add Provisioned=true condition to ProvReq %s/%s, err: %v", pr.Namespace, pr.Name, updateErr)
return st, errors.NewAutoscalerError(errors.InternalError, "scale up requested, but failed to admit workload: %s", updateErr.Error())
return st, errors.NewAutoscalerErrorf(errors.InternalError, "scale up requested, but failed to admit workload: %s", updateErr.Error())
}
return st, nil
}
Expand All @@ -129,7 +129,7 @@ func (o *bestEffortAtomicProvClass) Provision(
klog.Errorf("failed to add Provisioned=false condition to ProvReq %s/%s, err: %v", pr.Namespace, pr.Name, updateErr)
}
if err != nil {
return status.UpdateScaleUpError(&status.ScaleUpStatus{}, errors.NewAutoscalerError(errors.InternalError, "error during ScaleUp: %s", err.Error()))
return status.UpdateScaleUpError(&status.ScaleUpStatus{}, errors.NewAutoscalerErrorf(errors.InternalError, "error during ScaleUp: %s", err.Error()))
}
return st, nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func (o *checkCapacityProvClass) Provision(
// Gather ProvisioningRequests.
prs, err := o.getProvisioningRequestsAndPods(unschedulablePods)
if err != nil {
return status.UpdateScaleUpError(&status.ScaleUpStatus{}, errors.NewAutoscalerError(errors.InternalError, "Error fetching provisioning requests and associated pods: %s", err.Error()))
return status.UpdateScaleUpError(&status.ScaleUpStatus{}, errors.NewAutoscalerErrorf(errors.InternalError, "Error fetching provisioning requests and associated pods: %s", err.Error()))
} else if len(prs) == 0 {
return &status.ScaleUpStatus{Result: status.ScaleUpNotTried}, nil
}
Expand Down Expand Up @@ -213,7 +213,7 @@ func updateRequests(client *provreqclient.ProvisioningRequestClient, prWrappers
_, updErr := client.UpdateProvisioningRequest(provReq)
if updErr != nil {
err := fmt.Errorf("failed to update ProvReq %s/%s, err: %v", provReq.Namespace, provReq.Name, updErr)
scaleUpStatus, _ := status.UpdateScaleUpError(&status.ScaleUpStatus{}, errors.NewAutoscalerError(errors.InternalError, "error during ScaleUp: %s", err.Error()))
scaleUpStatus, _ := status.UpdateScaleUpError(&status.ScaleUpStatus{}, errors.NewAutoscalerErrorf(errors.InternalError, "error during ScaleUp: %s", err.Error()))
lock.Lock()
combinedStatus.Add(scaleUpStatus)
lock.Unlock()
Expand Down
Loading

0 comments on commit 7b64836

Please sign in to comment.