Address review comments

openshift · May 30, 2019 · 7fc19a8 · 7fc19a8
1 parent b0ef43f
commit 7fc19a8
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 20 deletions.
diff --git a/pkg/cloud/gcp/actuators/machine/actuator.go b/pkg/cloud/gcp/actuators/machine/actuator.go
@@ -95,6 +95,5 @@ func (a *Actuator) Delete(ctx context.Context, cluster *clusterv1.Cluster, machi
 	if err != nil {
 		return fmt.Errorf(scopeFailFmt, machine.Name, err)
 	}
-	defer scope.Close()
 	return newReconciler(scope).delete()
 }
diff --git a/pkg/cloud/gcp/actuators/machine/reconciler.go b/pkg/cloud/gcp/actuators/machine/reconciler.go
@@ -256,12 +256,12 @@ func (r *Reconciler) exists() (bool, error) {
 	instance, err := r.computeService.InstancesGet(r.projectID, zone, r.machine.Name)
 	if err == nil {
 		switch instance.Status {
-		case "PROVISIONING", "REPAIRING", "RUNNING", "STAGING":
-			klog.Infof("Machine %q already exists", r.machine.Name)
-			return true, nil
-		default:
+		case "TERMINATED":
 			klog.Infof("Machine %q is considered as non existent as its status is %q", instance.Status)
 			return false, nil
+		default:
+			klog.Infof("Machine %q already exists", r.machine.Name)
+			return true, nil
 		}
 	}
 	if isNotFoundError(err) {
@@ -273,23 +273,19 @@ func (r *Reconciler) exists() (bool, error) {
 
 // Returns true if machine exists.
 func (r *Reconciler) delete() error {
-	instance, err := r.computeService.InstancesGet(r.projectID, r.providerSpec.Zone, r.machine.Name)
+	exists, err := r.exists()
 	if err != nil {
 		return err
 	}
-	if instance == nil {
-		klog.Infof("Machine %q not found during delete, skipping", r.machine.Name)
+	if !exists {
+		klog.Infof("Machine %v not found during delete, skipping", r.machine.Name)
 		return nil
 	}
-	if _, err = r.computeService.InstancesDelete(r.projectID, r.providerSpec.Zone, r.machine.Name); err != nil {
+	if _, err = r.computeService.InstancesDelete(string(r.machine.UID), r.projectID, r.providerSpec.Zone, r.machine.Name); err != nil {
 		return fmt.Errorf("failed to delete instance via compute service: %v", err)
 	}
-	r.providerStatus.InstanceState = &instance.Status
-	if instance.Status != "TERMINATED" {
-		klog.Infof("machine %q status is %q, requeuing...", r.machine.Name, instance.Status)
-		return &clustererror.RequeueAfterError{RequeueAfter: requeueAfterSeconds * time.Second}
-	}
-	return nil
+	klog.Infof("machine %q status is exists, requeuing...", r.machine.Name)
+	return &clustererror.RequeueAfterError{RequeueAfter: requeueAfterSeconds * time.Second}
 }
 
 func (r *Reconciler) validateZone() error {

diff --git a/pkg/cloud/gcp/actuators/services/compute/computeservice.go b/pkg/cloud/gcp/actuators/services/compute/computeservice.go
@@ -1,14 +1,15 @@
 package computeservice
 
 import (
-	"google.golang.org/api/compute/v1"
 	"net/http"
+
+	"google.golang.org/api/compute/v1"
 )
 
 // GCPComputeService is a pass through wrapper for google.golang.org/api/compute/v1/compute
 // to enable tests to mock this struct and control behavior.
 type GCPComputeService interface {
-	InstancesDelete(project string, zone string, instance string) (*compute.Operation, error)
+	InstancesDelete(requestId string, project string, zone string, instance string) (*compute.Operation, error)
 	InstancesInsert(project string, zone string, instance *compute.Instance) (*compute.Operation, error)
 	InstancesGet(project string, zone string, instance string) (*compute.Instance, error)
 	ZonesGet(project string, zone string) (*compute.Zone, error)
@@ -44,8 +45,8 @@ func (c *computeService) InstancesGet(project string, zone string, instance stri
 	return c.service.Instances.Get(project, zone, instance).Do()
 }
 
-func (c *computeService) InstancesDelete(project string, zone string, instance string) (*compute.Operation, error) {
-	return c.service.Instances.Delete(project, zone, instance).Do()
+func (c *computeService) InstancesDelete(requestId string, project string, zone string, instance string) (*compute.Operation, error) {
+	return c.service.Instances.Delete(project, zone, instance).RequestId(requestId).Do()
 }
 
 func (c *computeService) ZonesGet(project string, zone string) (*compute.Zone, error) {

diff --git a/pkg/cloud/gcp/actuators/services/compute/computeservice_mock.go b/pkg/cloud/gcp/actuators/services/compute/computeservice_mock.go
@@ -17,7 +17,7 @@ func (c *GCPComputeServiceMock) InstancesInsert(project string, zone string, ins
 	return c.mockInstancesInsert(project, zone, instance)
 }
 
-func (c *GCPComputeServiceMock) InstancesDelete(project string, zone string, instance string) (*compute.Operation, error) {
+func (c *GCPComputeServiceMock) InstancesDelete(requestId string, project string, zone string, instance string) (*compute.Operation, error) {
 	return &compute.Operation{
 		Status: "DONE",
 	}, nil