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.

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

Removed any unwanted re-enqueuing of machine objects #139

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions pkg/controller/deployment_progress.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ package controller

import (
"fmt"
"reflect"
"time"

"github.com/golang/glog"
Expand Down Expand Up @@ -108,7 +107,7 @@ func (dc *controller) syncRolloutStatus(allISs []*v1alpha1.MachineSet, newIS *v1
}

// Do not update if there is nothing new to add.
if reflect.DeepEqual(d.Status, newStatus) {
if !statusUpdateRequired(d.Status, newStatus) {
// Requeue the deployment if required.
dc.requeueStuckMachineDeployment(d, newStatus)
return nil
Expand Down
38 changes: 38 additions & 0 deletions pkg/controller/deployment_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ package controller

import (
"fmt"
"reflect"
"sort"
"strconv"
"strings"
Expand Down Expand Up @@ -1017,3 +1018,40 @@ func ResolveFenceposts(maxSurge, maxUnavailable *intstrutil.IntOrString, desired

return int32(surge), int32(unavailable), nil
}

// statusUpdateRequired checks for if status update is required comparing two MachineDeployment statuses
func statusUpdateRequired(old v1alpha1.MachineDeploymentStatus, new v1alpha1.MachineDeploymentStatus) bool {

if old.AvailableReplicas == new.AvailableReplicas &&
old.CollisionCount == new.CollisionCount &&
len(old.FailedMachines) == len(new.FailedMachines) &&
old.ObservedGeneration == new.ObservedGeneration &&
old.ReadyReplicas == new.ReadyReplicas &&
old.Replicas == new.Replicas &&
old.UpdatedReplicas == new.UpdatedReplicas &&
reflect.DeepEqual(old.Conditions, new.Conditions) {
// If all conditions are matching

// Iterate through all new failed machines and check if there
// exists an older machine with same name and description
for _, newMachine := range new.FailedMachines {
found := false

for _, oldMachine := range old.FailedMachines {
if oldMachine.Name == newMachine.Name &&
oldMachine.LastOperation.Description == newMachine.LastOperation.Description {
found = true
continue
}
}

if !found {
return true
}
}

return false
}

return true
}
51 changes: 28 additions & 23 deletions pkg/controller/machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (c *controller) addMachine(obj interface{}) {
glog.Errorf("Couldn't get key for object %+v: %v", obj, err)
return
}
glog.V(4).Info("Adding machine object")
glog.V(4).Infof("Add/Update/Delete machine object %q", key)
c.machineQueue.Add(key)
}

Expand Down Expand Up @@ -184,7 +184,7 @@ func (c *controller) reconcileClusterMachine(machine *v1alpha1.Machine) error {
}
}

c.enqueueMachineAfter(machine, time.Minute*10)
c.enqueueMachineAfter(machine, 10*time.Minute)
return nil
}

Expand All @@ -207,6 +207,7 @@ func (c *controller) addNodeToMachine(obj interface{}) {
return
}

glog.V(4).Infof("Add machine object backing node %q", machine.Name)
c.enqueueMachine(machine)
}

Expand Down Expand Up @@ -545,8 +546,9 @@ func (c *controller) updateMachineStatus(
func (c *controller) updateMachineConditions(machine *v1alpha1.Machine, conditions []v1.NodeCondition) (*v1alpha1.Machine, error) {

var (
msg string
lastOperationType v1alpha1.MachineOperationType
msg string
lastOperationType v1alpha1.MachineOperationType
objectRequiresUpdate bool
)

// Get the latest version of the machine so that we can avoid conflicts
Expand All @@ -556,12 +558,14 @@ func (c *controller) updateMachineConditions(machine *v1alpha1.Machine, conditio
}

clone := machine.DeepCopy()
clone.Status.Conditions = conditions

//glog.Info(c.isHealthy(clone))
if nodeConditionsHaveChanged(clone.Status.Conditions, conditions) {
clone.Status.Conditions = conditions
objectRequiresUpdate = true
}

if clone.Status.CurrentStatus.Phase == v1alpha1.MachineTerminating {
// If machine is already in terminating state, don't update
// If machine is already in terminating state, don't update health status

} else if !c.isHealthy(clone) && clone.Status.CurrentStatus.Phase == v1alpha1.MachineRunning {
// If machine is not healthy, and current state is running,
Expand All @@ -580,6 +584,7 @@ func (c *controller) updateMachineConditions(machine *v1alpha1.Machine, conditio
Type: v1alpha1.MachineOperationHealthCheck,
LastUpdateTime: metav1.Now(),
}
objectRequiresUpdate = true

} else if c.isHealthy(clone) && clone.Status.CurrentStatus.Phase != v1alpha1.MachineRunning {
// If machine is healhy and current machinePhase is not running.
Expand Down Expand Up @@ -609,16 +614,22 @@ func (c *controller) updateMachineConditions(machine *v1alpha1.Machine, conditio
TimeoutActive: false,
LastUpdateTime: metav1.Now(),
}
objectRequiresUpdate = true

}

clone, err = c.controlMachineClient.Machines(clone.Namespace).Update(clone)
if err != nil {
// Keep retrying until update goes through
glog.V(2).Infof("Warning: Updated failed, retrying, error: %q", err)
return c.updateMachineConditions(machine, conditions)
if objectRequiresUpdate {
clone, err = c.controlMachineClient.Machines(clone.Namespace).Update(clone)
if err != nil {
// Keep retrying until update goes through
glog.Warningf("Updated failed, retrying, error: %q", err)
return c.updateMachineConditions(machine, conditions)
}

return clone, nil
}

return clone, nil
return machine, nil
}

func (c *controller) updateMachineFinalizers(machine *v1alpha1.Machine, finalizers []string) {
Expand Down Expand Up @@ -715,7 +726,7 @@ func (c *controller) checkMachineTimeout(machine *v1alpha1.Machine) {
// Timeout value obtained by subtracting last operation with expected time out period
timeOut := metav1.Now().Add(-timeOutDuration).Sub(machine.Status.CurrentStatus.LastUpdateTime.Time)
if timeOut > 0 {
// If machine health timeout occurs while joining or rejoining of machine
// Machine health timeout occurs while joining or rejoining of machine

if machine.Status.CurrentStatus.Phase == v1alpha1.MachinePending {
// Timeout occurred while machine creation
Expand Down Expand Up @@ -748,20 +759,14 @@ func (c *controller) checkMachineTimeout(machine *v1alpha1.Machine) {
// Log the error message for machine failure
glog.Error(description)

// Update the machine status to reflect the changes
c.updateMachineStatus(machine, lastOperation, currentStatus)

} else {
// If timeout has not occurred, re-enqueue the machine
// after a specified sleep time
c.enqueueMachineAfter(machine, sleepTime)
currentStatus = v1alpha1.CurrentStatus{
Phase: machine.Status.CurrentStatus.Phase,
TimeoutActive: true,
LastUpdateTime: machine.Status.CurrentStatus.LastUpdateTime,
}
lastOperation = machine.Status.LastOperation
}

// Update the machine status to reflect the changes
c.updateMachineStatus(machine, lastOperation, currentStatus)
}
}

Expand Down
16 changes: 16 additions & 0 deletions pkg/controller/machine_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,3 +199,19 @@ func (c *controller) validateMachineClass(classSpec *v1alpha1.ClassSpec) (interf

return MachineClass, secretRef, nil
}

// nodeConditionsHaveChanged compares two node statuses to see if any of the statuses have changed
func nodeConditionsHaveChanged(machineConditions []v1.NodeCondition, nodeConditions []v1.NodeCondition) bool {

if len(machineConditions) != len(nodeConditions) {
return true
}

for i := range nodeConditions {
if nodeConditions[i].Status != machineConditions[i].Status {
return true
}
}

return false
}
2 changes: 1 addition & 1 deletion pkg/controller/machineset.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func (c *controller) machineSetUpdate(old, cur interface{}) {
// that bad as MachineSets that haven't met expectations yet won't
// sync, and all the listing is done using local stores.
if oldMachineSet.Spec.Replicas != currentMachineSet.Spec.Replicas {
glog.V(4).Infof("%v %v updated. Desired machine count change: %d->%d", currentMachineSet.Name, oldMachineSet.Spec.Replicas, currentMachineSet.Spec.Replicas)
glog.V(4).Infof("%v updated. Desired machine count change: %d->%d", currentMachineSet.Name, oldMachineSet.Spec.Replicas, currentMachineSet.Spec.Replicas)
}
c.enqueueMachineSet(currentMachineSet)
}
Expand Down