Bugfix/azure wait for disk detach

pkg/controller/drain.go

@@ -303,10 +303,9 @@ func (o *DrainOptions) getPodsForDeletion() (pods []api.Pod, err error) {
 
 func (o *DrainOptions) deletePod(pod api.Pod) error {
 	deleteOptions := &metav1.DeleteOptions{}
-	if o.GracePeriodSeconds >= 0 {
-		gracePeriodSeconds := int64(o.GracePeriodSeconds)
-		deleteOptions.GracePeriodSeconds = &gracePeriodSeconds
-	}
+	gracePeriodSeconds := int64(0)
+	deleteOptions.GracePeriodSeconds = &gracePeriodSeconds
+
 	return o.client.Core().Pods(pod.Namespace).Delete(pod.Name, deleteOptions)
 }
 
@@ -347,7 +346,12 @@ func (o *DrainOptions) deleteOrEvictPods(pods []api.Pod) error {
 	}
 
 	if len(policyGroupVersion) > 0 {
-		return o.evictPods(pods, policyGroupVersion, getPodFn)
+		err := o.evictPods(pods, policyGroupVersion, getPodFn)
+		if err != nil {
+			glog.Warningf("Pod eviction was timed out, Error: %v. \nHowever, drain will continue to forcefully delete the pods by setting graceful termination period to 0s", err)
+		} else {
+			return nil
+		}
 	}
 	return o.deletePods(pods, getPodFn)
 }

pkg/controller/machine.go

@@ -152,8 +152,6 @@ func (c *controller) reconcileClusterMachine(machine *v1alpha1.Machine) error {
 		return nil
 	}
 
-	//glog.Info("REACHED ", actualProviderID, " ", machineID)
-	// Get the latest version of the machine so that we can avoid conflicts
 	machine, err = c.controlMachineClient.Machines(machine.Namespace).Get(machine.Name, metav1.GetOptions{})
 	if err != nil {
 		glog.Errorf("Could not fetch machine object %s", err)
@@ -312,7 +310,7 @@ func (c *controller) updateMachineState(machine *v1alpha1.Machine) (*v1alpha1.Ma
 */
 
 func (c *controller) machineCreate(machine *v1alpha1.Machine, driver driver.Driver) error {
-	glog.V(2).Infof("Creating machine %s, please wait!", machine.Name)
+	glog.V(2).Infof("Creating machine %q, please wait!", machine.Name)
 
 	actualProviderID, nodeName, err := driver.Create()
 	if err != nil {
@@ -331,7 +329,7 @@ func (c *controller) machineCreate(machine *v1alpha1.Machine, driver driver.Driv
 		c.updateMachineStatus(machine, lastOperation, currentStatus)
 		return err
 	}
-	glog.V(2).Infof("Created machine: %s, MachineID: %s", machine.Name, actualProviderID)
+	glog.V(2).Infof("Created machine: %q, MachineID: %s", machine.Name, actualProviderID)
 
 	for {
 		// Get the latest version of the machine so that we can avoid conflicts
@@ -424,7 +422,7 @@ func (c *controller) machineDelete(machine *v1alpha1.Machine, driver driver.Driv
 	var err error
 
 	if finalizers := sets.NewString(machine.Finalizers...); finalizers.Has(DeleteFinalizerName) {
-		glog.V(2).Infof("Deleting Machine %s", machine.Name)
+		glog.V(2).Infof("Deleting Machine %q", machine.Name)
 
 		// If machine was created on the cloud provider
 		machineID, _ := driver.GetExisting()
@@ -458,47 +456,51 @@ func (c *controller) machineDelete(machine *v1alpha1.Machine, driver driver.Driv
 			// Timeout value obtained by subtracting last operation with expected time out period
 			timeOut := metav1.Now().Add(-timeOutDuration).Sub(machine.Status.CurrentStatus.LastUpdateTime.Time)
 
-			// To perform drain 2 conditions must be satified
-			// 1. force-deletion: "True" label must not be present
-			// 2. Deletion operation must be less than 5 minutes old
-			if machine.Labels["force-deletion"] != "True" && timeOut < 0 {
-				buf := bytes.NewBuffer([]byte{})
-				errBuf := bytes.NewBuffer([]byte{})
-
-				nodeName := machine.Status.Node
-				drainOptions := NewDrainOptions(
-					c.targetCoreClient,
-					timeOutDuration, // TODO: Will need to configure timeout
-					nodeName,
-					-1,
-					true,
-					true,
-					true,
-					buf,
-					errBuf,
-				)
-				err = drainOptions.RunDrain()
-				if err != nil {
-					lastOperation := v1alpha1.LastOperation{
-						Description:    "Drain failed - " + err.Error(),
-						State:          v1alpha1.MachineStateFailed,
-						Type:           v1alpha1.MachineOperationDelete,
-						LastUpdateTime: metav1.Now(),
-					}
-					c.updateMachineStatus(machine, lastOperation, machine.Status.CurrentStatus)
-
-					// Machine still tries to terminate after drain failure
-					glog.Warningf("Drain failed for machine %s - \nBuf:%v \nErrBuf:%v \nErr-Message:%v", machine.Name, buf, errBuf, err)
-					return err
+			// To perform forceful drain either one of the below conditions must be satified
+			// 1. force-deletion: "True" label must be present
+			// 2. Deletion operation is more than drain-timeout minutes old
+			if machine.Labels["force-deletion"] == "True" || timeOut > 0 {
+				timeOutDuration = 30 * time.Second
+				glog.V(2).Infof("Force deletion has been triggerred for machine %q", machine.Name)
+			}
+
+			buf := bytes.NewBuffer([]byte{})
+			errBuf := bytes.NewBuffer([]byte{})
+
+			nodeName := machine.Status.Node
+			drainOptions := NewDrainOptions(
+				c.targetCoreClient,
+				timeOutDuration, // TODO: Will need to configure timeout
+				nodeName,
+				-1,
+				true,
+				true,
+				true,
+				buf,
+				errBuf,
+			)
+			err = drainOptions.RunDrain()
+			if err != nil {
+				lastOperation := v1alpha1.LastOperation{
+					Description:    "Drain failed - " + err.Error(),
+					State:          v1alpha1.MachineStateFailed,
+					Type:           v1alpha1.MachineOperationDelete,
+					LastUpdateTime: metav1.Now(),
 				}
-				glog.V(2).Infof("Drain successful for machine %s - %v %v", machine.Name, buf, errBuf)
+				c.updateMachineStatus(machine, lastOperation, machine.Status.CurrentStatus)
+
+				// Machine still tries to terminate after drain failure
+				glog.Warningf("Drain failed for machine %q \nBuf:%v \nErrBuf:%v \nErr-Message:%v", machine.Name, buf, errBuf, err)
+				return err
 			}
+			glog.V(2).Infof("Drain successful for machine %q \nBuf:%v \nErrBuf:%v", machine.Name, buf, errBuf)
+
 			err = driver.Delete()
 		}
 
 		if err != nil {
 			// When machine deletion fails
-			glog.Errorf("Deletion failed: %s", err)
+			glog.Errorf("Deletion failed for machine %q: %s", machine.Name, err)
 
 			lastOperation := v1alpha1.LastOperation{
 				Description:    "Cloud provider message - " + err.Error(),
@@ -512,7 +514,6 @@ func (c *controller) machineDelete(machine *v1alpha1.Machine, driver driver.Driv
 				LastUpdateTime: metav1.Now(),
 			}
 			c.updateMachineStatus(machine, lastOperation, currentStatus)
-
 			return err
 		}
 
@@ -524,7 +525,7 @@ func (c *controller) machineDelete(machine *v1alpha1.Machine, driver driver.Driv
 			return err
 		}
 
-		glog.V(2).Infof("Machine %s deleted successfully", machine.Name)
+		glog.V(2).Infof("Machine %q deleted successfully", machine.Name)
 	}
 	return nil
 }

pkg/controller/machine_test.go

@@ -624,6 +624,173 @@ var _ = Describe("machine", func() {
 		)
 	})
 
+	Describe("#machineDelete", func() {
+		type setup struct {
+			secrets  []*corev1.Secret
+			aws      []*machinev1.AWSMachineClass
+			machines []*machinev1.Machine
+		}
+		type action struct {
+			machine        string
+			fakeProviderID string
+			fakeNodeName   string
+			fakeError      error
+			forceDelete    bool
+		}
+		type expect struct {
+			machine     *machinev1.Machine
+			errOccurred bool
+		}
+		type data struct {
+			setup  setup
+			action action
+			expect expect
+		}
+		objMeta := &metav1.ObjectMeta{
+			GenerateName: "machine",
+			Namespace:    "test",
+		}
+		DescribeTable("##table",
+			func(data *data) {
+				stop := make(chan struct{})
+				defer close(stop)
+
+				machineObjects := []runtime.Object{}
+				for _, o := range data.setup.aws {
+					machineObjects = append(machineObjects, o)
+				}
+				for _, o := range data.setup.machines {
+					machineObjects = append(machineObjects, o)
+				}
+
+				coreObjects := []runtime.Object{}
+				for _, o := range data.setup.secrets {
+					coreObjects = append(coreObjects, o)
+				}
+
+				controller, trackers := createController(stop, objMeta.Namespace, machineObjects, nil, coreObjects)
+				defer trackers.Stop()
+
+				waitForCacheSync(stop, controller)
+
+				action := data.action
+				machine, err := controller.controlMachineClient.Machines(objMeta.Namespace).Get(action.machine, metav1.GetOptions{})
+				Expect(err).ToNot(HaveOccurred())
+
+				fakeDriver := driver.NewFakeDriver(
+					func() (string, string, error) {
+						return action.fakeProviderID, action.fakeNodeName, action.fakeError
+					},
+					func() error {
+						return nil
+					},
+					func() (string, error) {
+						return action.fakeProviderID, action.fakeError
+					},
+				)
+
+				// Create a machine that is to be deleted later
+				err = controller.machineCreate(machine, fakeDriver)
+				Expect(err).ToNot(HaveOccurred())
+
+				// Add finalizers
+				controller.addMachineFinalizers(machine)
+
+				// Fetch the latest machine version
+				machine, err = controller.controlMachineClient.Machines(objMeta.Namespace).Get(action.machine, metav1.GetOptions{})
+				Expect(err).ToNot(HaveOccurred())
+
+				if data.action.forceDelete {
+					// Add labels for force deletion
+					clone := machine.DeepCopy()
+					clone.Labels["force-deletion"] = "True"
+					machine, err = controller.controlMachineClient.Machines(objMeta.Namespace).Update(clone)
+					Expect(err).ToNot(HaveOccurred())
+				}
+
+				// Deletion of machine is triggered
+				err = controller.machineDelete(machine, fakeDriver)
+				Expect(err).To(BeNil())
+
+				machine, err = controller.controlMachineClient.Machines(machine.Namespace).Get(machine.Name, metav1.GetOptions{})
+				if data.expect.errOccurred {
+					Expect(err).To(HaveOccurred())
+				}
+			},
+			Entry("Simple machine delete", &data{
+				setup: setup{
+					secrets: []*corev1.Secret{
+						&corev1.Secret{
+							ObjectMeta: *newObjectMeta(objMeta, 0),
+						},
+					},
+					aws: []*machinev1.AWSMachineClass{
+						&machinev1.AWSMachineClass{
+							ObjectMeta: *newObjectMeta(objMeta, 0),
+							Spec: machinev1.AWSMachineClassSpec{
+								SecretRef: newSecretReference(objMeta, 0),
+							},
+						},
+					},
+					machines: newMachines(1, &machinev1.MachineTemplateSpec{
+						ObjectMeta: *newObjectMeta(objMeta, 0),
+						Spec: machinev1.MachineSpec{
+							Class: machinev1.ClassSpec{
+								Kind: "AWSMachineClass",
+								Name: "machine-0",
+							},
+						},
+					}, nil, nil),
+				},
+				action: action{
+					machine:        "machine-0",
+					fakeProviderID: "fakeID-0",
+					fakeNodeName:   "fakeNode-0",
+					fakeError:      nil,
+				},
+				expect: expect{
+					errOccurred: true,
+				},
+			}),
+			Entry("Machine force deletion", &data{
+				setup: setup{
+					secrets: []*corev1.Secret{
+						&corev1.Secret{
+							ObjectMeta: *newObjectMeta(objMeta, 0),
+						},
+					},
+					aws: []*machinev1.AWSMachineClass{
+						&machinev1.AWSMachineClass{
+							ObjectMeta: *newObjectMeta(objMeta, 0),
+							Spec: machinev1.AWSMachineClassSpec{
+								SecretRef: newSecretReference(objMeta, 0),
+							},
+						},
+					},
+					machines: newMachines(1, &machinev1.MachineTemplateSpec{
+						ObjectMeta: *newObjectMeta(objMeta, 0),
+						Spec: machinev1.MachineSpec{
+							Class: machinev1.ClassSpec{
+								Kind: "AWSMachineClass",
+								Name: "machine-0",
+							},
+						},
+					}, nil, nil),
+				},
+				action: action{
+					machine:        "machine-0",
+					fakeProviderID: "fakeID-0",
+					fakeNodeName:   "fakeNode-0",
+					fakeError:      nil,
+					forceDelete:    true,
+				},
+				expect: expect{
+					errOccurred: true,
+				},
+			}),
+		)
+	})
+
 	Describe("#checkMachineTimeout", func() {
 		type setup struct {
 			machines []*machinev1.Machine