Merge pull request #732 from alexander-demichev/fixwebhook

Bug 1882723: vSphere: Create warning on invalid specs and remove defaults

hack/fetch_ext_bins.sh

@@ -26,7 +26,7 @@ if [ -n "$TRACE" ]; then
   set -x
 fi
 
-k8s_version=1.18.8
+k8s_version=1.19.2
 etcd_version=3.4.10
 goarch=amd64
 goos="unknown"
@@ -93,26 +93,15 @@ function fetch_tools {
   fi
 
   header_text "fetching tools"
+  kb_tools_archive_name="kubebuilder-tools-$k8s_version-$goos-$goarch.tar.gz"
+  kb_tools_download_url="https://storage.googleapis.com/kubebuilder-tools/$kb_tools_archive_name"
 
-  mkdir -p "${kb_root_dir}/bin"
-
-  k8s_download_root="https://dl.k8s.io/v${k8s_version}/bin/${goos}/${goarch}"
-
-  curl -fsL "${k8s_download_root}/kubectl" -o "${kb_root_dir}/bin/kubectl"
-  chmod +x "${kb_root_dir}/bin/kubectl"
-
-  curl -fsL "${k8s_download_root}/kube-apiserver" -o "${kb_root_dir}/bin/kube-apiserver"
-  chmod +x "${kb_root_dir}/bin/kube-apiserver"
-
-  etcd_os_version="etcd-v${etcd_version}-${goos}-${goarch}"
-  etcd_archive="${etcd_os_version}.tar.gz"
-  etcd_download_url="https://github.com/etcd-io/etcd/releases/download/v${etcd_version}/${etcd_archive}"
-  curl -fsL "${etcd_download_url}" -o "${kb_root_dir}/${etcd_archive}"
+  kb_tools_archive_path="$tmp_root/$kb_tools_archive_name"
+  if [ ! -f $kb_tools_archive_path ]; then
+    curl -fsL ${kb_tools_download_url} -o "$kb_tools_archive_path"
+  fi
 
-  tar -zvxf "${kb_root_dir}/${etcd_archive}" -o "${etcd_os_version}/etcd"
-  mv "${etcd_os_version}/etcd" "${kb_root_dir}/bin/etcd"
-  rm "${kb_root_dir}/${etcd_archive}"
-  rm -rf "${etcd_os_version}"
+  tar -zvxf "$kb_tools_archive_path" -C "$tmp_root/"
 }
 
 function setup_envs {

pkg/apis/machine/v1beta1/machine_webhook.go

@@ -1035,13 +1035,13 @@ func validateVSphere(m *Machine, config *admissionConfig) (bool, []string, utile
 	errs = append(errs, validateVSphereNetwork(providerSpec.Network, field.NewPath("providerSpec", "network"))...)
 
 	if providerSpec.NumCPUs < minVSphereCPU {
-		warnings = append(warnings, fmt.Sprintf("providerSpec.numCPUs: %d is less than the minimum value (%d): the minimum value will be used instead", providerSpec.NumCPUs, minVSphereCPU))
+		warnings = append(warnings, fmt.Sprintf("providerSpec.numCPUs: %d is missing or less than the minimum value (%d): nodes may not boot correctly", providerSpec.NumCPUs, minVSphereCPU))
 	}
 	if providerSpec.MemoryMiB < minVSphereMemoryMiB {
-		warnings = append(warnings, fmt.Sprintf("providerSpec.memoryMiB: %d is less than the recommended minimum value (%d): nodes may not boot correctly", providerSpec.MemoryMiB, minVSphereMemoryMiB))
+		warnings = append(warnings, fmt.Sprintf("providerSpec.memoryMiB: %d is missing or less than the recommended minimum value (%d): nodes may not boot correctly", providerSpec.MemoryMiB, minVSphereMemoryMiB))
 	}
 	if providerSpec.DiskGiB < minVSphereDiskGiB {
-		warnings = append(warnings, fmt.Sprintf("providerSpec.diskGiB: %d is less than the recommended minimum (%d): nodes may fail to start if disk size is too low", providerSpec.DiskGiB, minVSphereDiskGiB))
+		warnings = append(warnings, fmt.Sprintf("providerSpec.diskGiB: %d is missing or less than the recommended minimum (%d): nodes may fail to start if disk size is too low", providerSpec.DiskGiB, minVSphereDiskGiB))
 	}
 
 	if providerSpec.UserDataSecret == nil {

pkg/apis/machine/v1beta1/machine_webhook_test.go

@@ -1919,7 +1919,7 @@ func TestValidateVSphereProviderSpec(t *testing.T) {
 			},
 			expectedOk:       true,
 			expectedError:    "",
-			expectedWarnings: []string{"providerSpec.numCPUs: 1 is less than the minimum value (2): the minimum value will be used instead"},
+			expectedWarnings: []string{"providerSpec.numCPUs: 1 is missing or less than the minimum value (2): nodes may not boot correctly"},
 		},
 		{
 			testCase: "with too little memory provided",
@@ -1928,7 +1928,7 @@ func TestValidateVSphereProviderSpec(t *testing.T) {
 			},
 			expectedOk:       true,
 			expectedError:    "",
-			expectedWarnings: []string{"providerSpec.memoryMiB: 1024 is less than the recommended minimum value (2048): nodes may not boot correctly"},
+			expectedWarnings: []string{"providerSpec.memoryMiB: 1024 is missing or less than the recommended minimum value (2048): nodes may not boot correctly"},
 		},
 		{
 			testCase: "with too little disk size provided",
@@ -1937,7 +1937,7 @@ func TestValidateVSphereProviderSpec(t *testing.T) {
 			},
 			expectedOk:       true,
 			expectedError:    "",
-			expectedWarnings: []string{"providerSpec.diskGiB: 1 is less than the recommended minimum (120): nodes may fail to start if disk size is too low"},
+			expectedWarnings: []string{"providerSpec.diskGiB: 1 is missing or less than the recommended minimum (120): nodes may fail to start if disk size is too low"},
 		},
 		{
 			testCase: "with no user data secret provided",
@@ -1976,6 +1976,30 @@ func TestValidateVSphereProviderSpec(t *testing.T) {
 			expectedOk:    true,
 			expectedError: "",
 		},
+		{
+			testCase: "with numCPUs equal to 0",
+			modifySpec: func(p *vsphere.VSphereMachineProviderSpec) {
+				p.NumCPUs = 0
+			},
+			expectedOk:       true,
+			expectedWarnings: []string{"providerSpec.numCPUs: 0 is missing or less than the minimum value (2): nodes may not boot correctly"},
+		},
+		{
+			testCase: "with memoryMiB equal to 0",
+			modifySpec: func(p *vsphere.VSphereMachineProviderSpec) {
+				p.MemoryMiB = 0
+			},
+			expectedOk:       true,
+			expectedWarnings: []string{"providerSpec.memoryMiB: 0 is missing or less than the recommended minimum value (2048): nodes may not boot correctly"},
+		},
+		{
+			testCase: "with diskGiB equal to 0",
+			modifySpec: func(p *vsphere.VSphereMachineProviderSpec) {
+				p.DiskGiB = 0
+			},
+			expectedOk:       true,
+			expectedWarnings: []string{"providerSpec.diskGiB: 0 is missing or less than the recommended minimum (120): nodes may fail to start if disk size is too low"},
+		},
 	}
 
 	infra := plainInfra.DeepCopy()

pkg/controller/vsphere/reconciler.go

@@ -27,8 +27,6 @@ import (
 )
 
 const (
-	minMemMB              = 2048
-	minCPU                = 2
 	fullCloneDiskMoveType = string(types.VirtualMachineRelocateDiskMoveOptionsMoveAllDiskBackingsAndConsolidate)
 	linkCloneDiskMoveType = string(types.VirtualMachineRelocateDiskMoveOptionsCreateNewChildDiskBacking)
 	ethCardType           = "vmxnet3"
@@ -548,17 +546,11 @@ func clone(s *machineScope) (string, error) {
 	}
 
 	numCPUs := s.providerSpec.NumCPUs
-	if numCPUs < minCPU {
-		numCPUs = minCPU
-	}
+
 	numCoresPerSocket := s.providerSpec.NumCoresPerSocket
 	if numCoresPerSocket == 0 {
 		numCoresPerSocket = numCPUs
 	}
-	memMiB := s.providerSpec.MemoryMiB
-	if memMiB == 0 {
-		memMiB = minMemMB
-	}
 
 	devices, err := vmTemplate.Device(s.Context)
 	if err != nil {
@@ -605,7 +597,7 @@ func clone(s *machineScope) (string, error) {
 			DeviceChange:      deviceSpecs,
 			NumCPUs:           numCPUs,
 			NumCoresPerSocket: numCoresPerSocket,
-			MemoryMB:          memMiB,
+			MemoryMB:          s.providerSpec.MemoryMiB,
 		},
 		Location: types.VirtualMachineRelocateSpec{
 			Datastore:    types.NewReference(datastore.Reference()),