Skip to content

Commit

Permalink
Change the reconcile logic to include machines being deleted (#342)
Browse files Browse the repository at this point in the history
This changes the logic from not counting machines that are being deleted
towards the machineset replica to counting them. The reason for the
change is that due to machine deletion taking non-trivial amount of
time, machine deployment rolling update may exceed the expected number
of machines utilized.

For example, for a machine set with 3 replicas and max surge 1, for a
max of 4 machines.
- create new machine in new machineset (4 machines)
- once new machine is ready, delete a machine from old machineset (4 machines, as machine is still being deleted)
- create new machine as deleted machine is not counted (5 machines)

For the duration of the old machine being deleted, we would exceed the
number of machines by 1.
  • Loading branch information
k4leung4 authored and k8s-ci-robot committed Jun 15, 2018
1 parent 3b00279 commit bfdc22a
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 8 deletions.
5 changes: 0 additions & 5 deletions pkg/controller/machineset/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,11 +257,6 @@ func (c *MachineSetControllerImpl) createMachine(machineSet *v1alpha1.MachineSet
// shoudExcludeMachine returns true if the machine should be filtered out, false otherwise.
func shouldExcludeMachine(machineSet *v1alpha1.MachineSet, machine *v1alpha1.Machine) bool {
// Ignore inactive machines.
if machine.DeletionTimestamp != nil || !machine.DeletionTimestamp.IsZero() {
glog.V(4).Infof("Skipping machine (%v), as it is being deleted.", machine.Name)
return true
}

if metav1.GetControllerOf(machine) != nil && !metav1.IsControlledBy(machine, machineSet) {
glog.V(4).Infof("%s not controlled by %v", machine.Name, machineSet.Name)
return true
Expand Down
5 changes: 2 additions & 3 deletions pkg/controller/machineset/reconcile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,13 +165,12 @@ func TestMachineSetControllerReconcileHandler(t *testing.T) {
expectedMachine: machineFromMachineSet(createMachineSet(1, "foo", "bar2", "acme"), "bar2"),
},
{
name: "scenario 9: the current machine is being deleted, thus a machine is created.",
name: "scenario 9: the current machine is being deleted, is still counted towards the machine set, no machine resource is created.",
startingMachineSets: []*v1alpha1.MachineSet{createMachineSet(1, "foo", "bar2", "acme")},
startingMachines: []*v1alpha1.Machine{setMachineDeleting(machineFromMachineSet(createMachineSet(1, "foo", "bar1", "acme"), "bar1"))},
machineSetToSync: "foo",
namespaceToSync: "acme",
expectedActions: []string{"create"},
expectedMachine: machineFromMachineSet(createMachineSet(1, "foo", "bar2", "acme"), "bar2"),
expectedActions: []string{},
},
{
name: "scenario 10: the current machine has no controller refs, owner refs preserved, machine should be adopted.",
Expand Down

0 comments on commit bfdc22a

Please sign in to comment.