Add reconcileMachineWithCloudState() to sync machine status

pkg/cloud/gcp/actuators/machine/actuator.go

@@ -42,6 +42,8 @@ func (a *Actuator) Create(ctx context.Context, cluster *clusterv1.Cluster, machi
 	if err != nil {
 		return fmt.Errorf("failed to create scope for machine %q: %v", machine.Name, err)
 	}
+	// scope and reconciler lifetime is a machine actuator operation
+	// when scope is closed, it will persist to etcd the given machine spec and machine status (if modified)
 	defer scope.Close()
 	return newReconciler(scope).create()
 }

pkg/cloud/gcp/actuators/machine/reconciler.go

@@ -10,6 +10,7 @@ import (
 	machinev1 "github.com/openshift/cluster-api/pkg/apis/machine/v1beta1"
 	"google.golang.org/api/compute/v1"
 	apicorev1 "k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/klog"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -35,6 +36,7 @@ func newReconciler(scope *machineScope) *Reconciler {
 
 // Create creates machine if and only if machine exists, handled by cluster-api
 func (r *Reconciler) create() error {
+	defer r.reconcileMachineWithCloudState()
 	if err := validateMachine(*r.machine, *r.providerSpec); err != nil {
 		return fmt.Errorf("failed validating machine provider spec: %v", err)
 	}
@@ -93,7 +95,7 @@ func (r *Reconciler) create() error {
 	}
 	instance.ServiceAccounts = serviceAccounts
 
-	// userdata
+	// userData
 	userData, err := r.getCustomUserData()
 	if err != nil {
 		return fmt.Errorf("error getting custom user data: %v", err)
@@ -116,9 +118,36 @@ func (r *Reconciler) create() error {
 
 	operation, err := r.computeService.InstancesInsert(r.projectID, zone, instance)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to create instance via compute service: %v", err)
 	}
-	return r.waitUntilOperationCompleted(zone, operation.Name)
+	if op, err := r.waitUntilOperationCompleted(zone, operation.Name); err != nil {
+		return fmt.Errorf("failed to wait for create operation via compute service. Operation status: %v. Error: %v", op, err)
+	}
+	return nil
+}
+
+func (r *Reconciler) reconcileMachineWithCloudState() error {
+	klog.Infof("Reconciling machine object %q with cloud state", r.machine.Name)
+	freshInstance, err := r.computeService.InstancesGet(r.projectID, r.providerSpec.Zone, r.machine.Name)
+	if err != nil {
+		return fmt.Errorf("failed to get instance via compute service: %v", err)
+	}
+
+	if len(freshInstance.NetworkInterfaces) < 1 {
+		return fmt.Errorf("could not find network interfaces for instance %q", freshInstance.Name)
+	}
+	networkInterface := freshInstance.NetworkInterfaces[0]
+
+	nodeAddresses := []v1.NodeAddress{{Type: v1.NodeInternalIP, Address: networkInterface.NetworkIP}}
+	for _, config := range networkInterface.AccessConfigs {
+		nodeAddresses = append(nodeAddresses, v1.NodeAddress{Type: v1.NodeExternalIP, Address: config.NatIP})
+	}
+
+	r.machine.Status.Addresses = nodeAddresses
+	r.providerStatus.InstanceState = &freshInstance.Status
+	r.providerStatus.InstanceID = &freshInstance.Name
+	r.machine.Spec.ProviderID = &r.providerID
+	return nil
 }
 
 func (r *Reconciler) getCustomUserData() (string, error) {
@@ -137,9 +166,11 @@ func (r *Reconciler) getCustomUserData() (string, error) {
 	return base64.StdEncoding.EncodeToString(data), nil
 }
 
-func (r *Reconciler) waitUntilOperationCompleted(zone, operationName string) error {
-	return wait.Poll(operationRetryWait, operationTimeOut, func() (bool, error) {
-		op, err := r.computeService.ZoneOperationsGet(r.projectID, zone, operationName)
+func (r *Reconciler) waitUntilOperationCompleted(zone, operationName string) (*compute.Operation, error) {
+	var op *compute.Operation
+	var err error
+	return op, wait.Poll(operationRetryWait, operationTimeOut, func() (bool, error) {
+		op, err = r.computeService.ZoneOperationsGet(r.projectID, zone, operationName)
 		if err != nil {
 			return false, err
 		}

pkg/cloud/gcp/actuators/machine/reconciler_test.go

@@ -1,30 +1,91 @@
 package machine
 
 import (
+	computeservice "github.com/openshift/cluster-api-provider-gcp/pkg/cloud/gcp/actuators/services/compute"
+	corev1 "k8s.io/api/core/v1"
+
 	"testing"
 
+	"fmt"
+
 	gcpv1beta1 "github.com/openshift/cluster-api-provider-gcp/pkg/apis/gcpprovider/v1beta1"
-	computeservice "github.com/openshift/cluster-api-provider-gcp/pkg/cloud/gcp/actuators/services/compute"
-	"github.com/openshift/cluster-api/pkg/apis/machine/v1beta1"
+	machinev1beta1 "github.com/openshift/cluster-api/pkg/apis/machine/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	controllerfake "sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
 func TestCreate(t *testing.T) {
 	_, mockComputeService := computeservice.NewComputeServiceMock()
 	machineScope := machineScope{
-		machine: &v1beta1.Machine{
+		machine: &machinev1beta1.Machine{
 			ObjectMeta: metav1.ObjectMeta{
 				Name:      "",
 				Namespace: "",
 			},
 		},
 		coreClient:     controllerfake.NewFakeClient(),
 		providerSpec:   &gcpv1beta1.GCPMachineProviderSpec{},
+		providerStatus: &gcpv1beta1.GCPMachineProviderStatus{},
 		computeService: mockComputeService,
 	}
 	reconciler := newReconciler(&machineScope)
 	if err := reconciler.create(); err != nil {
 		t.Errorf("reconciler was not expected to return error: %v", err)
 	}
 }
+
+func TestReconcileMachineWithCloudState(t *testing.T) {
+	_, mockComputeService := computeservice.NewComputeServiceMock()
+
+	zone := "us-east1-b"
+	projecID := "testProject"
+	instanceName := "testInstance"
+	machineScope := machineScope{
+		machine: &machinev1beta1.Machine{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      instanceName,
+				Namespace: "",
+			},
+		},
+		coreClient: controllerfake.NewFakeClient(),
+		providerSpec: &gcpv1beta1.GCPMachineProviderSpec{
+			Zone: zone,
+		},
+		projectID:      projecID,
+		providerID:     fmt.Sprintf("gce://%s/%s/%s", projecID, zone, instanceName),
+		providerStatus: &gcpv1beta1.GCPMachineProviderStatus{},
+		computeService: mockComputeService,
+	}
+
+	expectedNodeAddresses := []corev1.NodeAddress{
+		{
+			Type:    "InternalIP",
+			Address: "10.0.0.15",
+		},
+		{
+			Type:    "ExternalIP",
+			Address: "35.243.147.143",
+		},
+	}
+
+	r := newReconciler(&machineScope)
+	if err := r.reconcileMachineWithCloudState(); err != nil {
+		t.Errorf("reconciler was not expected to return error: %v", err)
+	}
+	if r.machine.Status.Addresses[0] != expectedNodeAddresses[0] {
+		t.Errorf("Expected: %s, got: %s", expectedNodeAddresses[0], r.machine.Status.Addresses[0])
+	}
+	if r.machine.Status.Addresses[1] != expectedNodeAddresses[1] {
+		t.Errorf("Expected: %s, got: %s", expectedNodeAddresses[1], r.machine.Status.Addresses[1])
+	}
+
+	if r.providerID != *r.machine.Spec.ProviderID {
+		t.Errorf("Expected: %s, got: %s", r.providerID, *r.machine.Spec.ProviderID)
+	}
+	if *r.providerStatus.InstanceState != "RUNNING" {
+		t.Errorf("Expected: %s, got: %s", "RUNNING", *r.providerStatus.InstanceState)
+	}
+	if *r.providerStatus.InstanceID != instanceName {
+		t.Errorf("Expected: %s, got: %s", instanceName, *r.providerStatus.InstanceID)
+	}
+}

pkg/cloud/gcp/actuators/services/compute/computeservice.go

@@ -11,6 +11,7 @@ import (
 type GCPComputeService interface {
 	InstancesInsert(project string, zone string, instance *compute.Instance) (*compute.Operation, error)
 	ZoneOperationsGet(project string, zone string, operation string) (*compute.Operation, error)
+	InstancesGet(project string, zone string, instance string) (*compute.Instance, error)
 }
 
 type computeService struct {
@@ -37,3 +38,8 @@ func (c *computeService) InstancesInsert(project string, zone string, instance *
 func (c *computeService) ZoneOperationsGet(project string, zone string, operation string) (*compute.Operation, error) {
 	return c.service.ZoneOperations.Get(project, zone, operation).Do()
 }
+
+// A pass through wrapper for compute.Service.Instances.Get(...)
+func (c *computeService) InstancesGet(project string, zone string, instance string) (*compute.Instance, error) {
+	return c.service.Instances.Get(project, zone, instance).Do()
+}

pkg/cloud/gcp/actuators/services/compute/computeservice_mock.go

@@ -7,6 +7,7 @@ import (
 type GCPComputeServiceMock struct {
 	mockInstancesInsert   func(project string, zone string, instance *compute.Instance) (*compute.Operation, error)
 	mockZoneOperationsGet func(project string, zone string, operation string) (*compute.Operation, error)
+	mockInstancesGet      func(project string, zone string, instance string) (*compute.Instance, error)
 }
 
 func (c *GCPComputeServiceMock) InstancesInsert(project string, zone string, instance *compute.Instance) (*compute.Operation, error) {
@@ -23,6 +24,26 @@ func (c *GCPComputeServiceMock) ZoneOperationsGet(project string, zone string, o
 	return c.mockZoneOperationsGet(project, zone, operation)
 }
 
+func (c *GCPComputeServiceMock) InstancesGet(project string, zone string, instance string) (*compute.Instance, error) {
+	return &compute.Instance{
+		Name:         instance,
+		Zone:         zone,
+		MachineType:  "n1-standard-1",
+		CanIpForward: true,
+		NetworkInterfaces: []*compute.NetworkInterface{
+			{
+				NetworkIP: "10.0.0.15",
+				AccessConfigs: []*compute.AccessConfig{
+					{
+						NatIP: "35.243.147.143",
+					},
+				},
+			},
+		},
+		Status: "RUNNING",
+	}, nil
+}
+
 func NewComputeServiceMock() (*compute.Instance, *GCPComputeServiceMock) {
 	var receivedInstance compute.Instance
 	computeServiceMock := GCPComputeServiceMock{