diff --git a/pkg/controller/vsphere/actuator_test.go b/pkg/controller/vsphere/actuator_test.go index e78965fc04..d5fdc614dd 100644 --- a/pkg/controller/vsphere/actuator_test.go +++ b/pkg/controller/vsphere/actuator_test.go @@ -150,6 +150,22 @@ func TestMachineEvents(t *testing.T) { g.Expect(k8sClient.Delete(context.Background(), infra)).To(Succeed()) }() + userDataSecretName := "vsphere-ignition" + userDataSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: userDataSecretName, + Namespace: testNamespaceName, + }, + Data: map[string][]byte{ + userDataSecretKey: []byte("{}"), + }, + } + + g.Expect(k8sClient.Create(context.Background(), userDataSecret)).To(Succeed()) + defer func() { + g.Expect(k8sClient.Delete(context.Background(), userDataSecret)).To(Succeed()) + }() + createTagAndCategory(session, "CLUSTERID", "CLUSTERID") ctx := context.Background() @@ -241,6 +257,9 @@ func TestMachineEvents(t *testing.T) { CredentialsSecret: &corev1.LocalObjectReference{ Name: "test", }, + UserDataSecret: &corev1.LocalObjectReference{ + Name: userDataSecretName, + }, }) gs.Expect(err).ToNot(HaveOccurred()) diff --git a/pkg/controller/vsphere/machine_scope.go b/pkg/controller/vsphere/machine_scope.go index 2fb4ef3916..112de9f1ca 100644 --- a/pkg/controller/vsphere/machine_scope.go +++ b/pkg/controller/vsphere/machine_scope.go @@ -8,6 +8,7 @@ import ( machinev1 "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1" apivsphere "github.com/openshift/machine-api-operator/pkg/apis/vsphereprovider/v1beta1" machineapierros "github.com/openshift/machine-api-operator/pkg/controller/machine" + machinecontroller "github.com/openshift/machine-api-operator/pkg/controller/machine" "github.com/openshift/machine-api-operator/pkg/controller/vsphere/session" apicorev1 "k8s.io/api/core/v1" apimachineryerrors "k8s.io/apimachinery/pkg/api/errors" @@ -133,7 +134,7 @@ func (s *machineScope) GetSession() *session.Session { // provider spec, if one is set. func (s *machineScope) GetUserData() ([]byte, error) { if s.providerSpec == nil || s.providerSpec.UserDataSecret == nil { - return nil, nil + return nil, machinecontroller.InvalidMachineConfiguration("user data secret is missing in provider spec") } userDataSecret := &apicorev1.Secret{} diff --git a/pkg/controller/vsphere/machine_scope_test.go b/pkg/controller/vsphere/machine_scope_test.go index bf3ccd2dc8..006f7f4484 100644 --- a/pkg/controller/vsphere/machine_scope_test.go +++ b/pkg/controller/vsphere/machine_scope_test.go @@ -99,14 +99,14 @@ func TestGetUserData(t *testing.T) { testCase: "no provider spec", userDataSecret: nil, providerSpec: nil, - expectError: false, + expectError: true, expectedUserdata: nil, }, { testCase: "no user-data in provider spec", userDataSecret: nil, providerSpec: &vspherev1.VSphereMachineProviderSpec{}, - expectError: false, + expectError: true, expectedUserdata: nil, }, } diff --git a/pkg/controller/vsphere/reconciler.go b/pkg/controller/vsphere/reconciler.go index 5a388b0450..c8b62990bc 100644 --- a/pkg/controller/vsphere/reconciler.go +++ b/pkg/controller/vsphere/reconciler.go @@ -454,7 +454,9 @@ func clone(s *machineScope) (string, error) { vmTemplate, err := s.GetSession().FindVM(*s, "", s.providerSpec.Template) if err != nil { - return "", err + const notFoundMsg = "template not found, specify valid value" + defaultError := fmt.Errorf("unable to get template %q: %w", s.providerSpec.Template, err) + return "", handleVSphereError("", notFoundMsg, defaultError, err) } var snapshotRef *types.ManagedObjectReference @@ -500,24 +502,24 @@ func clone(s *machineScope) (string, error) { folder, err := s.GetSession().Finder.FolderOrDefault(s, folderPath) if err != nil { - multipleFoundMsg := "multiple folders found, specify one in config" - notFoundMsg := "folder not found, specify valid value" + const multipleFoundMsg = "multiple folders found, specify one in config" + const notFoundMsg = "folder not found, specify valid value" defaultError := fmt.Errorf("unable to get folder for %q: %w", folderPath, err) return "", handleVSphereError(multipleFoundMsg, notFoundMsg, defaultError, err) } datastore, err := s.GetSession().Finder.DatastoreOrDefault(s, datastorePath) if err != nil { - multipleFoundMsg := "multiple datastores found, specify one in config" - notFoundMsg := "datastore not found, specify valid value" + const multipleFoundMsg = "multiple datastores found, specify one in config" + const notFoundMsg = "datastore not found, specify valid value" defaultError := fmt.Errorf("unable to get datastore for %q: %w", datastorePath, err) return "", handleVSphereError(multipleFoundMsg, notFoundMsg, defaultError, err) } resourcepool, err := s.GetSession().Finder.ResourcePoolOrDefault(s, resourcepoolPath) if err != nil { - multipleFoundMsg := "multiple resource pools found, specify one in config" - notFoundMsg := "resource pool not found, specify valid value" + const multipleFoundMsg = "multiple resource pools found, specify one in config" + const notFoundMsg = "resource pool not found, specify valid value" defaultError := fmt.Errorf("unable to get resource pool for %q: %w", resourcepool, err) return "", handleVSphereError(multipleFoundMsg, notFoundMsg, defaultError, err) } @@ -639,7 +641,10 @@ func getNetworkDevices(s *machineScope, devices object.VirtualDeviceList) ([]typ ref, err := s.GetSession().Finder.Network(s.Context, netSpec.NetworkName) if err != nil { - return nil, fmt.Errorf("unable to find network %q: %w", netSpec.NetworkName, err) + const multipleFoundMsg = "multiple networks found, specify one in config" + const notFoundMsg = "network not found, specify valid value" + defaultError := fmt.Errorf("unable to get network for %q: %w", netSpec.NetworkName, err) + return nil, handleVSphereError(multipleFoundMsg, notFoundMsg, defaultError, err) } backing, err := ref.EthernetCardBackingInfo(s.Context) diff --git a/pkg/controller/vsphere/reconciler_test.go b/pkg/controller/vsphere/reconciler_test.go index e3cfb69c98..0ad4603ec7 100644 --- a/pkg/controller/vsphere/reconciler_test.go +++ b/pkg/controller/vsphere/reconciler_test.go @@ -95,6 +95,7 @@ func TestClone(t *testing.T) { password, _ := server.URL.User.Password() namespace := "test" vm := simulator.Map.Any("VirtualMachine").(*simulator.VirtualMachine) + credentialsSecret := corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: "test", @@ -106,6 +107,17 @@ func TestClone(t *testing.T) { }, } + userDataSecretName := "vsphere-ignition" + userDataSecret := corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: userDataSecretName, + Namespace: namespace, + }, + Data: map[string][]byte{ + userDataSecretKey: []byte("{}"), + }, + } + testCases := []struct { testCase string cloneVM bool @@ -123,6 +135,10 @@ func TestClone(t *testing.T) { Workspace: &vsphereapi.Workspace{ Server: server.URL.Host, }, + Template: vm.Name, + UserDataSecret: &corev1.LocalObjectReference{ + Name: userDataSecretName, + }, }, cloneVM: true, machineName: "test0", @@ -137,6 +153,10 @@ func TestClone(t *testing.T) { Server: server.URL.Host, Folder: "custom-folder", }, + Template: vm.Name, + UserDataSecret: &corev1.LocalObjectReference{ + Name: userDataSecretName, + }, }, cloneVM: true, machineName: "test1", @@ -151,6 +171,10 @@ func TestClone(t *testing.T) { Server: server.URL.Host, ResourcePool: "invalid", }, + Template: vm.Name, + UserDataSecret: &corev1.LocalObjectReference{ + Name: userDataSecretName, + }, }, expectedError: errors.New("resource pool not found, specify valid value"), }, @@ -164,6 +188,10 @@ func TestClone(t *testing.T) { Server: server.URL.Host, ResourcePool: "/DC0/host/DC0_C0/Resources/...", }, + Template: vm.Name, + UserDataSecret: &corev1.LocalObjectReference{ + Name: userDataSecretName, + }, }, expectedError: errors.New("multiple resource pools found, specify one in config"), setupFailureCondition: func() error { @@ -195,6 +223,10 @@ func TestClone(t *testing.T) { Server: server.URL.Host, Folder: "invalid", }, + Template: vm.Name, + UserDataSecret: &corev1.LocalObjectReference{ + Name: userDataSecretName, + }, }, }, { @@ -207,6 +239,10 @@ func TestClone(t *testing.T) { Server: server.URL.Host, Folder: "/DC0/vm/...", }, + Template: vm.Name, + UserDataSecret: &corev1.LocalObjectReference{ + Name: userDataSecretName, + }, }, expectedError: errors.New("multiple folders found, specify one in config"), setupFailureCondition: func() error { @@ -236,6 +272,10 @@ func TestClone(t *testing.T) { Server: server.URL.Host, Datastore: "invalid", }, + Template: vm.Name, + UserDataSecret: &corev1.LocalObjectReference{ + Name: userDataSecretName, + }, }, expectedError: errors.New("datastore not found, specify valid value"), }, @@ -249,6 +289,10 @@ func TestClone(t *testing.T) { Server: server.URL.Host, Datastore: "/DC0/...", }, + Template: vm.Name, + UserDataSecret: &corev1.LocalObjectReference{ + Name: userDataSecretName, + }, }, expectedError: errors.New("multiple datastores found, specify one in config"), setupFailureCondition: func() error { @@ -276,6 +320,16 @@ func TestClone(t *testing.T) { return nil }, }, + { + testCase: "fail on invalid template", + providerSpec: vsphereapi.VSphereMachineProviderSpec{ + Template: "invalid", + UserDataSecret: &corev1.LocalObjectReference{ + Name: userDataSecretName, + }, + }, + expectedError: errors.New("template not found, specify valid value"), + }, } for _, tc := range testCases { @@ -300,11 +354,9 @@ func TestClone(t *testing.T) { providerSpec: &tc.providerSpec, session: session, providerStatus: &vsphereapi.VSphereMachineProviderStatus{}, - client: fake.NewFakeClientWithScheme(scheme.Scheme, &credentialsSecret), + client: fake.NewFakeClientWithScheme(scheme.Scheme, &credentialsSecret, &userDataSecret), } - machineScope.providerSpec.Template = vm.Name - if tc.machineName != "" { machineScope.machine.Name = tc.machineName } @@ -1247,6 +1299,17 @@ func TestCreate(t *testing.T) { }, } + userDataSecretName := "vsphere-ignition" + userDataSecret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: userDataSecretName, + Namespace: namespace, + }, + Data: map[string][]byte{ + userDataSecretKey: []byte("{}"), + }, + } + cases := []struct { name string expectedError error @@ -1264,6 +1327,9 @@ func TestCreate(t *testing.T) { CredentialsSecret: &corev1.LocalObjectReference{ Name: "test", }, + UserDataSecret: &corev1.LocalObjectReference{ + Name: userDataSecretName, + }, }, }, { @@ -1315,7 +1381,8 @@ func TestCreate(t *testing.T) { client := fake.NewFakeClientWithScheme(scheme.Scheme, credentialsSecret, configMap, - infra) + infra, + userDataSecret) rawProviderSpec, err := vsphereapi.RawExtensionFromProviderSpec(&tc.providerSpec) if err != nil {