From 95f3e60a823ea5174afe480e46166533657550f3 Mon Sep 17 00:00:00 2001 From: Talor Itzhak Date: Thu, 26 Sep 2024 12:47:52 +0300 Subject: [PATCH 01/10] vendor: bump deployer to v0.21.0 Signed-off-by: Talor Itzhak --- go.mod | 2 +- go.sum | 4 +- .../deployer/pkg/assets/selinux/consts.go | 3 +- .../pkg/deployer/platform/detect/detect.go | 2 +- .../deployer/pkg/manifests/manifests.go | 5 ++- .../deployer/pkg/manifests/rte/rte.go | 42 ++++++++++++------- .../deployer/pkg/objectupdate/rte/rte.go | 4 +- .../deployer/pkg/options/options.go | 14 ++++--- vendor/modules.txt | 2 +- 9 files changed, 47 insertions(+), 31 deletions(-) diff --git a/go.mod b/go.mod index 31201c73a..c4efc6838 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/go-logr/logr v1.4.2 github.com/google/go-cmp v0.6.0 github.com/jaypipes/ghw v0.12.0 - github.com/k8stopologyawareschedwg/deployer v0.20.4 + github.com/k8stopologyawareschedwg/deployer v0.21.0 github.com/k8stopologyawareschedwg/noderesourcetopology-api v0.1.2 github.com/k8stopologyawareschedwg/podfingerprint v0.2.2 github.com/k8stopologyawareschedwg/resource-topology-exporter v0.16.1 diff --git a/go.sum b/go.sum index e620a3e1a..46fd36a44 100644 --- a/go.sum +++ b/go.sum @@ -1177,8 +1177,8 @@ github.com/julienschmidt/httprouter v1.2.0/go.mod h1:SYymIcj16QtmaHHD7aYtjjsJG7V github.com/julienschmidt/httprouter v1.3.0/go.mod h1:JR6WtHb+2LUe8TCKY3cZOxFyyO8IZAc4RVcycCCAKdM= github.com/jung-kurt/gofpdf v1.0.0/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes= github.com/jung-kurt/gofpdf v1.0.3-0.20190309125859-24315acbbda5/go.mod h1:7Id9E/uU8ce6rXgefFLlgrJj/GYY22cpxn+r32jIOes= -github.com/k8stopologyawareschedwg/deployer v0.20.4 h1:aDpGoYHTEab8v+isLCqehgYy3ZafAattrAt6oLI6Jv0= -github.com/k8stopologyawareschedwg/deployer v0.20.4/go.mod h1:hnPU2dPLclkKXU28H+RkRrS21LFpmMTAU55mISsPuMk= +github.com/k8stopologyawareschedwg/deployer v0.21.0 h1:GA2HbYTIxynCc+R8ceGxnQo9AdQZBFUgNjHm3gJVRFQ= +github.com/k8stopologyawareschedwg/deployer v0.21.0/go.mod h1:hnPU2dPLclkKXU28H+RkRrS21LFpmMTAU55mISsPuMk= github.com/k8stopologyawareschedwg/noderesourcetopology-api v0.1.2 h1:uAwqOtyrFYggq3pVf3hs1XKkBxrQ8dkgjWz3LCLJsiY= github.com/k8stopologyawareschedwg/noderesourcetopology-api v0.1.2/go.mod h1:LBzS4n6GX1C69tzSd5EibZ9cGOXFuHP7GxEMDYVe1sM= github.com/k8stopologyawareschedwg/podfingerprint v0.2.2 h1:iFHPfZInM9pz2neye5RdmORMp1hPmte1EGJYpOOzZVg= diff --git a/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/assets/selinux/consts.go b/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/assets/selinux/consts.go index 8872d7c03..d47a89f6d 100644 --- a/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/assets/selinux/consts.go +++ b/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/assets/selinux/consts.go @@ -18,7 +18,8 @@ package selinux const ( RTEPolicyFileName = "/etc/selinux/rte.cil" - RTEContextType = "rte.process" + RTEContextType = "container_device_plugin_t" + RTEContextTypeLegacy = "rte.process" RTEContextLevel = "s0" RTEPolicyInstallServiceName = "rte-selinux-policy-install.service" ) diff --git a/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/deployer/platform/detect/detect.go b/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/deployer/platform/detect/detect.go index 12d9deb4a..bda33efb7 100644 --- a/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/deployer/platform/detect/detect.go +++ b/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/deployer/platform/detect/detect.go @@ -112,7 +112,7 @@ func PlatformFromClients(ctx context.Context, cvLister ClusterVersionsLister, in } func Version(ctx context.Context, plat platform.Platform) (platform.Version, error) { - if plat == platform.OpenShift { + if plat == platform.OpenShift || plat == platform.HyperShift { return OpenshiftVersion(ctx) } return KubernetesVersion(ctx) diff --git a/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/manifests/manifests.go b/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/manifests/manifests.go index 5d1d464b9..f871c4362 100644 --- a/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/manifests/manifests.go +++ b/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/manifests/manifests.go @@ -436,7 +436,7 @@ func getTemplateContent(templateContent []byte, templateArgs map[string]string) return fileContent.Bytes(), nil } -func SecurityContextConstraint(component string) (*securityv1.SecurityContextConstraints, error) { +func SecurityContextConstraint(component string, withCustomSELinuxPolicy bool) (*securityv1.SecurityContextConstraints, error) { if component != ComponentResourceTopologyExporter { return nil, fmt.Errorf("component %q is not an %q component", component, ComponentResourceTopologyExporter) } @@ -458,6 +458,9 @@ func SecurityContextConstraint(component string) (*securityv1.SecurityContextCon Level: selinuxassets.RTEContextLevel, }, } + if withCustomSELinuxPolicy { + scc.SELinuxContext.SELinuxOptions.Type = selinuxassets.RTEContextTypeLegacy + } return scc, nil } diff --git a/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/manifests/rte/rte.go b/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/manifests/rte/rte.go index 7cdaf2650..1577d4bc2 100644 --- a/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/manifests/rte/rte.go +++ b/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/manifests/rte/rte.go @@ -17,6 +17,7 @@ package rte import ( + selinuxassets "github.com/k8stopologyawareschedwg/deployer/pkg/assets/selinux" securityv1 "github.com/openshift/api/security/v1" machineconfigv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" appsv1 "k8s.io/api/apps/v1" @@ -67,8 +68,11 @@ func (mf Manifests) Clone() Manifests { ConfigMap: mf.ConfigMap.DeepCopy(), } - if mf.plat == platform.OpenShift { - ret.MachineConfig = mf.MachineConfig.DeepCopy() + if mf.plat == platform.OpenShift || mf.plat == platform.HyperShift { + // MachineConfig is obsolete starting from OCP v4.18 + if mf.MachineConfig != nil { + ret.MachineConfig = mf.MachineConfig.DeepCopy() + } ret.SecurityContextConstraint = mf.SecurityContextConstraint.DeepCopy() } @@ -107,15 +111,19 @@ func (mf Manifests) Render(opts options.UpdaterDaemon) (Manifests, error) { } rteupdate.DaemonSet(ret.DaemonSet, mf.plat, rteConfigMapName, opts.DaemonSet) - if mf.plat == platform.OpenShift { - rteupdate.SecurityContext(ret.DaemonSet) - - if opts.Name != "" { - ret.MachineConfig.Name = ocpupdate.MakeMachineConfigName(opts.Name) - } - if opts.MachineConfigPoolSelector != nil { - ret.MachineConfig.Labels = opts.MachineConfigPoolSelector.MatchLabels + if mf.plat == platform.OpenShift || mf.plat == platform.HyperShift { + selinuxType := selinuxassets.RTEContextType + if mf.MachineConfig != nil { + if opts.Name != "" { + ret.MachineConfig.Name = ocpupdate.MakeMachineConfigName(opts.Name) + } + if opts.MachineConfigPoolSelector != nil { + ret.MachineConfig.Labels = opts.MachineConfigPoolSelector.MatchLabels + } + // the MachineConfig installs this custom policy which is obsolete starting from OCP v4.18 + selinuxType = selinuxassets.RTEContextTypeLegacy } + rteupdate.SecurityContext(ret.DaemonSet, selinuxType) ocpupdate.SecurityContextConstraint(ret.SecurityContextConstraint, ret.ServiceAccount) } @@ -173,17 +181,19 @@ func New(plat platform.Platform) Manifests { return mf } -func GetManifests(plat platform.Platform, version platform.Version, namespace string, withCRIHooks bool) (Manifests, error) { +func GetManifests(plat platform.Platform, version platform.Version, namespace string, withCRIHooks, withCustomSELinuxPolicy bool) (Manifests, error) { var err error mf := New(plat) - if plat == platform.OpenShift { - mf.MachineConfig, err = manifests.MachineConfig(manifests.ComponentResourceTopologyExporter, version, withCRIHooks) - if err != nil { - return mf, err + if plat == platform.OpenShift || plat == platform.HyperShift { + if withCustomSELinuxPolicy { + mf.MachineConfig, err = manifests.MachineConfig(manifests.ComponentResourceTopologyExporter, version, withCRIHooks) + if err != nil { + return mf, err + } } - mf.SecurityContextConstraint, err = manifests.SecurityContextConstraint(manifests.ComponentResourceTopologyExporter) + mf.SecurityContextConstraint, err = manifests.SecurityContextConstraint(manifests.ComponentResourceTopologyExporter, withCustomSELinuxPolicy) if err != nil { return mf, err } diff --git a/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/objectupdate/rte/rte.go b/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/objectupdate/rte/rte.go index 911fa7ddd..942539ce0 100644 --- a/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/objectupdate/rte/rte.go +++ b/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/objectupdate/rte/rte.go @@ -174,7 +174,7 @@ func MetricsPort(ds *appsv1.DaemonSet, pNum int) { cntSpec.Ports = cp } -func SecurityContext(ds *appsv1.DaemonSet) { +func SecurityContext(ds *appsv1.DaemonSet, selinuxContextType string) { cntSpec := objectupdate.FindContainerByName(ds.Spec.Template.Spec.Containers, manifests.ContainerNameRTE) if cntSpec == nil { return @@ -186,7 +186,7 @@ func SecurityContext(ds *appsv1.DaemonSet) { cntSpec.SecurityContext = &corev1.SecurityContext{} } cntSpec.SecurityContext.SELinuxOptions = &corev1.SELinuxOptions{ - Type: selinuxassets.RTEContextType, + Type: selinuxContextType, Level: selinuxassets.RTEContextLevel, } } diff --git a/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/options/options.go b/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/options/options.go index cb85c6391..e666e9598 100644 --- a/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/options/options.go +++ b/vendor/github.com/k8stopologyawareschedwg/deployer/pkg/options/options.go @@ -34,6 +34,7 @@ type Options struct { UpdaterPFPEnable bool UpdaterNotifEnable bool UpdaterCRIHooksEnable bool + UpdaterCustomSELinuxPolicy bool UpdaterSyncPeriod time.Duration UpdaterVerbose int SchedProfileName string @@ -88,12 +89,13 @@ type UpdaterDaemon struct { } type Updater struct { - Platform platform.Platform - PlatformVersion platform.Version - WaitCompletion bool - RTEConfigData string - DaemonSet DaemonSet - EnableCRIHooks bool + Platform platform.Platform + PlatformVersion platform.Version + WaitCompletion bool + RTEConfigData string + DaemonSet DaemonSet + EnableCRIHooks bool + CustomSELinuxPolicy bool } func ForDaemonSet(commonOpts *Options) DaemonSet { diff --git a/vendor/modules.txt b/vendor/modules.txt index 0ad17fa23..ba033101f 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -175,7 +175,7 @@ github.com/josharian/intern # github.com/json-iterator/go v1.1.12 ## explicit; go 1.12 github.com/json-iterator/go -# github.com/k8stopologyawareschedwg/deployer v0.20.4 +# github.com/k8stopologyawareschedwg/deployer v0.21.0 ## explicit; go 1.21 github.com/k8stopologyawareschedwg/deployer/pkg/assets/rte github.com/k8stopologyawareschedwg/deployer/pkg/assets/selinux From da3445f500d798b0981be5c039051e408b0e3ae6 Mon Sep 17 00:00:00 2001 From: Talor Itzhak Date: Wed, 25 Sep 2024 15:31:55 +0300 Subject: [PATCH 02/10] annotation: emergency annotation to enable custom policy When cluster gets upgrade from 4.1X -> 4.18 the operator removes the MachineConfig (that contains the custom SELinux policy) and using the built-in policy instead. But we still want to have a way to enable the custom SELinux in case that something breaks during upgrade. For that we introduce a special annotation that would be present in the NUMAResourcesOperator CR and would force it to use the custom (legacy) SELinux. Once the annotation specified it would apply on all RTE pods regardless of their association with the different NodeGroups. Signed-off-by: Talor Itzhak --- internal/api/annotations/annotations.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 internal/api/annotations/annotations.go diff --git a/internal/api/annotations/annotations.go b/internal/api/annotations/annotations.go new file mode 100644 index 000000000..a6ba2164a --- /dev/null +++ b/internal/api/annotations/annotations.go @@ -0,0 +1,13 @@ +package annotations + +const ( + SELinuxPolicyConfigAnnotation = "config.node.openshift-kni.io/selinux-policy" + SELinuxPolicyCustom = "custom" +) + +func IsCustomPolicyEnabled(annot map[string]string) bool { + if v, ok := annot[SELinuxPolicyConfigAnnotation]; ok && v == SELinuxPolicyCustom { + return true + } + return false +} From b01893db99e3a67348f7992f6e1ff368afd1488d Mon Sep 17 00:00:00 2001 From: Talor Itzhak Date: Tue, 24 Sep 2024 20:38:50 +0300 Subject: [PATCH 03/10] controller: update MachineConfig reconciliation Starting from 4.18 RTE pods can use a built-in SELinux policy instead of custom one. This means that MachineConfig deployment is not mandatory anymore. However, it is still an option to deploy a custom policy using MachineConfig, in case of a failure. This commit updates the MahcineConfig reconciliation logic. By default when upgrade is done from 4.1X -> 4.18 the controller tries to remove the redundant MahcineConfig, unless user state explicitly that custom policy is needed using special annotation. Signed-off-by: Talor Itzhak --- .../numaresourcesoperator_controller.go | 68 ++++++++++++------- .../numaresourcesoperator_controller_test.go | 2 +- main.go | 2 +- pkg/objectstate/rte/rte.go | 3 +- 4 files changed, 46 insertions(+), 29 deletions(-) diff --git a/controllers/numaresourcesoperator_controller.go b/controllers/numaresourcesoperator_controller.go index 6e2be4780..38276fba3 100644 --- a/controllers/numaresourcesoperator_controller.go +++ b/controllers/numaresourcesoperator_controller.go @@ -22,10 +22,12 @@ import ( "reflect" "time" + "github.com/k8stopologyawareschedwg/deployer/pkg/assets/selinux" "github.com/k8stopologyawareschedwg/deployer/pkg/deployer/platform" "github.com/k8stopologyawareschedwg/deployer/pkg/manifests" apimanifests "github.com/k8stopologyawareschedwg/deployer/pkg/manifests/api" rtemanifests "github.com/k8stopologyawareschedwg/deployer/pkg/manifests/rte" + k8swgrteupdate "github.com/k8stopologyawareschedwg/deployer/pkg/objectupdate/rte" securityv1 "github.com/openshift/api/security/v1" machineconfigv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" "github.com/pkg/errors" @@ -52,6 +54,7 @@ import ( nropv1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1" nodegroupv1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1/helper/nodegroup" + "github.com/openshift-kni/numaresources-operator/internal/api/annotations" "github.com/openshift-kni/numaresources-operator/internal/relatedobjects" "github.com/openshift-kni/numaresources-operator/pkg/apply" "github.com/openshift-kni/numaresources-operator/pkg/hash" @@ -81,6 +84,8 @@ type NUMAResourcesOperatorReconciler struct { ForwardMCPConds bool } +type mcpWaitForUpdatedFunc func(string, *machineconfigv1.MachineConfigPool) bool + // TODO: narrow down // Namespace Scoped @@ -211,16 +216,16 @@ func (r *NUMAResourcesOperatorReconciler) reconcileResourceAPI(ctx context.Conte func (r *NUMAResourcesOperatorReconciler) reconcileResourceMachineConfig(ctx context.Context, instance *nropv1.NUMAResourcesOperator, trees []nodegroupv1.Tree) (bool, ctrl.Result, string, error) { // we need to sync machine configs first and wait for the MachineConfigPool updates // before checking additional components for updates - _, err := r.syncMachineConfigs(ctx, instance, trees) + mcpUpdatedFunc, err := r.syncMachineConfigs(ctx, instance, trees) if err != nil { r.Recorder.Eventf(instance, corev1.EventTypeWarning, "FailedMCSync", "Failed to set up machine configuration for worker nodes: %v", err) return true, ctrl.Result{}, status.ConditionDegraded, errors.Wrapf(err, "failed to sync machine configs") } r.Recorder.Eventf(instance, corev1.EventTypeNormal, "SuccessfulMCSync", "Enabled machine configuration for worker nodes") - // MCO need to update SELinux context and other stuff, and need to trigger a reboot. + // MCO needs to update the SELinux context removal and other stuff, and need to trigger a reboot. // It can take a while. - mcpStatuses, allMCPsUpdated := syncMachineConfigPoolsStatuses(instance.Name, trees, r.ForwardMCPConds) + mcpStatuses, allMCPsUpdated := syncMachineConfigPoolsStatuses(instance.Name, trees, r.ForwardMCPConds, mcpUpdatedFunc) instance.Status.MachineConfigPools = mcpStatuses if !allMCPsUpdated { // the Machine Config Pool still did not apply the machine config, wait for one minute @@ -318,41 +323,54 @@ func (r *NUMAResourcesOperatorReconciler) syncNodeResourceTopologyAPI(ctx contex return (updatedCount == len(objStates)), err } -func (r *NUMAResourcesOperatorReconciler) syncMachineConfigs(ctx context.Context, instance *nropv1.NUMAResourcesOperator, trees []nodegroupv1.Tree) (bool, error) { +func (r *NUMAResourcesOperatorReconciler) syncMachineConfigs(ctx context.Context, instance *nropv1.NUMAResourcesOperator, trees []nodegroupv1.Tree) (mcpWaitForUpdatedFunc, error) { klog.V(4).InfoS("Machine Config Sync start", "trees", len(trees)) defer klog.V(4).Info("Machine Config Sync stop") existing := rtestate.FromClient(ctx, r.Client, r.Platform, r.RTEManifests, instance, trees, r.Namespace) var err error - var updatedCount int + var mcpUpdatedFunc mcpWaitForUpdatedFunc objStates := existing.MachineConfigsState(r.RTEManifests) - for _, objState := range objStates { - if err2 := controllerutil.SetControllerReference(instance, objState.Desired, r.Scheme); err2 != nil { - err = errors.Wrapf(err2, "failed to set controller reference to %s %s", objState.Desired.GetNamespace(), objState.Desired.GetName()) - break + // Since 4.18 we're using a built-in SELinux policy, + // so the MachineConfig which applies the custom policy is no longer necessary. + // In case of operator upgrade from 4.1X → 4.18, it's necessary to remove the old MachineConfig, + // unless an emergency annotation is provided which forces the operator to use custom policy + if annotations.IsCustomPolicyEnabled(instance.Annotations) { + for _, objState := range objStates { + if !objState.IsNotFoundError() { + klog.V(4).InfoS("delete Machine Config", "MachineConfig", objState.Desired.GetName()) + if err2 := r.Client.Delete(ctx, objState.Desired); err2 != nil { + err = errors.Wrapf(err2, "could not delete MachineConfig %s", objState.Desired.GetName()) + } + klog.V(4).InfoS("Machine Config deleted successfully", "MachineConfig", objState.Desired.GetName()) + } // if not found, it's a fresh installation of 4.18+ (no upgrade) } + mcpUpdatedFunc = IsMachineConfigPoolUpdatedAfterDeletion + } else { + for _, objState := range objStates { + if err2 := controllerutil.SetControllerReference(instance, objState.Desired, r.Scheme); err2 != nil { + err = errors.Wrapf(err2, "failed to set controller reference to %s %s", objState.Desired.GetNamespace(), objState.Desired.GetName()) + break + } - if err2 := validateMachineConfigLabels(objState.Desired, trees); err2 != nil { - err = errors.Wrapf(err2, "machine conig %q labels validation failed", objState.Desired.GetName()) - break - } + if err2 := validateMachineConfigLabels(objState.Desired, trees); err2 != nil { + err = errors.Wrapf(err2, "machine conig %q labels validation failed", objState.Desired.GetName()) + break + } - _, updated, err2 := apply.ApplyObject(ctx, r.Client, objState) - if err2 != nil { - err = errors.Wrapf(err2, "could not apply (%s) %s/%s", objState.Desired.GetObjectKind().GroupVersionKind(), objState.Desired.GetNamespace(), objState.Desired.GetName()) - break - } - if !updated { - continue + _, _, err2 := apply.ApplyObject(ctx, r.Client, objState) + if err2 != nil { + err = errors.Wrapf(err2, "could not apply (%s) %s/%s", objState.Desired.GetObjectKind().GroupVersionKind(), objState.Desired.GetNamespace(), objState.Desired.GetName()) + break + } } - updatedCount++ + mcpUpdatedFunc = IsMachineConfigPoolUpdated } - - return (updatedCount == len(objStates)), err + return mcpUpdatedFunc, err } -func syncMachineConfigPoolsStatuses(instanceName string, trees []nodegroupv1.Tree, forwardMCPConds bool) ([]nropv1.MachineConfigPool, bool) { +func syncMachineConfigPoolsStatuses(instanceName string, trees []nodegroupv1.Tree, forwardMCPConds bool, updatedFunc mcpWaitForUpdatedFunc) ([]nropv1.MachineConfigPool, bool) { klog.V(4).InfoS("Machine Config Status Sync start", "trees", len(trees)) defer klog.V(4).Info("Machine Config Status Sync stop") @@ -361,7 +379,7 @@ func syncMachineConfigPoolsStatuses(instanceName string, trees []nodegroupv1.Tre for _, mcp := range tree.MachineConfigPools { mcpStatuses = append(mcpStatuses, extractMCPStatus(mcp, forwardMCPConds)) - isUpdated := IsMachineConfigPoolUpdated(instanceName, mcp) + isUpdated := updatedFunc(instanceName, mcp) klog.V(5).InfoS("Machine Config Pool state", "name", mcp.Name, "instance", instanceName, "updated", isUpdated) if !isUpdated { diff --git a/controllers/numaresourcesoperator_controller_test.go b/controllers/numaresourcesoperator_controller_test.go index c07ac6534..11b581e07 100644 --- a/controllers/numaresourcesoperator_controller_test.go +++ b/controllers/numaresourcesoperator_controller_test.go @@ -67,7 +67,7 @@ func NewFakeNUMAResourcesOperatorReconciler(plat platform.Platform, platVersion return nil, err } - rteManifests, err := rtemanifests.GetManifests(plat, platVersion, testNamespace, true) + rteManifests, err := rtemanifests.GetManifests(plat, platVersion, testNamespace, false, true) if err != nil { return nil, err } diff --git a/main.go b/main.go index 8ca257c5f..240881e14 100644 --- a/main.go +++ b/main.go @@ -211,7 +211,7 @@ func main() { namespace = defaultNamespace } - rteManifests, err := rtemanifests.GetManifests(clusterPlatform, clusterPlatformVersion, namespace, true) + rteManifests, err := rtemanifests.GetManifests(clusterPlatform, clusterPlatformVersion, namespace, false, true) if err != nil { klog.ErrorS(err, "unable to load the RTE manifests") os.Exit(1) diff --git a/pkg/objectstate/rte/rte.go b/pkg/objectstate/rte/rte.go index 6e1c0fe64..2d5952976 100644 --- a/pkg/objectstate/rte/rte.go +++ b/pkg/objectstate/rte/rte.go @@ -91,7 +91,6 @@ func (em *ExistingManifests) MachineConfigsState(mf rtemanifests.Manifests) []ob klog.Warningf("failed to find machine config %q under the namespace %q", mcName, desiredMachineConfig.Namespace) continue } - ret = append(ret, objectstate.ObjectState{ Existing: existingMachineConfig.machineConfig, @@ -281,7 +280,7 @@ func FromClient(ctx context.Context, cli client.Client, plat platform.Platform, ret.existing.ServiceAccount = sa } - if plat == platform.OpenShift { + if plat != platform.Kubernetes { scc := &securityv1.SecurityContextConstraints{} if ret.sccError = cli.Get(ctx, client.ObjectKeyFromObject(mf.SecurityContextConstraint), scc); ret.sccError == nil { ret.existing.SecurityContextConstraint = scc From 3e1611ae4cfb8fbad509db1d00798387e0a28791 Mon Sep 17 00:00:00 2001 From: Talor Itzhak Date: Tue, 24 Sep 2024 20:40:03 +0300 Subject: [PATCH 04/10] selinux: use correct SELinux context Use the legacy RTE pod context, if the emergency annotation is provided. Signed-off-by: Talor Itzhak --- controllers/numaresourcesoperator_controller.go | 12 +++++++++++- pkg/objectstate/rte/rte.go | 15 ++++++++------- pkg/objectupdate/rte/rte.go | 11 +++++++++++ 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/controllers/numaresourcesoperator_controller.go b/controllers/numaresourcesoperator_controller.go index 38276fba3..548e7106f 100644 --- a/controllers/numaresourcesoperator_controller.go +++ b/controllers/numaresourcesoperator_controller.go @@ -479,9 +479,10 @@ func (r *NUMAResourcesOperatorReconciler) syncNUMAResourcesOperatorResources(ctx } rteupdate.DaemonSetHashAnnotation(r.RTEManifests.DaemonSet, cmHash) } + rteupdate.SecurityContextConstraint(r.RTEManifests.SecurityContextConstraint, annotations.IsCustomPolicyEnabled(instance.Annotations)) existing := rtestate.FromClient(ctx, r.Client, r.Platform, r.RTEManifests, instance, trees, r.Namespace) - for _, objState := range existing.State(r.RTEManifests, daemonsetUpdater) { + for _, objState := range existing.State(r.RTEManifests, daemonsetUpdater, annotations.IsCustomPolicyEnabled(instance.Annotations)) { if objState.Error != nil { // We are likely in the bootstrap scenario. In this case, which is expected once, everything is fine. // If it happens past bootstrap, still carry on. We know what to do, and we do want to enforce the desired state. @@ -774,6 +775,15 @@ func daemonsetUpdater(mcpName string, gdm *rtestate.GeneratedDesiredManifest) er klog.V(5).InfoS("DaemonSet update: cannot update config", "mcp", mcpName, "daemonset", gdm.DaemonSet.Name, "error", err) return err } + if gdm.ClusterPlatform != platform.Kubernetes { + if gdm.IsCustomPolicyEnabled && gdm.ClusterPlatform == platform.OpenShift { + k8swgrteupdate.SecurityContext(gdm.DaemonSet, selinux.RTEContextTypeLegacy) + klog.V(5).InfoS("DaemonSet update: selinux options", "container", manifests.ContainerNameRTE, "context", selinux.RTEContextTypeLegacy) + } else { + k8swgrteupdate.SecurityContext(gdm.DaemonSet, selinux.RTEContextType) + klog.V(5).InfoS("DaemonSet update: selinux options", "container", manifests.ContainerNameRTE, "context", selinux.RTEContextType) + } + } return nil } diff --git a/pkg/objectstate/rte/rte.go b/pkg/objectstate/rte/rte.go index 2d5952976..10f7e9664 100644 --- a/pkg/objectstate/rte/rte.go +++ b/pkg/objectstate/rte/rte.go @@ -19,7 +19,6 @@ package rte import ( "context" "fmt" - appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -128,7 +127,8 @@ type GeneratedDesiredManifest struct { MachineConfigPool *machineconfigv1.MachineConfigPool NodeGroup *nropv1.NodeGroup // generated manifests - DaemonSet *appsv1.DaemonSet + DaemonSet *appsv1.DaemonSet + IsCustomPolicyEnabled bool } type GenerateDesiredManifestUpdater func(mcpName string, gdm *GeneratedDesiredManifest) error @@ -137,7 +137,7 @@ func SkipManifestUpdate(gdm *GeneratedDesiredManifest) error { return nil } -func (em *ExistingManifests) State(mf rtemanifests.Manifests, updater GenerateDesiredManifestUpdater) []objectstate.ObjectState { +func (em *ExistingManifests) State(mf rtemanifests.Manifests, updater GenerateDesiredManifestUpdater, isCustomPolicyEnabled bool) []objectstate.ObjectState { ret := []objectstate.ObjectState{ // service account { @@ -217,10 +217,11 @@ func (em *ExistingManifests) State(mf rtemanifests.Manifests, updater GenerateDe if updater != nil { gdm := GeneratedDesiredManifest{ - ClusterPlatform: em.plat, - MachineConfigPool: mcp.DeepCopy(), - NodeGroup: tree.NodeGroup.DeepCopy(), - DaemonSet: desiredDaemonSet, + ClusterPlatform: em.plat, + MachineConfigPool: mcp.DeepCopy(), + NodeGroup: tree.NodeGroup.DeepCopy(), + DaemonSet: desiredDaemonSet, + IsCustomPolicyEnabled: isCustomPolicyEnabled, } err := updater(mcp.Name, &gdm) diff --git a/pkg/objectupdate/rte/rte.go b/pkg/objectupdate/rte/rte.go index 9d0d351e5..e56e24b5b 100644 --- a/pkg/objectupdate/rte/rte.go +++ b/pkg/objectupdate/rte/rte.go @@ -25,6 +25,9 @@ import ( "k8s.io/apimachinery/pkg/api/resource" "k8s.io/klog/v2" + securityv1 "github.com/openshift/api/security/v1" + + "github.com/k8stopologyawareschedwg/deployer/pkg/assets/selinux" "github.com/k8stopologyawareschedwg/deployer/pkg/flagcodec" k8swgobjupdate "github.com/k8stopologyawareschedwg/deployer/pkg/objectupdate" k8swgrteupdate "github.com/k8stopologyawareschedwg/deployer/pkg/objectupdate/rte" @@ -219,6 +222,14 @@ func AddVolumeMountMemory(podSpec *corev1.PodSpec, cnt *corev1.Container, mountN ) } +func SecurityContextConstraint(scc *securityv1.SecurityContextConstraints, legacyRTEContext bool) { + if legacyRTEContext { + scc.SELinuxContext.SELinuxOptions.Type = selinux.RTEContextTypeLegacy + return + } + scc.SELinuxContext.SELinuxOptions.Type = selinux.RTEContextType +} + func isPodFingerprintEnabled(conf *nropv1.NodeGroupConfig) (bool, string) { cfg := nropv1.DefaultNodeGroupConfig() if conf == nil || conf.PodsFingerprinting == nil { From 21a1b61df51827e5f15b49d973d0738e0054f5a8 Mon Sep 17 00:00:00 2001 From: Talor Itzhak Date: Thu, 26 Sep 2024 11:20:14 +0300 Subject: [PATCH 05/10] test-unit: emulating upgrade to 4.18 Emulating an integration test that emulates an upgrade from 4.1X -> 4.18 and verifies that the MachineConfigs are deleted. Signed-off-by: Talor Itzhak --- .../numaresourcesoperator_controller_test.go | 147 ++++++++++++++++++ internal/objects/objects.go | 19 +++ 2 files changed, 166 insertions(+) diff --git a/controllers/numaresourcesoperator_controller_test.go b/controllers/numaresourcesoperator_controller_test.go index 11b581e07..47ef6c0ca 100644 --- a/controllers/numaresourcesoperator_controller_test.go +++ b/controllers/numaresourcesoperator_controller_test.go @@ -18,6 +18,7 @@ package controllers import ( "context" + "github.com/openshift-kni/numaresources-operator/internal/api/annotations" "time" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -149,6 +150,7 @@ var _ = Describe("Test NUMAResourcesOperator Reconcile", func() { {MatchLabels: label1}, {MatchLabels: label2}, }) + nro.Annotations = map[string]string{annotations.SELinuxPolicyConfigAnnotation: annotations.SELinuxPolicyCustom} mcp1 = testobjs.NewMachineConfigPool("test1", label1, &metav1.LabelSelector{MatchLabels: label1}, &metav1.LabelSelector{MatchLabels: label1}) mcp2 = testobjs.NewMachineConfigPool("test2", label2, &metav1.LabelSelector{MatchLabels: label2}, &metav1.LabelSelector{MatchLabels: label2}) @@ -210,6 +212,41 @@ var _ = Describe("Test NUMAResourcesOperator Reconcile", func() { } Expect(reconciler.Client.Get(context.TODO(), mcp2DSKey, ds)).To(Succeed()) }) + When("NRO updated to remove the custom policy annotation", func() { + BeforeEach(func() { + // check we have at least two NodeGroups + Expect(len(nro.Spec.NodeGroups)).To(BeNumerically(">", 1)) + + By("Update NRO to have both NodeGroups") + key := client.ObjectKeyFromObject(nro) + nro := &nropv1.NUMAResourcesOperator{} + Expect(reconciler.Client.Get(context.TODO(), key, nro)).NotTo(HaveOccurred()) + + nro.Annotations = map[string]string{} + Expect(reconciler.Client.Update(context.TODO(), nro)).NotTo(HaveOccurred()) + + thirdLoopResult, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key}) + Expect(err).ToNot(HaveOccurred()) + Expect(thirdLoopResult).To(Equal(reconcile.Result{RequeueAfter: time.Minute})) + }) + It("should not create a machine config", func() { + mc := &machineconfigv1.MachineConfig{} + + // Check mc1 not created + mc1Key := client.ObjectKey{ + Name: objectnames.GetMachineConfigName(nro.Name, mcp1.Name), + } + err := reconciler.Client.Get(context.TODO(), mc1Key, mc) + Expect(apierrors.IsNotFound(err)).To(BeTrue(), "MachineConfig %s is expected to not be found", mc1Key.String()) + + // Check mc1 not created + mc2Key := client.ObjectKey{ + Name: objectnames.GetMachineConfigName(nro.Name, mcp2.Name), + } + err = reconciler.Client.Get(context.TODO(), mc2Key, mc) + Expect(apierrors.IsNotFound(err)).To(BeTrue(), "MachineConfig %s is expected to not be found", mc2Key.String()) + }) + }) When("a NodeGroup is deleted", func() { BeforeEach(func() { // check we have at least two NodeGroups @@ -223,6 +260,7 @@ var _ = Describe("Test NUMAResourcesOperator Reconcile", func() { nro.Spec.NodeGroups = []nropv1.NodeGroup{{ MachineConfigPoolSelector: &metav1.LabelSelector{MatchLabels: label1}, }} + nro.Annotations = map[string]string{annotations.SELinuxPolicyConfigAnnotation: annotations.SELinuxPolicyCustom} Expect(reconciler.Client.Update(context.TODO(), nro)).NotTo(HaveOccurred()) thirdLoopResult, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key}) @@ -319,6 +357,7 @@ var _ = Describe("Test NUMAResourcesOperator Reconcile", func() { {MatchLabels: label1}, {MatchLabels: label2}, }) + nro.Annotations = map[string]string{annotations.SELinuxPolicyConfigAnnotation: annotations.SELinuxPolicyCustom} mcp1 = testobjs.NewMachineConfigPool("test1", label1, &metav1.LabelSelector{MatchLabels: label1}, &metav1.LabelSelector{MatchLabels: label1}) mcp2 = testobjs.NewMachineConfigPool("test2", label2, &metav1.LabelSelector{MatchLabels: label2}, &metav1.LabelSelector{MatchLabels: label2}) @@ -558,6 +597,7 @@ var _ = Describe("Test NUMAResourcesOperator Reconcile", func() { }, }, } + nro.Annotations = map[string]string{annotations.SELinuxPolicyConfigAnnotation: annotations.SELinuxPolicyCustom} }) When("machine config selector matches machine config labels", func() { @@ -1088,6 +1128,113 @@ var _ = Describe("Test NUMAResourcesOperator Reconcile", func() { Expect(ds.Spec.Template.Spec.Tolerations).To(Equal(reconciler.RTEManifests.DaemonSet.Spec.Template.Spec.Tolerations), "DS tolerations not restored to defaults") }) }) + Context("emulating upgrade from 4.1X to 4.18 which has a built-in selinux policy for RTE pods", func() { + var nro *nropv1.NUMAResourcesOperator + var mcp1 *machineconfigv1.MachineConfigPool + var mcp2 *machineconfigv1.MachineConfigPool + + var reconciler *NUMAResourcesOperatorReconciler + var label1, label2 map[string]string + + BeforeEach(func() { + label1 = map[string]string{ + "test1": "test1", + } + label2 = map[string]string{ + "test2": "test2", + } + + nro = testobjs.NewNUMAResourcesOperator(objectnames.DefaultNUMAResourcesOperatorCrName, []*metav1.LabelSelector{ + {MatchLabels: label1}, + {MatchLabels: label2}, + }) + // reconciling NRO object with custom policy, emulates the old behavior version + nro.Annotations = map[string]string{annotations.SELinuxPolicyConfigAnnotation: annotations.SELinuxPolicyCustom} + + mcp1 = testobjs.NewMachineConfigPool("test1", label1, &metav1.LabelSelector{MatchLabels: label1}, &metav1.LabelSelector{MatchLabels: label1}) + mcp2 = testobjs.NewMachineConfigPool("test2", label2, &metav1.LabelSelector{MatchLabels: label2}, &metav1.LabelSelector{MatchLabels: label2}) + + var err error + reconciler, err = NewFakeNUMAResourcesOperatorReconciler(platform.OpenShift, defaultOCPVersion, nro, mcp1, mcp2) + Expect(err).ToNot(HaveOccurred()) + + key := client.ObjectKeyFromObject(nro) + firstLoopResult, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key}) + Expect(err).ToNot(HaveOccurred()) + Expect(firstLoopResult).To(Equal(reconcile.Result{RequeueAfter: time.Minute})) + + // Ensure mcp1 is ready + Expect(reconciler.Client.Get(context.TODO(), client.ObjectKeyFromObject(mcp1), mcp1)).To(Succeed()) + mcp1.Status.Configuration.Source = []corev1.ObjectReference{ + { + Name: objectnames.GetMachineConfigName(nro.Name, mcp1.Name), + }, + } + mcp1.Status.Conditions = []machineconfigv1.MachineConfigPoolCondition{ + { + Type: machineconfigv1.MachineConfigPoolUpdated, + Status: corev1.ConditionTrue, + }, + } + Expect(reconciler.Client.Update(context.TODO(), mcp1)).To(Succeed()) + + // ensure mcp2 is ready + Expect(reconciler.Client.Get(context.TODO(), client.ObjectKeyFromObject(mcp2), mcp2)).To(Succeed()) + mcp2.Status.Configuration.Source = []corev1.ObjectReference{ + { + Name: objectnames.GetMachineConfigName(nro.Name, mcp2.Name), + }, + } + mcp2.Status.Conditions = []machineconfigv1.MachineConfigPoolCondition{ + { + Type: machineconfigv1.MachineConfigPoolUpdated, + Status: corev1.ConditionTrue, + }, + } + Expect(reconciler.Client.Update(context.TODO(), mcp2)).To(Succeed()) + + secondLoopResult, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key}) + Expect(err).ToNot(HaveOccurred()) + Expect(secondLoopResult).To(Equal(reconcile.Result{RequeueAfter: 5 * time.Second})) + + By("Check DaemonSets are created") + mcp1DSKey := client.ObjectKey{ + Name: objectnames.GetComponentName(nro.Name, mcp1.Name), + Namespace: testNamespace, + } + ds := &appsv1.DaemonSet{} + Expect(reconciler.Client.Get(context.TODO(), mcp1DSKey, ds)).ToNot(HaveOccurred()) + + mcp2DSKey := client.ObjectKey{ + Name: objectnames.GetComponentName(nro.Name, mcp2.Name), + Namespace: testNamespace, + } + Expect(reconciler.Client.Get(context.TODO(), mcp2DSKey, ds)).To(Succeed()) + + By("upgrading from 4.1X to 4.18") + Expect(reconciler.Client.Get(context.TODO(), client.ObjectKeyFromObject(nro), nro)).To(Succeed()) + nro.Annotations = map[string]string{} + Expect(reconciler.Client.Update(context.TODO(), nro)).To(Succeed()) + + thirdLoopResult, err := reconciler.Reconcile(context.TODO(), reconcile.Request{NamespacedName: key}) + Expect(err).ToNot(HaveOccurred()) + Expect(thirdLoopResult).To(Equal(reconcile.Result{RequeueAfter: time.Minute})) + }) + It("should delete existing mc", func() { + mc1Key := client.ObjectKey{ + Name: objectnames.GetMachineConfigName(nro.Name, mcp1.Name), + } + mc := &machineconfigv1.MachineConfig{} + err := reconciler.Client.Get(context.TODO(), mc1Key, mc) + Expect(apierrors.IsNotFound(err)).To(BeTrue(), "MachineConfig %s expected to be deleted; err=%v", mc1Key.Name, err) + + mc2Key := client.ObjectKey{ + Name: objectnames.GetMachineConfigName(nro.Name, mcp2.Name), + } + err = reconciler.Client.Get(context.TODO(), mc2Key, mc) + Expect(apierrors.IsNotFound(err)).To(BeTrue(), "MachineConfig %s expected to be deleted; err=%v", mc2Key.Name, err) + }) + }) }) func getConditionByType(conditions []metav1.Condition, conditionType string) *metav1.Condition { diff --git a/internal/objects/objects.go b/internal/objects/objects.go index 55e61ff50..4ca53b351 100644 --- a/internal/objects/objects.go +++ b/internal/objects/objects.go @@ -27,6 +27,7 @@ import ( kubeletconfigv1beta1 "k8s.io/kubelet/config/v1beta1" nropv1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1" + "github.com/openshift-kni/numaresources-operator/internal/api/annotations" ) func NewNUMAResourcesOperator(name string, labelSelectors []*metav1.LabelSelector) *nropv1.NUMAResourcesOperator { @@ -59,6 +60,10 @@ func NewNUMAResourcesOperatorWithNodeGroupConfig(name string, selector *metav1.L }, ObjectMeta: metav1.ObjectMeta{ Name: name, + // add the custom policy to keep the existing test's behavior consistent + Annotations: map[string]string{ + annotations.SELinuxPolicyConfigAnnotation: annotations.SELinuxPolicyCustom, + }, }, Spec: nropv1.NUMAResourcesOperatorSpec{ NodeGroups: []nropv1.NodeGroup{ @@ -107,6 +112,20 @@ func NewMachineConfigPool(name string, labels map[string]string, machineConfigSe } } +func NewMachineConfig(name string, labels map[string]string) *machineconfigv1.MachineConfig { + return &machineconfigv1.MachineConfig{ + TypeMeta: metav1.TypeMeta{ + Kind: "MachineConfig", + APIVersion: machineconfigv1.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Labels: labels, + }, + Spec: machineconfigv1.MachineConfigSpec{}, + } +} + func NewKubeletConfig(name string, labels map[string]string, machineConfigSelector *metav1.LabelSelector, kubeletConfig *kubeletconfigv1beta1.KubeletConfiguration) *machineconfigv1.KubeletConfig { data, _ := json.Marshal(kubeletConfig) return NewKubeletConfigWithData(name, labels, machineConfigSelector, data) From 1f59b17439d1523e075607d339e14c70a3b80d2c Mon Sep 17 00:00:00 2001 From: Talor Itzhak Date: Thu, 26 Sep 2024 12:57:21 +0300 Subject: [PATCH 06/10] tests: fix GetManifests signature Signed-off-by: Talor Itzhak --- test/e2e/install/install_test.go | 2 +- test/e2e/serial/tests/tolerations.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/install/install_test.go b/test/e2e/install/install_test.go index 05150357e..8c6cc57d3 100644 --- a/test/e2e/install/install_test.go +++ b/test/e2e/install/install_test.go @@ -288,7 +288,7 @@ var _ = Describe("[Install] durability", func() { By("checking there are no leftovers") // by taking the ns from the ds we're avoiding the need to figure out in advanced // at which ns we should look for the resources - mf, err := rte.GetManifests(configuration.Plat, configuration.PlatVersion, ds.Namespace, true) + mf, err := rte.GetManifests(configuration.Plat, configuration.PlatVersion, ds.Namespace, true, true) Expect(err).ToNot(HaveOccurred()) Eventually(func() bool { diff --git a/test/e2e/serial/tests/tolerations.go b/test/e2e/serial/tests/tolerations.go index f64def142..e0cc77b25 100644 --- a/test/e2e/serial/tests/tolerations.go +++ b/test/e2e/serial/tests/tolerations.go @@ -169,7 +169,7 @@ var _ = Describe("[serial][disruptive][rtetols] numaresources RTE tolerations su It("[tier3] should enable to change tolerations in the RTE daemonsets", Label("tier3"), func(ctx context.Context) { By("getting RTE manifests object") // TODO: this is similar but not quite what the main operator does - rteManifests, err := rtemanifests.GetManifests(configuration.Plat, configuration.PlatVersion, "", true) + rteManifests, err := rtemanifests.GetManifests(configuration.Plat, configuration.PlatVersion, "", true, true) Expect(err).ToNot(HaveOccurred(), "cannot get the RTE manifests") expectedTolerations := rteManifests.DaemonSet.Spec.Template.Spec.Tolerations // shortcut From 2c17b34684d8c4dffe2d41c46447d655e5b1ebcf Mon Sep 17 00:00:00 2001 From: Talor Itzhak Date: Mon, 30 Sep 2024 22:04:57 +0300 Subject: [PATCH 07/10] e2e: waitForMcpsCondition -> WaitForMCPsCondition This function waits for MCP defined condition, and can wait for multiple MCPs in parallel. Make the function public so it could be used in later commit. Signed-off-by: Talor Itzhak --- test/e2e/serial/tests/configuration.go | 2 +- test/e2e/serial/tests/tolerations.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/e2e/serial/tests/configuration.go b/test/e2e/serial/tests/configuration.go index 1350a95d7..99a96f7eb 100644 --- a/test/e2e/serial/tests/configuration.go +++ b/test/e2e/serial/tests/configuration.go @@ -1216,7 +1216,7 @@ func accumulateKubeletConfigNames(cms []corev1.ConfigMap) sets.Set[string] { return cmNames } -func waitForMcpsCondition(cli client.Client, ctx context.Context, mcps []*machineconfigv1.MachineConfigPool, condition machineconfigv1.MachineConfigPoolConditionType) error { +func WaitForMCPsCondition(cli client.Client, ctx context.Context, mcps []*machineconfigv1.MachineConfigPool, condition machineconfigv1.MachineConfigPoolConditionType) error { var eg errgroup.Group interval := configuration.MachineConfigPoolUpdateInterval if condition == machineconfigv1.MachineConfigPoolUpdating { diff --git a/test/e2e/serial/tests/tolerations.go b/test/e2e/serial/tests/tolerations.go index e0cc77b25..2c82b67ae 100644 --- a/test/e2e/serial/tests/tolerations.go +++ b/test/e2e/serial/tests/tolerations.go @@ -841,7 +841,7 @@ func waitForMcpUpdate(cli client.Client, ctx context.Context, mcpsInfo []mcpInfo By(fmt.Sprintf("verify updates for mcp %q", info.obj.Name)) klog.Info("waiting for mcp to start updating") - err := waitForMcpsCondition(cli, ctx, []*machineconfigv1.MachineConfigPool{info.obj}, machineconfigv1.MachineConfigPoolUpdating) + err := WaitForMCPsCondition(cli, ctx, []*machineconfigv1.MachineConfigPool{info.obj}, machineconfigv1.MachineConfigPoolUpdating) if err != nil { // just warn here because the switch between the mcp conditions: updated->updating->updated can be faster // and may be missed while the condition was actually met at some point @@ -850,7 +850,7 @@ func waitForMcpUpdate(cli client.Client, ctx context.Context, mcpsInfo []mcpInfo klog.Info("wait for mcp to get updated") //here we must fail on errors - Expect(waitForMcpsCondition(cli, ctx, []*machineconfigv1.MachineConfigPool{info.obj}, machineconfigv1.MachineConfigPoolUpdated)).To(Succeed()) + Expect(WaitForMCPsCondition(cli, ctx, []*machineconfigv1.MachineConfigPool{info.obj}, machineconfigv1.MachineConfigPoolUpdated)).To(Succeed()) var updatedMcp machineconfigv1.MachineConfigPool Expect(cli.Get(ctx, client.ObjectKeyFromObject(info.obj), &updatedMcp)).To(Succeed()) From f285882db3986cfb7fd159ca89e37673b69c92f2 Mon Sep 17 00:00:00 2001 From: Talor Itzhak Date: Mon, 30 Sep 2024 23:32:38 +0300 Subject: [PATCH 08/10] e2e: change MCP wait logic Now that we're using a built-in SELinux policy, we do not wait for the MachineConfig creation. We'll wait for MCP update only when the legacy policy is used or when we create KubeletConfig for the tests. In addition, we'll wait for MCP to transition from updated <-> updating. The old behavior was to wait for specific MachineConfig name, but in case of KubeletConfig creation for example it is not good enough. Signed-off-by: Talor Itzhak --- test/e2e/install/install_test.go | 12 +++- test/e2e/serial/tests/configuration.go | 27 --------- test/e2e/serial/tests/tolerations.go | 5 +- test/utils/deploy/deploy.go | 77 +++++++++++++------------- 4 files changed, 50 insertions(+), 71 deletions(-) diff --git a/test/e2e/install/install_test.go b/test/e2e/install/install_test.go index 8c6cc57d3..2fd80a8db 100644 --- a/test/e2e/install/install_test.go +++ b/test/e2e/install/install_test.go @@ -36,10 +36,13 @@ import ( "github.com/k8stopologyawareschedwg/deployer/pkg/manifests/rte" nropv1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1" + "github.com/openshift-kni/numaresources-operator/internal/api/annotations" "github.com/openshift-kni/numaresources-operator/pkg/status" + machineconfigv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" nrowait "github.com/openshift-kni/numaresources-operator/internal/wait" + nropmcp "github.com/openshift-kni/numaresources-operator/internal/machineconfigpools" e2eclient "github.com/openshift-kni/numaresources-operator/test/utils/clients" "github.com/openshift-kni/numaresources-operator/test/utils/configuration" "github.com/openshift-kni/numaresources-operator/test/utils/crds" @@ -194,7 +197,7 @@ var _ = Describe("[Install] durability", func() { nroObj := &nropv1.NUMAResourcesOperator{} immediate := true - err := wait.PollUntilContextTimeout(context.Background(), 10*time.Second, 5*time.Minute, immediate, func(ctx context.Context) (bool, error) { + err := wait.PollUntilContextTimeout(context.Background(), 10*time.Second, 10*time.Minute, immediate, func(ctx context.Context) (bool, error) { err := e2eclient.Client.Get(ctx, nroKey, nroObj) if err != nil { return false, err @@ -316,7 +319,12 @@ var _ = Describe("[Install] durability", func() { err = e2eclient.Client.Create(context.TODO(), nroObjRedep) Expect(err).ToNot(HaveOccurred()) - deploy.WaitForMCPUpdatedAfterNROCreated(nroObj) + if annotations.IsCustomPolicyEnabled(nroObj.Annotations) { + mcps, err := nropmcp.GetListByNodeGroupsV1(context.TODO(), e2eclient.Client, nroObj.Spec.NodeGroups) + Expect(err).NotTo(HaveOccurred()) + Expect(deploy.WaitForMCPsCondition(e2eclient.Client, context.TODO(), mcps, machineconfigv1.MachineConfigPoolUpdating)).To(Succeed()) + Expect(deploy.WaitForMCPsCondition(e2eclient.Client, context.TODO(), mcps, machineconfigv1.MachineConfigPoolUpdated)).To(Succeed()) + } Eventually(func() bool { updatedNroObj := &nropv1.NUMAResourcesOperator{} diff --git a/test/e2e/serial/tests/configuration.go b/test/e2e/serial/tests/configuration.go index 99a96f7eb..01ffa8281 100644 --- a/test/e2e/serial/tests/configuration.go +++ b/test/e2e/serial/tests/configuration.go @@ -42,8 +42,6 @@ import ( "sigs.k8s.io/yaml" "github.com/google/go-cmp/cmp" - "golang.org/x/sync/errgroup" - depnodes "github.com/k8stopologyawareschedwg/deployer/pkg/clientutil/nodes" nrtv1alpha2 "github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/apis/topology/v1alpha2" @@ -1216,31 +1214,6 @@ func accumulateKubeletConfigNames(cms []corev1.ConfigMap) sets.Set[string] { return cmNames } -func WaitForMCPsCondition(cli client.Client, ctx context.Context, mcps []*machineconfigv1.MachineConfigPool, condition machineconfigv1.MachineConfigPoolConditionType) error { - var eg errgroup.Group - interval := configuration.MachineConfigPoolUpdateInterval - if condition == machineconfigv1.MachineConfigPoolUpdating { - // the transition from updated to updating to updated can be very fast sometimes. so if - // the status changed to updating and then to updated while on wait it will miss the updating - // status, and it will keep waiting and lastly fail the test. to avoid that decrease the interval - // to allow more often checks for the status - interval = 2 * time.Second - } - for _, mcp := range mcps { - klog.Infof("wait for mcp %q to meet condition %q", mcp.Name, condition) - mcp := mcp - eg.Go(func() error { - defer GinkgoRecover() - err := wait.With(cli). - Interval(interval). - Timeout(configuration.MachineConfigPoolUpdateTimeout). - ForMachineConfigPoolCondition(ctx, mcp, condition) - return err - }) - } - return eg.Wait() -} - const ( roleMCPTest = "mcp-test" ) diff --git a/test/e2e/serial/tests/tolerations.go b/test/e2e/serial/tests/tolerations.go index 2c82b67ae..acb6d2884 100644 --- a/test/e2e/serial/tests/tolerations.go +++ b/test/e2e/serial/tests/tolerations.go @@ -46,6 +46,7 @@ import ( "github.com/openshift-kni/numaresources-operator/pkg/status" e2eclient "github.com/openshift-kni/numaresources-operator/test/utils/clients" "github.com/openshift-kni/numaresources-operator/test/utils/configuration" + "github.com/openshift-kni/numaresources-operator/test/utils/deploy" e2efixture "github.com/openshift-kni/numaresources-operator/test/utils/fixture" "github.com/openshift-kni/numaresources-operator/test/utils/k8simported/taints" "github.com/openshift-kni/numaresources-operator/test/utils/objects" @@ -841,7 +842,7 @@ func waitForMcpUpdate(cli client.Client, ctx context.Context, mcpsInfo []mcpInfo By(fmt.Sprintf("verify updates for mcp %q", info.obj.Name)) klog.Info("waiting for mcp to start updating") - err := WaitForMCPsCondition(cli, ctx, []*machineconfigv1.MachineConfigPool{info.obj}, machineconfigv1.MachineConfigPoolUpdating) + err := deploy.WaitForMCPsCondition(cli, ctx, []*machineconfigv1.MachineConfigPool{info.obj}, machineconfigv1.MachineConfigPoolUpdating) if err != nil { // just warn here because the switch between the mcp conditions: updated->updating->updated can be faster // and may be missed while the condition was actually met at some point @@ -850,7 +851,7 @@ func waitForMcpUpdate(cli client.Client, ctx context.Context, mcpsInfo []mcpInfo klog.Info("wait for mcp to get updated") //here we must fail on errors - Expect(WaitForMCPsCondition(cli, ctx, []*machineconfigv1.MachineConfigPool{info.obj}, machineconfigv1.MachineConfigPoolUpdated)).To(Succeed()) + Expect(deploy.WaitForMCPsCondition(cli, ctx, []*machineconfigv1.MachineConfigPool{info.obj}, machineconfigv1.MachineConfigPoolUpdated)).To(Succeed()) var updatedMcp machineconfigv1.MachineConfigPool Expect(cli.Get(ctx, client.ObjectKeyFromObject(info.obj), &updatedMcp)).To(Succeed()) diff --git a/test/utils/deploy/deploy.go b/test/utils/deploy/deploy.go index eaa21aa48..578f6589f 100644 --- a/test/utils/deploy/deploy.go +++ b/test/utils/deploy/deploy.go @@ -19,6 +19,8 @@ package deploy import ( "context" "fmt" + "github.com/openshift-kni/numaresources-operator/internal/api/annotations" + "golang.org/x/sync/errgroup" "os" "sync" "time" @@ -36,9 +38,8 @@ import ( nropv1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1" "github.com/openshift-kni/numaresources-operator/controllers" nropmcp "github.com/openshift-kni/numaresources-operator/internal/machineconfigpools" - "github.com/openshift-kni/numaresources-operator/pkg/status" - "github.com/openshift-kni/numaresources-operator/internal/wait" + "github.com/openshift-kni/numaresources-operator/pkg/status" e2eclient "github.com/openshift-kni/numaresources-operator/test/utils/clients" "github.com/openshift-kni/numaresources-operator/test/utils/configuration" @@ -86,6 +87,7 @@ func OverallDeployment() NroDeployment { unpause, err := e2epause.MachineConfigPoolsByNodeGroups(nroObj.Spec.NodeGroups) Expect(err).NotTo(HaveOccurred()) + var createKubelet bool if _, ok := os.LookupEnv("E2E_NROP_INSTALL_SKIP_KC"); ok { By("using cluster kubeletconfig (if any)") } else { @@ -93,6 +95,7 @@ func OverallDeployment() NroDeployment { err = e2eclient.Client.Create(context.TODO(), kcObj) Expect(err).NotTo(HaveOccurred()) deployedObj.KcObj = kcObj + createKubelet = true } By(fmt.Sprintf("creating the NRO object: %s", nroObj.Name)) @@ -106,9 +109,13 @@ func OverallDeployment() NroDeployment { Expect(err).NotTo(HaveOccurred()) deployedObj.NroObj = nroObj - By("waiting for MCP to get updated") - WaitForMCPUpdatedAfterNROCreated(nroObj) - + if createKubelet || annotations.IsCustomPolicyEnabled(nroObj.Annotations) { + By("waiting for MCP to get updated") + mcps, err := nropmcp.GetListByNodeGroupsV1(context.TODO(), e2eclient.Client, nroObj.Spec.NodeGroups) + Expect(err).NotTo(HaveOccurred()) + Expect(WaitForMCPsCondition(e2eclient.Client, context.TODO(), mcps, machineconfigv1.MachineConfigPoolUpdating)).To(Succeed()) + Expect(WaitForMCPsCondition(e2eclient.Client, context.TODO(), mcps, machineconfigv1.MachineConfigPoolUpdated)).To(Succeed()) + } return deployedObj } @@ -201,22 +208,6 @@ func WaitForMCPUpdatedAfterNRODeleted(nroObj *nropv1.NUMAResourcesOperator) { }).WithTimeout(configuration.MachineConfigPoolUpdateTimeout).WithPolling(configuration.MachineConfigPoolUpdateInterval).Should(BeTrue()) } -// isMachineConfigPoolsUpdated checks if all related to NUMAResourceOperator CR machines config pools have updated status -func isMachineConfigPoolsUpdated(nro *nropv1.NUMAResourcesOperator) (bool, error) { - mcps, err := nropmcp.GetListByNodeGroupsV1(context.TODO(), e2eclient.Client, nro.Spec.NodeGroups) - if err != nil { - return false, err - } - - for _, mcp := range mcps { - if !controllers.IsMachineConfigPoolUpdated(nro.Name, mcp) { - return false, nil - } - } - - return true, nil -} - // isMachineConfigPoolsUpdatedAfterDeletion checks if all related to NUMAResourceOperator CR machines config pools have updated status // after MachineConfig deletion func isMachineConfigPoolsUpdatedAfterDeletion(nro *nropv1.NUMAResourcesOperator) (bool, error) { @@ -234,25 +225,6 @@ func isMachineConfigPoolsUpdatedAfterDeletion(nro *nropv1.NUMAResourcesOperator) return true, nil } -func WaitForMCPUpdatedAfterNROCreated(nroObj *nropv1.NUMAResourcesOperator) { - GinkgoHelper() - - if configuration.Plat != platform.OpenShift { - // nothing to do - return - } - - Eventually(func() bool { - updated, err := isMachineConfigPoolsUpdated(nroObj) - if err != nil { - klog.Errorf("failed to information about machine config pools: %v", err) - return false - } - - return updated - }).WithTimeout(configuration.MachineConfigPoolUpdateTimeout).WithPolling(configuration.MachineConfigPoolUpdateInterval).Should(BeTrue()) -} - // Deploy a test NUMAResourcesScheduler and waits until its available // or a timeout happens (5 min right now). // @@ -301,3 +273,28 @@ func TeardownNROScheduler(nroSched *nropv1.NUMAResourcesScheduler, timeout time. Expect(err).ToNot(HaveOccurred(), "NROScheduler %q failed to be deleted", nroSched.Name) } } + +func WaitForMCPsCondition(cli client.Client, ctx context.Context, mcps []*machineconfigv1.MachineConfigPool, condition machineconfigv1.MachineConfigPoolConditionType) error { + var eg errgroup.Group + interval := configuration.MachineConfigPoolUpdateInterval + if condition == machineconfigv1.MachineConfigPoolUpdating { + // the transition from updated to updating to updated can be very fast sometimes. so if + // the status changed to updating and then to updated while on wait it will miss the updating + // status, and it will keep waiting and lastly fail the test. to avoid that decrease the interval + // to allow more often checks for the status + interval = 2 * time.Second + } + for _, mcp := range mcps { + klog.Infof("wait for mcp %q to meet condition %q", mcp.Name, condition) + mcp := mcp + eg.Go(func() error { + defer GinkgoRecover() + err := wait.With(cli). + Interval(interval). + Timeout(configuration.MachineConfigPoolUpdateTimeout). + ForMachineConfigPoolCondition(ctx, mcp, condition) + return err + }) + } + return eg.Wait() +} From 1327010d3967cd0f332baf28eefd894ccb6ab0be Mon Sep 17 00:00:00 2001 From: Talor Itzhak Date: Mon, 30 Sep 2024 23:37:55 +0300 Subject: [PATCH 09/10] ssc: update correct SELinux option set the SSC with the correct SELinux option, given we use custom policy or not. Signed-off-by: Talor Itzhak --- controllers/numaresourcesoperator_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/numaresourcesoperator_controller.go b/controllers/numaresourcesoperator_controller.go index 548e7106f..2b6655eb0 100644 --- a/controllers/numaresourcesoperator_controller.go +++ b/controllers/numaresourcesoperator_controller.go @@ -336,7 +336,7 @@ func (r *NUMAResourcesOperatorReconciler) syncMachineConfigs(ctx context.Context // so the MachineConfig which applies the custom policy is no longer necessary. // In case of operator upgrade from 4.1X → 4.18, it's necessary to remove the old MachineConfig, // unless an emergency annotation is provided which forces the operator to use custom policy - if annotations.IsCustomPolicyEnabled(instance.Annotations) { + if !annotations.IsCustomPolicyEnabled(instance.Annotations) { for _, objState := range objStates { if !objState.IsNotFoundError() { klog.V(4).InfoS("delete Machine Config", "MachineConfig", objState.Desired.GetName()) From 2823ed04958f85e9772834b2b8a043cc9a026625 Mon Sep 17 00:00:00 2001 From: Talor Itzhak Date: Tue, 8 Oct 2024 13:57:46 +0300 Subject: [PATCH 10/10] e2e: validate selinux context After RTE pod deployment, the test validates it's running with the correct SELinux context. Signed-off-by: Talor Itzhak --- test/e2e/install/install_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/e2e/install/install_test.go b/test/e2e/install/install_test.go index 2fd80a8db..3e11e2630 100644 --- a/test/e2e/install/install_test.go +++ b/test/e2e/install/install_test.go @@ -34,6 +34,7 @@ import ( "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/k8stopologyawareschedwg/deployer/pkg/assets/selinux" "github.com/k8stopologyawareschedwg/deployer/pkg/manifests/rte" nropv1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1" "github.com/openshift-kni/numaresources-operator/internal/api/annotations" @@ -132,6 +133,13 @@ var _ = Describe("[Install] continuousIntegration", func() { } return true }).WithTimeout(1*time.Minute).WithPolling(5*time.Second).Should(BeTrue(), "DaemonSet Status was not correct") + + By("checking DaemonSet pods are running with correct SELinux context") + ds, err := getDaemonSetByOwnerReference(updatedNROObj.UID) + Expect(err).NotTo(HaveOccurred()) + rteContainer, err := findContainerByName(*ds, containerNameRTE) + Expect(err).ToNot(HaveOccurred()) + Expect(rteContainer.SecurityContext.SELinuxOptions.Type).To(Equal(selinux.RTEContextType), "container %s is running with wrong selinux context", rteContainer.Name) }) }) })