Skip to content

Commit

Permalink
remove redundant error checks in mark/unmark deletion functions
Browse files Browse the repository at this point in the history
This change removes a few nil checks against resources returned in the
Mark and Unmark deletion functions of the cluster-autoscaler CAPI
provider. These checks look to see if the returned value for a resource
are nil, but the function will not return nil if it returns an
error[0]. We only need to check the error return as discussed here[1].

[0]
https://github.com/kubernetes/client-go/blob/master/dynamic/simple.go#L234
[1]
https://github.com/openshift/kubernetes-autoscaler/pull/141/files#r414480960
  • Loading branch information
elmiko committed Jun 3, 2020
1 parent 6b9c067 commit d5af08d
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -138,13 +138,9 @@ func (r machineDeploymentScalableResource) UnmarkMachineForDeletion(machine *Mac

func (r machineDeploymentScalableResource) MarkMachineForDeletion(machine *Machine) error {
u, err := r.controller.dynamicclient.Resource(*r.controller.machineResource).Namespace(machine.Namespace).Get(context.TODO(), machine.Name, metav1.GetOptions{})

if err != nil {
return err
}
if u == nil {
return fmt.Errorf("unknown machine %s", machine.Name)
}

u = u.DeepCopy()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,9 @@ func (r machineSetScalableResource) SetSize(nreplicas int32) error {

func (r machineSetScalableResource) MarkMachineForDeletion(machine *Machine) error {
u, err := r.controller.dynamicclient.Resource(*r.controller.machineResource).Namespace(machine.Namespace).Get(context.TODO(), machine.Name, metav1.GetOptions{})

if err != nil {
return err
}
if u == nil {
return fmt.Errorf("unknown machine %s", machine.Name)
}

u = u.DeepCopy()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ limitations under the License.

package clusterapi

import (
"context"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// scalableResource is a resource that can be scaled up and down by
// adjusting its replica count field.
type scalableResource interface {
Expand Down Expand Up @@ -50,3 +56,19 @@ type scalableResource interface {
// UnmarkMachineForDeletion unmarks machine for deletion
UnmarkMachineForDeletion(machine *Machine) error
}

func unmarkMachineForDeletion(controller *machineController, machine *Machine) error {
u, err := controller.dynamicclient.Resource(*controller.machineResource).Namespace(machine.Namespace).Get(context.TODO(), machine.Name, metav1.GetOptions{})
if err != nil {
return err
}

annotations := u.GetAnnotations()
if _, ok := annotations[machineDeleteAnnotationKey]; ok {
delete(annotations, machineDeleteAnnotationKey)
u.SetAnnotations(annotations)
_, updateErr := controller.dynamicclient.Resource(*controller.machineResource).Namespace(u.GetNamespace()).Update(context.TODO(), u, metav1.UpdateOptions{})
return updateErr
}
return nil
}

0 comments on commit d5af08d

Please sign in to comment.