Manage status

pkg/apis/gcpprovider/v1beta1/gcpmachineproviderstatus_types.go

@@ -13,7 +13,14 @@ import (
 type GCPMachineProviderStatus struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
-	//TODO(alberto): Populate this and implement status
+
+	// InstanceID is the ID of the instance in GCP
+	// +optional
+	InstanceID *string `json:"instanceId,omitempty"`
+
+	// InstanceState is the provisioning state of the GCP Instance.
+	// +optional
+	InstanceState *string `json:"instanceState,omitempty"`
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

pkg/apis/gcpprovider/v1beta1/register.go

@@ -7,8 +7,14 @@
 package v1beta1
 
 import (
+	"encoding/json"
+	"fmt"
+
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/klog"
 	"sigs.k8s.io/controller-runtime/pkg/runtime/scheme"
+	"sigs.k8s.io/yaml"
 )
 
 var (
@@ -18,3 +24,67 @@ var (
 	// SchemeBuilder is used to add go types to the GroupVersionKind scheme
 	SchemeBuilder = &scheme.Builder{GroupVersion: SchemeGroupVersion}
 )
+
+// RawExtensionFromProviderSpec marshals the machine provider spec.
+func RawExtensionFromProviderSpec(spec *GCPMachineProviderSpec) (*runtime.RawExtension, error) {
+	if spec == nil {
+		return &runtime.RawExtension{}, nil
+	}
+
+	var rawBytes []byte
+	var err error
+	if rawBytes, err = json.Marshal(spec); err != nil {
+		return nil, fmt.Errorf("error marshalling providerSpec: %v", err)
+	}
+
+	return &runtime.RawExtension{
+		Raw: rawBytes,
+	}, nil
+}
+
+// RawExtensionFromProviderStatus marshals the provider status
+func RawExtensionFromProviderStatus(status *GCPMachineProviderStatus) (*runtime.RawExtension, error) {
+	if status == nil {
+		return &runtime.RawExtension{}, nil
+	}
+
+	var rawBytes []byte
+	var err error
+	if rawBytes, err = json.Marshal(status); err != nil {
+		return nil, fmt.Errorf("error marshalling providerStatus: %v", err)
+	}
+
+	return &runtime.RawExtension{
+		Raw: rawBytes,
+	}, nil
+}
+
+// ProviderSpecFromRawExtension unmarshals the JSON-encoded spec
+func ProviderSpecFromRawExtension(rawExtension *runtime.RawExtension) (*GCPMachineProviderSpec, error) {
+	if rawExtension == nil {
+		return &GCPMachineProviderSpec{}, nil
+	}
+
+	spec := new(GCPMachineProviderSpec)
+	if err := yaml.Unmarshal(rawExtension.Raw, &spec); err != nil {
+		return nil, fmt.Errorf("error unmarshalling providerSpec: %v", err)
+	}
+
+	klog.V(5).Infof("Got provider spec from raw extension: %+v", spec)
+	return spec, nil
+}
+
+// ProviderStatusFromRawExtension unmarshals a raw extension into a GCPMachineProviderStatus type
+func ProviderStatusFromRawExtension(rawExtension *runtime.RawExtension) (*GCPMachineProviderStatus, error) {
+	if rawExtension == nil {
+		return &GCPMachineProviderStatus{}, nil
+	}
+
+	providerStatus := new(GCPMachineProviderStatus)
+	if err := yaml.Unmarshal(rawExtension.Raw, providerStatus); err != nil {
+		return nil, fmt.Errorf("error unmarshalling providerStatus: %v", err)
+	}
+
+	klog.V(5).Infof("Got provider Status from raw extension: %+v", providerStatus)
+	return providerStatus, nil
+}

pkg/apis/gcpprovider/v1beta1/register_test.go

@@ -0,0 +1,81 @@
+package v1beta1
+
+import (
+	"reflect"
+	"testing"
+
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+)
+
+var (
+	expectedProviderSpec = GCPMachineProviderSpec{
+		Zone:         "us-east1-b",
+		MachineType:  "n1-standard-1",
+		Region:       "us-east1",
+		CanIPForward: true,
+		UserDataSecret: &corev1.LocalObjectReference{
+			Name: "myUserData",
+		},
+		NetworkInterfaces: []*GCPNetworkInterface{
+			{
+				Subnetwork: "my-subnet",
+			},
+		},
+	}
+	expectedRawForProviderSpec = `{"metadata":{"creationTimestamp":null},"userDataSecret":{"name":"myUserData"},"canIPForward":true,"deletionProtection":false,"networkInterfaces":[{"subnetwork":"my-subnet"}],"serviceAccounts":null,"machineType":"n1-standard-1","region":"us-east1","zone":"us-east1-b"}`
+
+	instanceID             = "my-instance-id"
+	instanceState          = "RUNNING"
+	expectedProviderStatus = GCPMachineProviderStatus{
+		InstanceID:    &instanceID,
+		InstanceState: &instanceState,
+	}
+	expectedRawForProviderStatus = `{"metadata":{"creationTimestamp":null},"instanceId":"my-instance-id","instanceState":"RUNNING"}`
+)
+
+func TestRawExtensionFromProviderSpec(t *testing.T) {
+	rawExtension, err := RawExtensionFromProviderSpec(&expectedProviderSpec)
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+	if string(rawExtension.Raw) != expectedRawForProviderSpec {
+		t.Errorf("Expected: %s, got: %s", expectedRawForProviderSpec, string(rawExtension.Raw))
+	}
+}
+
+func TestProviderSpecFromRawExtension(t *testing.T) {
+	rawExtension := runtime.RawExtension{
+		Raw: []byte(expectedRawForProviderSpec),
+	}
+	providerSpec, err := ProviderSpecFromRawExtension(&rawExtension)
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+	if reflect.DeepEqual(providerSpec, expectedProviderSpec) {
+		t.Errorf("Expected: %v, got: %v", expectedProviderSpec, providerSpec)
+	}
+}
+
+func TestRawExtensionFromProviderStatus(t *testing.T) {
+	rawExtension, err := RawExtensionFromProviderStatus(&expectedProviderStatus)
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+	if string(rawExtension.Raw) != expectedRawForProviderStatus {
+		t.Errorf("Expected: %s, got: %s", expectedRawForProviderStatus, string(rawExtension.Raw))
+	}
+}
+
+func TestProviderStatusFromRawExtension(t *testing.T) {
+	rawExtension := runtime.RawExtension{
+		Raw: []byte(expectedRawForProviderStatus),
+	}
+	providerStatus, err := ProviderSpecFromRawExtension(&rawExtension)
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+	if reflect.DeepEqual(providerStatus, expectedProviderStatus) {
+		t.Errorf("Expected: %v, got: %v", expectedProviderStatus, providerStatus)
+	}
+}

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/machine_scope.go

@@ -6,6 +6,8 @@ import (
 	"fmt"
 	"net/http"
 
+	"github.com/pkg/errors"
+
 	"golang.org/x/oauth2"
 	"golang.org/x/oauth2/google"
 	"google.golang.org/api/compute/v1"
@@ -15,11 +17,9 @@ import (
 	machinev1 "github.com/openshift/cluster-api/pkg/apis/machine/v1beta1"
 	machineclient "github.com/openshift/cluster-api/pkg/client/clientset_generated/clientset/typed/machine/v1beta1"
 	apicorev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/klog"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	controllerclient "sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/yaml"
 )
 
 const (
@@ -38,6 +38,7 @@ type machineScope struct {
 	machineClient  machineclient.MachineInterface
 	coreClient     controllerclient.Client
 	projectID      string
+	providerID     string
 	computeService computeservice.GCPComputeService
 	machine        *machinev1.Machine
 	providerSpec   *v1beta1.GCPMachineProviderSpec
@@ -47,11 +48,16 @@ type machineScope struct {
 // newMachineScope creates a new MachineScope from the supplied parameters.
 // This is meant to be called for each machine actuator operation.
 func newMachineScope(params machineScopeParams) (*machineScope, error) {
-	providerSpec, err := machineConfigFromProviderSpec(params.machine.Spec.ProviderSpec)
+	providerSpec, err := v1beta1.ProviderSpecFromRawExtension(params.machine.Spec.ProviderSpec.Value)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get machine config: %v", err)
 	}
 
+	providerStatus, err := v1beta1.ProviderStatusFromRawExtension(params.machine.Status.ProviderStatus)
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to get machine provider status")
+	}
+
 	serviceAccountJSON, err := getCredentialsSecret(params.coreClient, *params.machine, *providerSpec)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get serviceAccountJSON: %v", err)
@@ -72,37 +78,56 @@ func newMachineScope(params machineScopeParams) (*machineScope, error) {
 		return nil, fmt.Errorf("error creating compute service: %v", err)
 	}
 	return &machineScope{
-		machineClient:  params.machineClient.Machines(params.machine.Namespace),
-		coreClient:     params.coreClient,
-		projectID:      projectID,
+		machineClient: params.machineClient.Machines(params.machine.Namespace),
+		coreClient:    params.coreClient,
+		projectID:     projectID,
+		// https://github.com/kubernetes/kubernetes/blob/8765fa2e48974e005ad16e65cb5c3acf5acff17b/staging/src/k8s.io/legacy-cloud-providers/gce/gce_util.go#L204
+		providerID:     fmt.Sprintf("gce://%s/%s/%s", projectID, providerSpec.Zone, params.machine.Name),
 		computeService: computeService,
 		machine:        params.machine,
 		providerSpec:   providerSpec,
+		providerStatus: providerStatus,
 	}, nil
 }
 
-// Close the MachineScope by updating the machine spec, machine status.
-func (m *machineScope) Close() {
-	//TODO (alberto): implement this. Status can be refreshed here
+// Close the MachineScope by persisting the machine spec, machine status after reconciling.
+func (s *machineScope) Close() {
+	if s.machineClient == nil {
+		klog.Errorf("No machineClient is set for this scope")
+		return
+	}
+
+	latestMachine, err := s.storeMachineSpec(s.machine)
+	if err != nil {
+		klog.Errorf("[machinescope] failed to update machine %q in namespace %q: %v", s.machine.Name, s.machine.Namespace, err)
+		return
+	}
+
+	_, err = s.storeMachineStatus(latestMachine)
+	if err != nil {
+		klog.Errorf("[machinescope] failed to store provider status for machine %q in namespace %q: %v", s.machine.Name, s.machine.Namespace, err)
+	}
 }
 
-// machineConfigFromProviderSpec tries to decode the JSON-encoded spec, falling back on getting a MachineClass if the value is absent.
-func machineConfigFromProviderSpec(providerConfig machinev1.ProviderSpec) (*v1beta1.GCPMachineProviderSpec, error) {
-	if providerConfig.Value == nil {
-		return nil, fmt.Errorf("unable to find machine provider config: Spec.ProviderSpec.Value is not set")
+func (s *machineScope) storeMachineSpec(machine *machinev1.Machine) (*machinev1.Machine, error) {
+	ext, err := v1beta1.RawExtensionFromProviderSpec(s.providerSpec)
+	if err != nil {
+		return nil, err
 	}
-	return unmarshalProviderSpec(providerConfig.Value)
+
+	machine.Spec.ProviderSpec.Value = ext
+	return s.machineClient.Update(machine)
 }
 
-func unmarshalProviderSpec(spec *runtime.RawExtension) (*v1beta1.GCPMachineProviderSpec, error) {
-	var config v1beta1.GCPMachineProviderSpec
-	if spec != nil {
-		if err := yaml.Unmarshal(spec.Raw, &config); err != nil {
-			return nil, fmt.Errorf("error unmarshalling providerSpec: %v", err)
-		}
+func (s *machineScope) storeMachineStatus(machine *machinev1.Machine) (*machinev1.Machine, error) {
+	ext, err := v1beta1.RawExtensionFromProviderStatus(s.providerStatus)
+	if err != nil {
+		return nil, err
 	}
-	klog.V(5).Infof("Found ProviderSpec: %+v", config)
-	return &config, nil
+
+	s.machine.Status.DeepCopyInto(&machine.Status)
+	machine.Status.ProviderStatus = ext
+	return s.machineClient.UpdateStatus(machine)
 }
 
 // This expects the https://github.com/openshift/cloud-credential-operator to make a secret