diff --git a/manifests/machineconfigcontroller/install-config-role.yaml b/manifests/machineconfigcontroller/install-config-role.yaml new file mode 100644 index 0000000000..21c7eab702 --- /dev/null +++ b/manifests/machineconfigcontroller/install-config-role.yaml @@ -0,0 +1,10 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: machine-config-controller-install-config + namespace: kube-system +rules: +- apiGroups: [""] + resources: ["configmaps"] + resourceNames: ["cluster-config-v1"] + verbs: ["get"] diff --git a/manifests/machineconfigcontroller/install-config-rolebinding.yaml b/manifests/machineconfigcontroller/install-config-rolebinding.yaml new file mode 100644 index 0000000000..e821ef96e1 --- /dev/null +++ b/manifests/machineconfigcontroller/install-config-rolebinding.yaml @@ -0,0 +1,13 @@ +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: machine-config-controller-install-config + namespace: kube-system +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: machine-config-controller-install-config +subjects: +- kind: ServiceAccount + namespace: {{.TargetNamespace}} + name: machine-config-controller diff --git a/pkg/controller/bootimage/platform_helpers.go b/pkg/controller/bootimage/platform_helpers.go index 6581ab262d..2c5fc22822 100644 --- a/pkg/controller/bootimage/platform_helpers.go +++ b/pkg/controller/bootimage/platform_helpers.go @@ -208,7 +208,7 @@ func reconcileAWSProviderSpec(streamData *stream.Stream, arch string, _ *osconfi return true, false, newProviderSpec, nil } -func reconcileVSphereProviderSpec(streamData *stream.Stream, arch string, infra *osconfigv1.Infrastructure, providerSpec *machinev1beta1.VSphereMachineProviderSpec, _ string, secretClient clientset.Interface) (bool, bool, *machinev1beta1.VSphereMachineProviderSpec, error) { +func reconcileVSphereProviderSpec(streamData *stream.Stream, arch string, infra *osconfigv1.Infrastructure, providerSpec *machinev1beta1.VSphereMachineProviderSpec, _ string, kubeClient clientset.Interface) (bool, bool, *machinev1beta1.VSphereMachineProviderSpec, error) { if infra.Spec.PlatformSpec.VSphere == nil { klog.Warningf("Reconcile skipped: VSphere field is nil in PlatformSpec %v", infra.Spec.PlatformSpec) @@ -228,12 +228,12 @@ func reconcileVSphereProviderSpec(streamData *stream.Stream, arch string, infra newProviderSpec := providerSpec.DeepCopy() // Fetch the creds configmap - credsSc, err := secretClient.CoreV1().Secrets("kube-system").Get(context.TODO(), "vsphere-creds", metav1.GetOptions{}) + credsSc, err := kubeClient.CoreV1().Secrets("kube-system").Get(context.TODO(), "vsphere-creds", metav1.GetOptions{}) if err != nil { return false, false, nil, fmt.Errorf("failed to fetch vsphere-creds Secret during machineset sync: %w", err) } - newBootImg, patchRequired, err := createNewVMTemplate(streamData, providerSpec, infra, credsSc, arch, artifacts.Release) + newBootImg, patchRequired, err := createNewVMTemplate(streamData, providerSpec, infra, credsSc, kubeClient, arch, artifacts.Release) if err != nil { return false, false, nil, err } @@ -241,7 +241,7 @@ func reconcileVSphereProviderSpec(streamData *stream.Stream, arch string, infra // If patch is required, marshal the new providerspec into the machineset if patchRequired { // Ensure the ignition stub is the minimum acceptable spec required for boot image updates - if err := upgradeStubIgnitionIfRequired(providerSpec.UserDataSecret.Name, secretClient); err != nil { + if err := upgradeStubIgnitionIfRequired(providerSpec.UserDataSecret.Name, kubeClient); err != nil { return false, false, nil, err } newProviderSpec.Template = newBootImg diff --git a/pkg/controller/bootimage/vsphere_helpers.go b/pkg/controller/bootimage/vsphere_helpers.go index 62f24d7736..230c20f78a 100644 --- a/pkg/controller/bootimage/vsphere_helpers.go +++ b/pkg/controller/bootimage/vsphere_helpers.go @@ -26,8 +26,11 @@ import ( osconfigv1 "github.com/openshift/api/config/v1" machinev1beta1 "github.com/openshift/api/machine/v1beta1" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + clientset "k8s.io/client-go/kubernetes" "k8s.io/klog/v2" "k8s.io/utils/ptr" + "sigs.k8s.io/yaml" "github.com/openshift/machine-config-operator/pkg/controller/bootimage/cache" ) @@ -202,10 +205,31 @@ func findAllRequiredResources(ctx context.Context, finder *find.Finder, provider } else { return nil, fmt.Errorf("finder had error: %w", err) } + } else if !isInFolder(vr.existingVM, vr.folder) { + // Name match outside the workspace folder: a customer-managed VM, not the one MCO manages. + // Leave it untouched so the caller creates a fresh template instead of swapping it out. + klog.Infof("VM %s exists outside the expected workspace folder %s; leaving the customer-managed VM untouched", name, providerSpec.Workspace.Folder) + vr.existingVM = nil } return &vr, nil } +// isInFolder reports whether vm is a direct child of folder. govmomi's Finder.VirtualMachine does a +// global, recursive, name-based search of the entire vCenter inventory with no folder scoping, so a +// name match alone doesn't mean the VM is the one MCO manages. MCO always creates/imports templates +// directly into providerSpec.Workspace.Folder, so anything else found by name is customer-managed and +// must be left untouched. +// +// A nil folder (the workspace folder itself couldn't be resolved) trusts the match instead of +// rejecting it: verifying locality isn't a prerequisite for this function's caller, and anything that +// actually needs to write to vSphere re-resolves the same folder and hard-fails there if it's broken. +func isInFolder(vm *object.VirtualMachine, folder *object.Folder) bool { + if folder == nil { + return true + } + return path.Dir(vm.InventoryPath) == folder.InventoryPath +} + // getDiskTypeFromExistingVM inspects the given VM's disk backing configuration and returns its disk provisioning type (thin, thick, eagerZeroedThick). func getDiskTypeFromExistingVM(vmMo mo.VirtualMachine) string { diskType := "" @@ -229,6 +253,153 @@ func getDiskTypeFromExistingVM(vmMo mo.VirtualMachine) string { return diskType } +const ( + installConfigConfigMapName = "cluster-config-v1" + installConfigConfigMapKey = "install-config" +) + +// installConfig mirrors the subset of the install-config schema needed to recover the vSphere disk +// provisioning type. This is only ever set at install time and is not persisted anywhere else the +// operator can read it (VSphereMachineProviderSpec, Infrastructure, failure domain topology all omit it). +type installConfig struct { + Platform struct { + VSphere struct { + DiskType string `json:"diskType"` + } `json:"vsphere"` + } `json:"platform"` +} + +// getDiskTypeFromInstallConfig reads platform.vsphere.diskType from the cluster's install-config +// configmap. Used for brand-new failure domains, where there is no existing template VM to inspect +// via getDiskTypeFromExistingVM. Returns "" (no error) if the installer left diskType unset, which is +// a legitimate value: getCISP treats "" as "use the datastore's default storage policy." +func getDiskTypeFromInstallConfig(ctx context.Context, kubeClient clientset.Interface) (string, error) { + cm, err := kubeClient.CoreV1().ConfigMaps("kube-system").Get(ctx, installConfigConfigMapName, metav1.GetOptions{}) + if err != nil { + return "", fmt.Errorf("failed to fetch %s configmap: %w", installConfigConfigMapName, err) + } + + raw, ok := cm.Data[installConfigConfigMapKey] + if !ok { + return "", fmt.Errorf("%s configmap has no %q key", installConfigConfigMapName, installConfigConfigMapKey) + } + + var ic installConfig + if err := yaml.Unmarshal([]byte(raw), &ic); err != nil { + return "", fmt.Errorf("failed to parse install-config: %w", err) + } + + return ic.Platform.VSphere.DiskType, nil +} + +// resolveExistingTemplateVM locates the template VM for a failure domain using a two-step lookup: providerSpec.Template first +// then the infra computed name. If neither exists in the workspace folder, it creates a fresh template from the OVA and +// signals the caller via created=true. resolvedName is the vSphere name of the VM that was actually found, which may differ +// from name when the VM was located via providerSpec.Template; callers must use resolvedName (not name) for divergence checks. +// +// A name match outside providerSpec.Workspace.Folder is never treated as the existing template: govmomi's finder searches +// the entire vCenter inventory by name, so a match elsewhere may be a customer-managed VM MCO must not touch (see isInFolder). +// If the workspace folder itself can't be resolved, this check is skipped (logging a warning) rather than failing the +// reconcile: verifying it isn't this function's job — it's a locality check for classification, not a prerequisite for +// action. +func resolveExistingTemplateVM( + ctx context.Context, + finder *find.Finder, + providerSpec *machinev1beta1.VSphereMachineProviderSpec, + failureDomain osconfigv1.VSpherePlatformFailureDomainSpec, + streamData *stream.Stream, + client *govmomi.Client, + tagManager *tags.Manager, + kubeClient clientset.Interface, + infraID, arch string, +) (vm *object.VirtualMachine, resolvedName string, created bool, err error) { + + computedName := fmt.Sprintf("%s-rhcos-%s", infraID, failureDomain.Name) + var notFoundErr *find.NotFoundError + + workspaceFolder, folderErr := finder.Folder(ctx, providerSpec.Workspace.Folder) + if folderErr != nil { + klog.Warningf("failed to resolve workspace folder %q; cannot verify template VM locality this reconcile, proceeding with name-based lookup only: %v", providerSpec.Workspace.Folder, folderErr) + } + + // Check providerSpec.Template first so a freshly-added failure domain whose MachineSet + // already has a valid template doesn't fail just because the infra computed name isn't a match. + if providerSpec.Template != "" && providerSpec.Template != computedName { + tmplVM, tmplErr := finder.VirtualMachine(ctx, providerSpec.Template) + switch { + case tmplErr == nil && isInFolder(tmplVM, workspaceFolder): + return tmplVM, providerSpec.Template, false, nil + case tmplErr == nil: + klog.Infof("providerSpec.Template %s exists outside the expected workspace folder %s; leaving the customer-managed VM untouched and falling back to computed name %s", providerSpec.Template, providerSpec.Workspace.Folder, computedName) + case errors.As(tmplErr, ¬FoundErr): + klog.Infof("providerSpec.Template %s not found in vSphere; falling back to computed name %s", providerSpec.Template, computedName) + default: + klog.Warningf("Unexpected error looking up providerSpec.Template %s: %v; falling back to computed name %s", providerSpec.Template, tmplErr, computedName) + } + } + + vm, err = finder.VirtualMachine(ctx, computedName) + switch { + case err == nil && isInFolder(vm, workspaceFolder): + return vm, computedName, false, nil + case err == nil: + klog.Infof("VM %s exists outside the expected workspace folder %s; leaving the customer-managed VM untouched and creating a fresh template", computedName, providerSpec.Workspace.Folder) + case !errors.As(err, ¬FoundErr): + return nil, "", false, fmt.Errorf("finder had error: %w", err) + } + + // Computed name not found in the workspace folder — either nothing exists there, or the name is + // held by a customer-managed VM elsewhere in the inventory that must be left alone. Either way, + // check for a rollback VM left by a prior mid-swap crash before creating a fresh template. + oldTempName := atomicTempName("mco-old", computedName) + oldVM, oldErr := finder.VirtualMachine(ctx, oldTempName) + if oldErr != nil { + if !errors.As(oldErr, ¬FoundErr) { + return nil, "", false, fmt.Errorf("error looking up rollback VM %s: %w", oldTempName, oldErr) + } + // No template exists anywhere — failure domain is newly added; create from OVA. + klog.Infof("No existing template found for failure domain %s; creating new template %s from OVA", failureDomain.Name, computedName) + if len(computedName) > 80 { + return nil, "", false, fmt.Errorf("length of VM template name `%s` exceeds the permitted limit of 80 characters", computedName) + } + ova, ovaErr := streamData.QueryDisk(arch, "vmware", "ova") + if ovaErr != nil { + return nil, "", false, ovaErr + } + ovaPath, ovaErr := cache.DownloadOva(ova) + if ovaErr != nil { + return nil, "", false, fmt.Errorf("failed to download %s: %w", ova.Location, ovaErr) + } + // No existing VM to inspect for this brand-new failure domain; the only other place + // disk provisioning type is recorded is the install-time install-config. + diskType, diskTypeErr := getDiskTypeFromInstallConfig(ctx, kubeClient) + if diskTypeErr != nil { + return nil, "", false, fmt.Errorf("failed to determine disk provisioning type for new template %s: %w", computedName, diskTypeErr) + } + if createErr := createNewVMTemplateWithNameForFailureDomain(ctx, providerSpec, failureDomain, finder, client, tagManager, computedName, ovaPath, infraID, diskType); createErr != nil { + return nil, "", false, createErr + } + return nil, computedName, true, nil + } + + // Rollback: restore the old template renamed away during a crashed atomic swap. + klog.Infof("Recovering from mid-swap crash: renaming %s back to %s", oldTempName, computedName) + renameTask, renameErr := oldVM.Rename(ctx, computedName) + if renameErr != nil { + return nil, "", false, fmt.Errorf("failed to initiate rollback rename of %s to %s: %w", oldTempName, computedName, renameErr) + } + if renameErr = renameTask.Wait(ctx); renameErr != nil { + return nil, "", false, fmt.Errorf("failed to complete rollback rename of %s to %s: %w", oldTempName, computedName, renameErr) + } + // Confirm the rollback succeeded; any stale mco-tmp-* VM will be cleaned up by + // createNewVMTemplateWithNameForFailureDomain if an update is subsequently needed. + vm, err = finder.VirtualMachine(ctx, computedName) + if err != nil { + return nil, "", false, fmt.Errorf("failed to fetch rolled-back template %s: %w", computedName, err) + } + return vm, computedName, false, nil +} + // atomicTempName returns a short, fixed-length (17-char) VM name for use during atomic template // swaps. The hash makes it deterministic and unique per final name, bypassing the 80-char limit. func atomicTempName(prefix, name string) string { @@ -499,11 +670,10 @@ func getClientsFromServerURL(ctx context.Context, server, username, password str } // Creates a Template VM which has the relevant OVA/OVF file -func createNewVMTemplate(streamData *stream.Stream, providerSpec *machinev1beta1.VSphereMachineProviderSpec, infra *osconfigv1.Infrastructure, credsSc *corev1.Secret, arch, release string) (string, bool, error) { +func createNewVMTemplate(streamData *stream.Stream, providerSpec *machinev1beta1.VSphereMachineProviderSpec, infra *osconfigv1.Infrastructure, credsSc *corev1.Secret, kubeClient clientset.Interface, arch, release string) (string, bool, error) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - var name string for _, vcenter := range infra.Spec.PlatformSpec.VSphere.VCenters { if vcenter.Server != providerSpec.Workspace.Server { continue @@ -546,36 +716,12 @@ func createNewVMTemplate(streamData *stream.Stream, providerSpec *machinev1beta1 } finder = finder.SetDatacenter(datacenter) - name = fmt.Sprintf("%s-rhcos-%s", infraID, failureDomain.Name) - - existingTemplateVM, err := finder.VirtualMachine(ctx, name) + existingTemplateVM, resolvedName, created, err := resolveExistingTemplateVM(ctx, finder, providerSpec, failureDomain, streamData, client, tagManager, kubeClient, infraID, arch) if err != nil { - if _, ok := err.(*find.NotFoundError); !ok { - return "", false, fmt.Errorf("finder had error: %w", err) - } - // Template not found; update likely crashed between the two rename steps of a prior - // atomic swap. Check for the old template under its rollback name and restore it - // so the next reconciliation loop can proceed normally. - oldTempName := atomicTempName("mco-old", name) - oldVM, oldErr := finder.VirtualMachine(ctx, oldTempName) - if oldErr != nil { - return "", false, fmt.Errorf("template %s not found : %w and no rollback VM %s present: %w", name, err, oldTempName, oldErr) - } - klog.Infof("Recovering from mid-swap crash: renaming %s back to %s", oldTempName, name) - renameTask, renameErr := oldVM.Rename(ctx, name) - if renameErr != nil { - return "", false, fmt.Errorf("failed to initiate rollback rename of %s to %s: %w", oldTempName, name, renameErr) - } - if renameErr = renameTask.Wait(ctx); renameErr != nil { - return "", false, fmt.Errorf("failed to complete rollback rename of %s to %s: %w", oldTempName, name, renameErr) - } - // Fresh fetch by name to confirm the rollback succeeded and get a clean reference. - // Any stale mco-tmp-* VM will be cleaned up by createNewVMTemplateWithNameForFailureDomain - // if an update is subsequently needed. - existingTemplateVM, err = finder.VirtualMachine(ctx, name) - if err != nil { - return "", false, fmt.Errorf("failed to fetch rolled-back template %s: %w", name, err) - } + return "", false, err + } + if created { + return resolvedName, true, nil } var vmMo mo.VirtualMachine @@ -584,50 +730,58 @@ func createNewVMTemplate(streamData *stream.Stream, providerSpec *machinev1beta1 return "", false, fmt.Errorf("Unable to extract properties from existing Template VM: %w", err) } - if vmMo.Summary.Config.Product != nil { - templateProductVersion := vmMo.Summary.Config.Product.Version - if templateProductVersion == "" { - return "", false, fmt.Errorf("unable to determine RHCOS version of virtual machine: %s", providerSpec.Template) - } - - if templateProductVersion != release { - klog.Infof("Existing RHCOS v%s does not match current RHCOS v%s. Starting reconciliation process.", templateProductVersion, release) + if vmMo.Summary.Config.Product == nil { + return "", false, fmt.Errorf("unable to determine RHCOS version of virtual machine: %s", providerSpec.Template) + } - // Find and download the relevant OVA file - ova, err := streamData.QueryDisk(arch, "vmware", "ova") - if err != nil { - return "", false, err - } + templateProductVersion := vmMo.Summary.Config.Product.Version + if templateProductVersion == "" { + return "", false, fmt.Errorf("unable to determine RHCOS version of virtual machine: %s", providerSpec.Template) + } - ovaPath, err := cache.DownloadOva(ova) - if err != nil { - return "", false, fmt.Errorf("Failed to download %s: %w", ova.Location, err) - } + if templateProductVersion != release { + klog.Infof("Existing RHCOS v%s does not match current RHCOS v%s. Starting reconciliation process.", templateProductVersion, release) - if len(name) > 80 { - return "", false, fmt.Errorf("Length of VM template name `%s` exceeds the permitted limit of 80 characters", name) - } + // Find and download the relevant OVA file + ova, err := streamData.QueryDisk(arch, "vmware", "ova") + if err != nil { + return "", false, err + } - diskType := getDiskTypeFromExistingVM(vmMo) + ovaPath, err := cache.DownloadOva(ova) + if err != nil { + return "", false, fmt.Errorf("failed to download %s: %w", ova.Location, err) + } - err = createNewVMTemplateWithNameForFailureDomain(ctx, providerSpec, failureDomain, finder, client, tagManager, name, ovaPath, infraID, diskType) - if err != nil { - return "", false, err - } - return name, true, nil + if len(resolvedName) > 80 { + return "", false, fmt.Errorf("length of VM template name `%s` exceeds the permitted limit of 80 characters", resolvedName) } - klog.Infof("Existing RHCOS v%s does match current RHCOS v%s. Skipping reconciliation process using govmomi.", templateProductVersion, release) - if providerSpec.Template != name { - klog.Infof("ProviderSpec template name: %s has diverged from the VM Template of name: %s that exists in VSphere. Reconciling the name change.", providerSpec.Template, name) - return name, true, nil + diskType := getDiskTypeFromExistingVM(vmMo) + + err = createNewVMTemplateWithNameForFailureDomain(ctx, providerSpec, failureDomain, finder, client, tagManager, resolvedName, ovaPath, infraID, diskType) + if err != nil { + return "", false, err } + return resolvedName, true, nil + } - } else { - return "", false, fmt.Errorf("unable to determine RHCOS version of virtual machine: %s", providerSpec.Template) + klog.Infof("Existing RHCOS v%s does match current RHCOS v%s. Skipping reconciliation process using govmomi.", templateProductVersion, release) + if providerSpec.Template != resolvedName { + klog.Infof("ProviderSpec template name: %s has diverged from the VM Template of name: %s that exists in VSphere. Reconciling the name change.", providerSpec.Template, resolvedName) + return resolvedName, true, nil } + return resolvedName, false, nil } } - return "", false, nil + workspaceDetails := fmt.Sprintf("server: %s, datacenter: %s, datastore: %s, resourcePool: %s", + providerSpec.Workspace.Server, providerSpec.Workspace.Datacenter, providerSpec.Workspace.Datastore, providerSpec.Workspace.ResourcePool) + // vmGroup only applies to HostGroup-zonal failure domains (VSphereFailureDomainZoneAffinity.Type + // == HostGroup; see the vmGroup derivation above) — omit it here when unset rather than implying + // it's a universal match criterion for every failure domain type (e.g. ComputeCluster-zoned ones). + if providerSpec.Workspace.VMGroup != "" { + workspaceDetails += fmt.Sprintf(", vmGroup: %s", providerSpec.Workspace.VMGroup) + } + return "", false, fmt.Errorf("providerSpec workspace (%s) does not match any vCenter/failure domain in the Infrastructure object", workspaceDetails) } diff --git a/pkg/controller/bootimage/vsphere_helpers_test.go b/pkg/controller/bootimage/vsphere_helpers_test.go new file mode 100644 index 0000000000..1f77db8bb7 --- /dev/null +++ b/pkg/controller/bootimage/vsphere_helpers_test.go @@ -0,0 +1,114 @@ +package bootimage + +import ( + "testing" + + "github.com/vmware/govmomi/object" + "github.com/vmware/govmomi/vim25/types" + + machinev1beta1 "github.com/openshift/api/machine/v1beta1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + osconfigv1 "github.com/openshift/api/config/v1" +) + +// TestCreateNewVMTemplate_NoMatchingFailureDomain verifies that when a MachineSet's +// providerSpec.Workspace doesn't match any vCenter/failure domain in the Infrastructure object, +// createNewVMTemplate returns a descriptive error instead of silently no-op'ing. This is the +// degrade-on-no-match behavior added in "bootimage: degrade when vsphere fd not found" — it never +// reaches getClientsFromServerURL (no real vCenter connectivity needed), since the outer loop over +// infra.Spec.PlatformSpec.VSphere.VCenters has nothing to match against. +func TestCreateNewVMTemplate_NoMatchingFailureDomain(t *testing.T) { + providerSpec := &machinev1beta1.VSphereMachineProviderSpec{ + Workspace: &machinev1beta1.Workspace{ + Server: "vcenter.example.com", + Datacenter: "dc1", + Datastore: "datastore1", + ResourcePool: "/dc1/host/cluster1/Resources", + }, + } + + infra := &osconfigv1.Infrastructure{ + Spec: osconfigv1.InfrastructureSpec{ + PlatformSpec: osconfigv1.PlatformSpec{ + VSphere: &osconfigv1.VSpherePlatformSpec{ + // Deliberately empty: no vCenters/failure domains for providerSpec.Workspace + // to match against. + }, + }, + }, + } + + resolvedName, patchRequired, err := createNewVMTemplate(nil, providerSpec, infra, nil, nil, "x86_64", "9.6.20260210-0") + + require.Error(t, err) + assert.Contains(t, err.Error(), "does not match any vCenter/failure domain") + assert.Contains(t, err.Error(), "vcenter.example.com") + assert.Empty(t, resolvedName) + assert.False(t, patchRequired) +} + +func newTestVM(inventoryPath string) *object.VirtualMachine { + vm := object.NewVirtualMachine(nil, types.ManagedObjectReference{}) + vm.InventoryPath = inventoryPath + return vm +} + +func newTestFolder(inventoryPath string) *object.Folder { + folder := object.NewFolder(nil, types.ManagedObjectReference{}) + folder.InventoryPath = inventoryPath + return folder +} + +// TestIsInFolder verifies the folder-scoping check used to distinguish MCO-managed template VMs +// from customer-managed VMs that merely share a name. govmomi's finder searches by name across the +// entire vCenter inventory, so a name match alone doesn't guarantee the VM lives where MCO expects +// (providerSpec.Workspace.Folder) — only a direct child of that folder counts as MCO-owned. +func TestIsInFolder(t *testing.T) { + workspaceFolder := newTestFolder("/dc1/vm/openshift4-folder") + + tests := []struct { + name string + vm *object.VirtualMachine + folder *object.Folder + want bool + }{ + { + name: "direct child of workspace folder", + vm: newTestVM("/dc1/vm/openshift4-folder/infra-rhcos-fd1"), + folder: workspaceFolder, + want: true, + }, + { + name: "sibling folder", + vm: newTestVM("/dc1/vm/customer-folder/infra-rhcos-fd1"), + folder: workspaceFolder, + want: false, + }, + { + name: "directly under the datacenter's default vm folder", + vm: newTestVM("/dc1/vm/infra-rhcos-fd1"), + folder: workspaceFolder, + want: false, + }, + { + name: "nested subfolder beneath the workspace folder", + vm: newTestVM("/dc1/vm/openshift4-folder/nested/infra-rhcos-fd1"), + folder: workspaceFolder, + want: false, + }, + { + name: "nil folder (workspace folder unresolved) trusts the match", + vm: newTestVM("/dc1/vm/customer-folder/infra-rhcos-fd1"), + folder: nil, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, isInFolder(tt.vm, tt.folder)) + }) + } +} diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index c3a85dd82e..49a94ff11f 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -96,6 +96,8 @@ const ( mccEventsRoleBindingTargetManifestPath = "manifests/machineconfigcontroller/events-rolebinding-target.yaml" mccConfigMapsRoleTargetManifestPath = "manifests/machineconfigcontroller/configmaps-role-target.yaml" mccConfigMapsRoleBindingTargetManifestPath = "manifests/machineconfigcontroller/configmaps-rolebinding-target.yaml" + mccInstallConfigRoleManifestPath = "manifests/machineconfigcontroller/install-config-role.yaml" + mccInstallConfigRoleBindingManifestPath = "manifests/machineconfigcontroller/install-config-rolebinding.yaml" mccClusterRoleBindingManifestPath = "manifests/machineconfigcontroller/clusterrolebinding.yaml" mccServiceAccountManifestPath = "manifests/machineconfigcontroller/sa.yaml" mccKubeRbacProxyConfigMapPath = "manifests/machineconfigcontroller/kube-rbac-proxy-config.yaml" @@ -1188,12 +1190,14 @@ func (optr *Operator) syncMachineConfigController(config *renderConfig, _ *confi roles: []string{ mccKubeRbacProxyPrometheusRolePath, mccConfigMapsRoleTargetManifestPath, + mccInstallConfigRoleManifestPath, }, roleBindings: []string{ mccEventsRoleBindingDefaultManifestPath, mccEventsRoleBindingTargetManifestPath, mccKubeRbacProxyPrometheusRoleBindingPath, mccConfigMapsRoleBindingTargetManifestPath, + mccInstallConfigRoleBindingManifestPath, mopRoleBindingManifestPath, }, clusterRoleBindings: []string{