Merge pull request kubernetes-sigs#668 from akutz/feature/rm-moref

Replace managed object reference with provider ID

api/v1alpha2/types.go

@@ -210,9 +210,6 @@ type VirtualMachine struct {
 	// BiosUUID is the VM's BIOS UUID.
 	BiosUUID string `json:"biosUUID"`
 
-	// InstanceUUID is the VM's instance UUID.
-	InstanceUUID string `json:"instanceUUID"`
-
 	// State is the VM's state.
 	State VirtualMachineState `json:"state"`
 

api/v1alpha2/vspheremachine_types.go

@@ -36,12 +36,6 @@ type VSphereMachineSpec struct {
 	// +optional
 	ProviderID *string `json:"providerID,omitempty"`
 
-	// This value is set automatically at runtime and should not be set or
-	// modified by users.
-	// MachineRef is used to lookup the VM.
-	// +optional
-	MachineRef string `json:"machineRef,omitempty"`
-
 	// Template is the name, inventory path, or instance UUID of the template
 	// used to clone new machines.
 	Template string `json:"template"`

config/crd/bases/infrastructure.cluster.x-k8s.io_vspheremachines.yaml

@@ -44,10 +44,6 @@ spec:
                 this machine is cloned.
               format: int32
               type: integer
-            machineRef:
-              description: This value is set automatically at runtime and should not
-                be set or modified by users. MachineRef is used to lookup the VM.
-              type: string
             memoryMiB:
               description: MemoryMiB is the size of a virtual machine's memory, in
                 MiB. Defaults to the analogue property value in the template from

config/crd/bases/infrastructure.cluster.x-k8s.io_vspheremachinetemplates.yaml

@@ -159,11 +159,6 @@ spec:
                         from which this machine is cloned.
                       format: int32
                       type: integer
-                    machineRef:
-                      description: This value is set automatically at runtime and
-                        should not be set or modified by users. MachineRef is used
-                        to lookup the VM.
-                      type: string
                     memoryMiB:
                       description: MemoryMiB is the size of a virtual machine's memory,
                         in MiB. Defaults to the analogue property value in the template

controllers/vspheremachine_controller.go

@@ -39,6 +39,7 @@ import (
 	"sigs.k8s.io/cluster-api-provider-vsphere/pkg/context"
 	"sigs.k8s.io/cluster-api-provider-vsphere/pkg/services"
 	"sigs.k8s.io/cluster-api-provider-vsphere/pkg/services/govmomi"
+	infrautilv1 "sigs.k8s.io/cluster-api-provider-vsphere/pkg/util"
 )
 
 // VSphereMachineReconciler reconciles a VSphereMachine object
@@ -270,7 +271,10 @@ func (r *VSphereMachineReconciler) reconcileNetwork(ctx *context.MachineContext,
 }
 
 func (r *VSphereMachineReconciler) reconcileProviderID(ctx *context.MachineContext, vm infrav1.VirtualMachine, vmService services.VirtualMachineService) error {
-	providerID := fmt.Sprintf("vsphere://%s", vm.BiosUUID)
+	providerID := infrautilv1.ConvertUUIDToProviderID(vm.BiosUUID)
+	if providerID == "" {
+		return errors.Errorf("invalid BIOS UUID %s for %s", vm.BiosUUID, ctx)
+	}
 	if ctx.VSphereMachine.Spec.ProviderID == nil || *ctx.VSphereMachine.Spec.ProviderID != providerID {
 		ctx.VSphereMachine.Spec.ProviderID = &providerID
 		ctx.Logger.V(6).Info("updated provider ID", "provider-id", providerID)

docs/design/machine-controller-reconcile.puml

@@ -7,37 +7,31 @@ start;
 repeat
   :Reconcile;
 
-  if (Machine has a TaskRef) then (yes)
-    if (Task exists) then (yes)
-      if (Task is running/pending) then (yes)
-        #LightGreen:Requeue;
-      elseif (Task is success/failed) then (yes)
-        #edf7de:Remove TaskRef from Machine;
-      endif
-    else (no)
-      #edf7de:Remove TaskRef from Machine;
+  if (Machine has DeletionTimestamp) then (yes)
+    if (Machine has in-flight task) then (yes)
       #LightGreen:Requeue;
-    endif
-  endif
-  if (Is requeued or in error) then (no)
-    if (Machine has a MachineRef) then (no)
-      if (Can find VM by InstanceUUID) then (yes)
-        #edf7de:Assign MachineRef to Machine;
-      endif
-    endif
-    if (Machine has a MachineRef) then (no)
-      if (Can find VM by MachineRef) then (no)
-        #edf7de:Remove MachineRef from Machine;
+    else (no)
+      if (Machine has a ProviderID) then (yes)
+        #edf7de:Find VM by BIOS UUID;
+      else (no)
+        #edf7de:Find VM by instance UUID;
       endif
-    endif
-    if (Machine has DeletionTimestamp) then (yes)
-      if (Machine has a MachineRef) then (yes)
+      if (VM exists) then (yes)
         #edf7de:Delete VM;
         #edf7de:Assign delete TaskRef to Machine;
         #LightGreen:Requeue;
       endif
+    endif
+  else (no)
+    if (Machine has in-flight task) then (yes)
+      #LightGreen:Requeue;
     else (no)
-      if (Machine has a MachineRef) then (yes)
+      if (Machine has a ProviderID) then (yes)
+        #edf7de:Find VM by BIOS UUID;
+      else (no)
+        #edf7de:Find VM by instance UUID;
+      endif
+      if (VM exists) then (yes)
         if (VM metadata matches calculated metadata) then (no)
           #edf7de:Reconfigure VM with calculated metadata;
           #edf7de:Assign reconfigure TaskRef to Machine;

hack/release.sh

@@ -103,6 +103,10 @@ function build_images() {
       MANAGER_IMAGE_NAME=${CAPV_MANAGER_IMAGE_RELEASE}
       MANIFESTS_IMAGE_NAME=${CAPV_MANIFESTS_IMAGE_RELEASE}
       ;;
+    *)
+      # A local dev build
+      MANAGER_IMAGE_NAME="${BUILD_RELEASE_TYPE}-manager"
+      MANIFESTS_IMAGE_NAME="${BUILD_RELEASE_TYPE}-manifests"
   esac
 
   # Manager image
@@ -212,7 +216,7 @@ case "${BUILD_RELEASE_TYPE}" in
     # do nothing
     ;;
   *)
-    fatal "invalid BUILD_RELEASE_TYPE: ${BUILD_RELEASE_TYPE}"
+    echo "custom BUILD_RELEASE_TYPE: ${BUILD_RELEASE_TYPE}"
     ;;
 esac
 

pkg/context/session.go

@@ -93,28 +93,26 @@ func getOrCreateCachedSession(ctx *MachineContext) (*Session, error) {
 	return &session, nil
 }
 
+// FindByBIOSUUID finds an object by its BIOS UUID.
+//
+// To avoid comments about this function's name, please see the Golang
+// WIKI https://github.com/golang/go/wiki/CodeReviewComments#initialisms.
+// This function is named in accordance with the example "XMLHTTP".
+func (s *Session) FindByBIOSUUID(ctx context.Context, uuid string) (object.Reference, error) {
+	return s.findByUUID(ctx, uuid, false)
+}
+
 // FindByInstanceUUID finds an object by its instance UUID.
 func (s *Session) FindByInstanceUUID(ctx context.Context, uuid string) (object.Reference, error) {
-	if s.Client == nil {
-		return nil, errors.New("vSphere client is not initialized")
-	}
-	si := object.NewSearchIndex(s.Client.Client)
-	findFlag := true
-	ref, err := si.FindByUuid(ctx, s.datacenter, uuid, true, &findFlag)
-	if err != nil {
-		return nil, errors.Wrapf(err, "error finding object by instance uuid %q", uuid)
-	}
-	return ref, nil
+	return s.findByUUID(ctx, uuid, true)
 }
 
-// FindByUUID finds an object by its UUID.
-func (s *Session) FindByUUID(ctx context.Context, uuid string) (object.Reference, error) {
+func (s *Session) findByUUID(ctx context.Context, uuid string, findByInstanceUUID bool) (object.Reference, error) {
 	if s.Client == nil {
 		return nil, errors.New("vSphere client is not initialized")
 	}
 	si := object.NewSearchIndex(s.Client.Client)
-	findFlag := false
-	ref, err := si.FindByUuid(ctx, s.datacenter, uuid, true, &findFlag)
+	ref, err := si.FindByUuid(ctx, s.datacenter, uuid, true, &findByInstanceUUID)
 	if err != nil {
 		return nil, errors.Wrapf(err, "error finding object by uuid %q", uuid)
 	}

pkg/services/govmomi/context.go

@@ -0,0 +1,32 @@
+/*
+Copyright 2019 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package govmomi
+
+import (
+	"github.com/vmware/govmomi/object"
+	"github.com/vmware/govmomi/vim25/types"
+
+	infrav1 "sigs.k8s.io/cluster-api-provider-vsphere/api/v1alpha2"
+	"sigs.k8s.io/cluster-api-provider-vsphere/pkg/context"
+)
+
+type virtualMachineContext struct {
+	context.MachineContext
+	Ref   types.ManagedObjectReference
+	Obj   *object.VirtualMachine
+	State *infrav1.VirtualMachine
+}

pkg/services/govmomi/errors.go

@@ -0,0 +1,41 @@
+/*
+Copyright 2019 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package govmomi
+
+import "fmt"
+
+// errNotFound is returned by the findVM function when a VM is not found.
+type errNotFound struct {
+	instanceUUID bool
+	uuid         string
+}
+
+func (e errNotFound) Error() string {
+	if e.instanceUUID {
+		return fmt.Sprintf("vm with instance uuid %s not found", e.uuid)
+	}
+	return fmt.Sprintf("vm with bios uuid %s not found", e.uuid)
+}
+
+func isNotFound(err error) bool {
+	switch err.(type) {
+	case errNotFound, *errNotFound:
+		return true
+	default:
+		return false
+	}
+}