Centralize dstore/pool/folder in CCM config

akutz · akutz · commit 62fa84c7c51a · 2019-08-07T17:27:56.000-05:00
This patch moves the machine config properties for datastore, resource
pool, and folder into the cluster's cloud provider configuration. CAPV
cannot deploy clusters that do not use the vSphere cloud provider anyway
due to the way ready state is determined by the node ref controller.
diff --git a/cmd/clusterctl/examples/vsphere/cluster.yaml.template b/cmd/clusterctl/examples/vsphere/cluster.yaml.template
@@ -24,6 +24,7 @@ spec:
           secretNamespace: "kube-system"
         virtualCenter:
           "${VSPHERE_SERVER}":
+            datacenters: "${VSPHERE_DATACENTER}"
         network:
           name: "${VSPHERE_NETWORK}"
         workspace:
diff --git a/cmd/clusterctl/examples/vsphere/machines.yaml.template b/cmd/clusterctl/examples/vsphere/machines.yaml.template
@@ -13,9 +13,6 @@ items:
         apiVersion: vsphere.cluster.k8s.io/v1alpha1
         kind: VsphereMachineProviderSpec
         datacenter: "${VSPHERE_DATACENTER}"
-        datastore: "${VSPHERE_DATASTORE}"
-        resourcePool: "${VSPHERE_RESOURCE_POOL}"
-        folder: "${VSPHERE_FOLDER}"
         network:
           devices:
           - networkName: "${VSPHERE_NETWORK}"
diff --git a/cmd/clusterctl/examples/vsphere/machineset.yaml.template b/cmd/clusterctl/examples/vsphere/machineset.yaml.template
@@ -22,9 +22,6 @@ spec:
           apiVersion: vsphere.cluster.k8s.io/v1alpha1
           kind: VsphereMachineProviderSpec
           datacenter: "${VSPHERE_DATACENTER}"
-          datastore: "${VSPHERE_DATASTORE}"
-          resourcePool: "${VSPHERE_RESOURCE_POOL}"
-          folder: "${VSPHERE_FOLDER}"
           network:
             devices:
             - networkName: "${VSPHERE_NETWORK}"
diff --git a/config/crds/vsphere_v1alpha1_vspheremachineproviderspec.yaml b/config/crds/vsphere_v1alpha1_vspheremachineproviderspec.yaml
@@ -23,24 +23,12 @@ spec:
           description: Datacenter is the name or inventory path of the datacenter
             where this machine's VM is created/located.
           type: string
-        datastore:
-          description: Datastore is the name or inventory path of the datastore where
-            this machine's VM is created/located. When omitted, GoVmomi's DatastoreOrDefault
-            method is used to determine the default datastore. It is recommended to
-            explicitly set this value.
-          type: string
         diskGiB:
           description: DiskGiB is the size of a virtual machine's disk, in GiB. Defaults
             to the analogue property value in the template from which this machine
             is cloned.
           format: int32
           type: integer
-        folder:
-          description: Folder is the name or inventory path of the folder where this
-            machine's VM is created/located. When omitted, GoVmomi's FolderOrDefault
-            method is used to determine the default folder. It is recommended to explicitly
-            set this value.
-          type: string
         kind:
           description: 'Kind is a string value representing the REST resource this
             object represents. Servers may infer this from the endpoint the client
@@ -342,7 +330,7 @@ spec:
           properties:
             devices:
               description: Devices is the list of network devices used by the virtual
-                machine.
+                machine. TODO(akutz) Make sure at least one network matches the             ClusterSpec.CloudProviderConfiguration.Network.Name
               items:
                 properties:
                   dhcp4:
@@ -469,12 +457,6 @@ spec:
             template from which this machine is cloned.
           format: int32
           type: integer
-        resourcePool:
-          description: ResourcePool is the name or inventory path of the resource
-            pool where this machine's VM is created/located. When omitted, GoVmomi's
-            ResourcePoolOrDefault method is used to determine the default resource
-            pool. It is recommended to explicitly set this value.
-          type: string
         template:
           description: Template is the name, inventory path, or instance UUID of the
             template used to clone new machines.
diff --git a/docs/getting_started.md b/docs/getting_started.md
@@ -182,7 +182,7 @@ spec:
         workspace:
           server: "<REDACTED>"
           datacenter: "SDDC-Datacenter"
-          datastore: "WorkloadDatastore"
+          datastore: "DefaultDatastore"
           resourcePool: "Resources"
           folder: "vm"
 ---
@@ -198,9 +198,6 @@ spec:
       apiVersion: vsphere.cluster.k8s.io/v1alpha1
       kind: VsphereMachineProviderSpec
       datacenter: "SDDC-Datacenter"
-      datastore: "DefaultDatastore"
-      resourcePool: "Resources"
-      folder: "vm"
       network:
         devices:
         - networkName: "vm-network-1"
@@ -239,9 +236,6 @@ spec:
           apiVersion: vsphere.cluster.k8s.io/v1alpha1
           kind: VsphereMachineProviderSpec
           datacenter: "SDDC-Datacenter"
-          datastore: "DefaultDatastore"
-          resourcePool: "Resources"
-          folder: "vm"
           network:
             devices:
             - networkName: "vm-network-1"
diff --git a/pkg/apis/vsphere/v1alpha1/cloud/types.go b/pkg/apis/vsphere/v1alpha1/cloud/types.go
@@ -29,7 +29,7 @@ type Config struct {
 
 	// Network is the vSphere cloud provider's network configuration.
 	// +optional
-	Network NetworConfig `gcfg:"Network,omitempty" json:"network,omitempty"`
+	Network NetworkConfig `gcfg:"Network,omitempty" json:"network,omitempty"`
 
 	// Disk is the vSphere cloud provider's disk configuration.
 	// +optional
@@ -148,8 +148,8 @@ type VCenterConfig struct {
 	Thumbprint string `gcfg:"thumbprint,omitempty" json:"thumbprint,omitempty"`
 }
 
-// NetworConfig is the network configuration for the vSphere cloud provider.
-type NetworConfig struct {
+// NetworkConfig is the network configuration for the vSphere cloud provider.
+type NetworkConfig struct {
 	// Name is the name of the network to which VMs are connected.
 	// +optional
 	Name string `gcfg:"public-network,omitempty" json:"name,omitempty"`
diff --git a/pkg/apis/vsphere/v1alpha1/cloud/zz_generated.deepcopy.go b/pkg/apis/vsphere/v1alpha1/cloud/zz_generated.deepcopy.go
diff --git a/pkg/apis/vsphere/v1alpha1/vspheremachineproviderspec_types.go b/pkg/apis/vsphere/v1alpha1/vspheremachineproviderspec_types.go
@@ -43,24 +43,6 @@ type VsphereMachineProviderSpec struct {
 	// Datacenter is the name or inventory path of the datacenter where this
 	// machine's VM is created/located.
 	Datacenter string `json:"datacenter"`
-	// Datastore is the name or inventory path of the datastore where this
-	// machine's VM is created/located.
-	// When omitted, GoVmomi's DatastoreOrDefault method is used to determine
-	// the default datastore. It is recommended to explicitly set this value.
-	// +optional
-	Datastore string `json:"datastore,omitempty"`
-	// ResourcePool is the name or inventory path of the resource pool where this
-	// machine's VM is created/located.
-	// When omitted, GoVmomi's ResourcePoolOrDefault method is used to determine
-	// the default resource pool. It is recommended to explicitly set this value.
-	// +optional
-	ResourcePool string `json:"resourcePool,omitempty"`
-	// Folder is the name or inventory path of the folder where this
-	// machine's VM is created/located.
-	// When omitted, GoVmomi's FolderOrDefault method is used to determine
-	// the default folder. It is recommended to explicitly set this value.
-	// +optional
-	Folder string `json:"folder,omitempty"`
 
 	// Network is the network configuration for this machine's VM.
 	Network NetworkSpec `json:"network"`
@@ -115,6 +97,8 @@ type KubeadmConfiguration struct {
 // NetworkSpec defines the virtual machine's network configuration.
 type NetworkSpec struct {
 	// Devices is the list of network devices used by the virtual machine.
+	// TODO(akutz) Make sure at least one network matches the
+	//             ClusterSpec.CloudProviderConfiguration.Network.Name
 	Devices []NetworkDeviceSpec `json:"devices"`
 
 	// Routes is a list of optional, static routes applied to the virtual
diff --git a/pkg/apis/vsphere/v1alpha1/vspheremachineproviderspec_types_test.go b/pkg/apis/vsphere/v1alpha1/vspheremachineproviderspec_types_test.go
@@ -35,10 +35,8 @@ func TestStorageVsphereMachineProviderSpec(t *testing.T) {
 			Name:      "foo",
 			Namespace: "default",
 		},
-		MachineRef:   "ref123",
-		Datacenter:   "dc1",
-		Datastore:    "ds1",
-		ResourcePool: "rp1",
+		MachineRef: "ref123",
+		Datacenter: "dc1",
 		Network: NetworkSpec{
 			Devices: []NetworkDeviceSpec{
 				{
diff --git a/pkg/cloud/vsphere/services/govmomi/create_test.go b/pkg/cloud/vsphere/services/govmomi/create_test.go
@@ -118,10 +118,7 @@ func TestCreate(t *testing.T) {
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: "vsphereproviderconfig/v1alpha1",
 		},
-		Datacenter:   "",
-		Datastore:    "",
-		ResourcePool: "",
-		Folder:       "",
+		Datacenter: "",
 		Network: v1alpha1.NetworkSpec{
 			Devices: []v1alpha1.NetworkDeviceSpec{
 				{
diff --git a/pkg/cloud/vsphere/services/govmomi/vcenter/clone.go b/pkg/cloud/vsphere/services/govmomi/vcenter/clone.go
@@ -43,17 +43,17 @@ func Clone(ctx *context.MachineContext, userData []byte) error {
 		return err
 	}
 
-	folder, err := ctx.Session.Finder.FolderOrDefault(ctx, ctx.MachineConfig.Folder)
+	folder, err := ctx.Session.Finder.FolderOrDefault(ctx, ctx.ClusterConfig.CloudProviderConfiguration.Workspace.Folder)
 	if err != nil {
 		return errors.Wrapf(err, "unable to get folder for %q", ctx)
 	}
 
-	datastore, err := ctx.Session.Finder.DatastoreOrDefault(ctx, ctx.MachineConfig.Datastore)
+	datastore, err := ctx.Session.Finder.DatastoreOrDefault(ctx, ctx.ClusterConfig.CloudProviderConfiguration.Workspace.Datastore)
 	if err != nil {
 		return errors.Wrapf(err, "unable to get datastore for %q", ctx)
 	}
 
-	pool, err := ctx.Session.Finder.ResourcePoolOrDefault(ctx, ctx.MachineConfig.ResourcePool)
+	pool, err := ctx.Session.Finder.ResourcePoolOrDefault(ctx, ctx.ClusterConfig.CloudProviderConfiguration.Workspace.ResourcePool)
 	if err != nil {
 		return errors.Wrapf(err, "unable to get resource pool for %q", ctx)
 	}
