diff --git a/Dockerfile b/Dockerfile index 1f14031e70..338d9c50b7 100644 --- a/Dockerfile +++ b/Dockerfile @@ -13,7 +13,7 @@ COPY --from=builder /go/src/github.com/openshift-kni/numaresources-operator/bin/ COPY --from=builder /go/src/github.com/openshift-kni/numaresources-operator/bin/buildinfo.json /usr/local/share RUN mkdir /etc/resource-topology-exporter/ && \ touch /etc/resource-topology-exporter/config.yaml -RUN microdnf install -y hwdata && \ +RUN microdnf install -y --disablerepo='rhel-*' hwdata && \ microdnf clean -y all USER 65532:65532 ENTRYPOINT ["/bin/numaresources-operator"] diff --git a/internal/controller/numaresourcesoperator_controller.go b/internal/controller/numaresourcesoperator_controller.go index 9acf764782..6a2ee36831 100644 --- a/internal/controller/numaresourcesoperator_controller.go +++ b/internal/controller/numaresourcesoperator_controller.go @@ -522,22 +522,32 @@ func (r *NUMAResourcesOperatorReconciler) syncNUMAResourcesOperatorResources(ctx dsPoolPairs := []poolDaemonSet{} // using a slice of poolDaemonSet instead of a map because Go maps assignment order is not consistent and non-deterministic - err = rteupdate.DaemonSetUserImageSettings(r.RTEManifests.Core.DaemonSet, instance.Spec.ExporterImage, r.Images.Preferred(), r.ImagePullPolicy) + // mutatedManifests are rendered manifests which their base derives from the existing resources + // found on the cluster. + // those manifests were mutated by the API server and other controllers running on the cluster, hence the name. + // this way it minimizes the diffs between the desired and existing state only to the fields we care about. + // in case the manifest/resource was not found on the cluster (should be created) + // it uses the local stored manifest. + mutatedManifests, err := rtestate.MutatedFromExisting(existing, r.RTEManifests, r.Namespace) if err != nil { return dsPoolPairs, err } - err = rteupdate.DaemonSetPauseContainerSettings(r.RTEManifests.Core.DaemonSet) + err = rteupdate.DaemonSetUserImageSettings(mutatedManifests.Core.DaemonSet, instance.Spec.ExporterImage, r.Images.Preferred(), r.ImagePullPolicy) if err != nil { return dsPoolPairs, err } - err = loglevel.UpdatePodSpec(&r.RTEManifests.Core.DaemonSet.Spec.Template.Spec, manifests.ContainerNameRTE, instance.Spec.LogLevel) + err = rteupdate.DaemonSetPauseContainerSettings(mutatedManifests.Core.DaemonSet) if err != nil { return dsPoolPairs, err } - rteupdate.SecurityContextConstraint(r.RTEManifests.Core.SecurityContextConstraint, true) // force to legacy context + err = loglevel.UpdatePodSpec(&mutatedManifests.Core.DaemonSet.Spec.Template.Spec, manifests.ContainerNameRTE, instance.Spec.LogLevel) + if err != nil { + return dsPoolPairs, err + } + rteupdate.SecurityContextConstraint(mutatedManifests.Core.SecurityContextConstraint, true) // force to legacy context // SCC v2 needs no updates existing = existing.WithManifestsUpdater(func(poolName string, gdm *rtestate.GeneratedDesiredManifest) error { @@ -549,7 +559,7 @@ func (r *NUMAResourcesOperatorReconciler) syncNUMAResourcesOperatorResources(ctx return nil }) - for _, objState := range existing.State(r.RTEManifests) { + for _, objState := range existing.State(mutatedManifests) { 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. diff --git a/pkg/apply/apply.go b/pkg/apply/apply.go index 33ce77725f..a2948420d9 100644 --- a/pkg/apply/apply.go +++ b/pkg/apply/apply.go @@ -81,6 +81,7 @@ func ApplyObject(ctx context.Context, cli k8sclient.Client, objState objectstate updated := false if !ok { klog.InfoS("updating", "object", objDesc) + recordApplyClientUpdate() if err := cli.Update(ctx, merged); err != nil { return nil, updated, fmt.Errorf("could not update object %s: %w", objDesc, err) } diff --git a/pkg/apply/metrics.go b/pkg/apply/metrics.go new file mode 100644 index 0000000000..1ef63f5f42 --- /dev/null +++ b/pkg/apply/metrics.go @@ -0,0 +1,42 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package apply + +import ( + "github.com/prometheus/client_golang/prometheus" + + crmetrics "sigs.k8s.io/controller-runtime/pkg/metrics" +) + +// MetricApplyClientUpdatesTotal is the Prometheus metric name for applyClientUpdatesTotal. +const MetricApplyClientUpdatesTotal = "numaresources_operator_apply_client_updates_total" + +var applyClientUpdatesTotal = prometheus.NewCounter( + prometheus.CounterOpts{ + Namespace: "numaresources_operator", + Name: "apply_client_updates_total", + Help: "Number of client Update calls issued from apply.ApplyObject. Incremented for every Update call.", + }, +) + +func init() { + crmetrics.Registry.MustRegister(applyClientUpdatesTotal) +} + +func recordApplyClientUpdate() { + applyClientUpdatesTotal.Inc() +} diff --git a/pkg/numaresourcesscheduler/objectstate/sched/sched.go b/pkg/numaresourcesscheduler/objectstate/sched/sched.go index 73204b5c00..8c532562f4 100644 --- a/pkg/numaresourcesscheduler/objectstate/sched/sched.go +++ b/pkg/numaresourcesscheduler/objectstate/sched/sched.go @@ -35,6 +35,7 @@ import ( "github.com/openshift-kni/numaresources-operator/pkg/objectstate" "github.com/openshift-kni/numaresources-operator/pkg/objectstate/compare" "github.com/openshift-kni/numaresources-operator/pkg/objectstate/merge" + "github.com/openshift-kni/numaresources-operator/pkg/objectupdate/volume" ) const ( @@ -224,14 +225,13 @@ func SchedulerNameFromObject(obj client.Object) (string, bool) { } func NewSchedConfigVolume(schedVolumeConfigName, configMapName string) corev1.Volume { - return corev1.Volume{ - Name: schedVolumeConfigName, - VolumeSource: corev1.VolumeSource{ - ConfigMap: &corev1.ConfigMapVolumeSource{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: configMapName, - }, - }, - }, - } + // Create a temporary pod spec and container to use the volume package + podSpec := &corev1.PodSpec{} + container := &corev1.Container{} + + // Use the volume package to create the ConfigMap volume + volume.AddConfigMap(podSpec, container, schedVolumeConfigName, "/tmp", configMapName, volume.DefaultMode, false, false) + + // Return just the volume part (we don't need the mount) + return podSpec.Volumes[0] } diff --git a/pkg/objectstate/api/api.go b/pkg/objectstate/api/api.go index 0d18efbcd8..23e0ccadee 100644 --- a/pkg/objectstate/api/api.go +++ b/pkg/objectstate/api/api.go @@ -37,11 +37,18 @@ type ExistingManifests struct { } func (em *ExistingManifests) State(mf apimanifests.Manifests) []objectstate.ObjectState { + desired := mf.Crd.DeepCopy() + // Use the existing object as a base when available so we preserve + // defaults applied by the API server (e.g. Conversion strategy) + o := objectstate.ObjectState{Error: em.CrdError} + if !o.IsNotFoundError() && em.Existing.Crd != nil { + desired.Spec.Conversion = em.Existing.Crd.Spec.Conversion + } return []objectstate.ObjectState{ { Existing: em.Existing.Crd, Error: em.CrdError, - Desired: mf.Crd.DeepCopy(), + Desired: desired, Compare: compare.Object, Merge: merge.ObjectForUpdate, }, diff --git a/pkg/objectstate/rte/rte.go b/pkg/objectstate/rte/rte.go index 77285041a3..d900486c6a 100644 --- a/pkg/objectstate/rte/rte.go +++ b/pkg/objectstate/rte/rte.go @@ -18,6 +18,7 @@ package rte import ( "context" + "time" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -34,6 +35,7 @@ import ( "github.com/k8stopologyawareschedwg/deployer/pkg/deployer/platform" rtemanifests "github.com/k8stopologyawareschedwg/deployer/pkg/manifests/rte" k8swgrteupdate "github.com/k8stopologyawareschedwg/deployer/pkg/objectupdate/rte" + "github.com/k8stopologyawareschedwg/deployer/pkg/options" nropv1 "github.com/openshift-kni/numaresources-operator/api/v1" nodegroupv1 "github.com/openshift-kni/numaresources-operator/api/v1/helper/nodegroup" @@ -169,6 +171,101 @@ func SkipManifestUpdate(mcpName string, gdm *GeneratedDesiredManifest) error { return nil } +func MutatedFromExisting(existingMF *ExistingManifests, defaultManifests Manifests, namespace string) (Manifests, error) { + mf := Manifests{ + Core: existingMF.existing.Core.Clone(), + Metrics: existingMF.existing.Metrics.Clone(), + } + + o := objectstate.ObjectState{Error: existingMF.errs.Core.ClusterRole} + if o.IsNotFoundError() { + mf.Core.ClusterRole = defaultManifests.Core.ClusterRole.DeepCopy() + } + + o = objectstate.ObjectState{Error: existingMF.errs.Core.ClusterRoleBinding} + if o.IsNotFoundError() { + mf.Core.ClusterRoleBinding = defaultManifests.Core.ClusterRoleBinding.DeepCopy() + } + + o = objectstate.ObjectState{Error: existingMF.errs.Core.Role} + if o.IsNotFoundError() { + mf.Core.Role = defaultManifests.Core.Role.DeepCopy() + } + + o = objectstate.ObjectState{Error: existingMF.errs.Core.RoleBinding} + if o.IsNotFoundError() { + mf.Core.RoleBinding = defaultManifests.Core.RoleBinding.DeepCopy() + } + + o = objectstate.ObjectState{Error: existingMF.errs.Core.ServiceAccount} + if o.IsNotFoundError() { + mf.Core.ServiceAccount = defaultManifests.Core.ServiceAccount.DeepCopy() + } + + if defaultManifests.Core.SecurityContextConstraint != nil { + o = objectstate.ObjectState{Error: existingMF.errs.Core.SCC} + if o.IsNotFoundError() { + mf.Core.SecurityContextConstraint = defaultManifests.Core.SecurityContextConstraint.DeepCopy() + } + } + + if defaultManifests.Core.SecurityContextConstraintV2 != nil { + o = objectstate.ObjectState{Error: existingMF.errs.Core.SCCv2} + if o.IsNotFoundError() { + mf.Core.SecurityContextConstraintV2 = defaultManifests.Core.SecurityContextConstraintV2.DeepCopy() + } + } + + o = objectstate.ObjectState{Error: existingMF.errs.Core.APIServerNetworkPolicy} + if o.IsNotFoundError() { + mf.Core.APIServerNetworkPolicy = defaultManifests.Core.APIServerNetworkPolicy.DeepCopy() + } + + o = objectstate.ObjectState{Error: existingMF.errs.Core.MetricsServerNetworkPolicy} + if o.IsNotFoundError() { + mf.Core.MetricsServerNetworkPolicy = defaultManifests.Core.MetricsServerNetworkPolicy.DeepCopy() + } + + o = objectstate.ObjectState{Error: existingMF.errs.Core.DefaultNetworkPolicy} + if o.IsNotFoundError() { + mf.Core.DefaultNetworkPolicy = defaultManifests.Core.DefaultNetworkPolicy.DeepCopy() + } + + // there are multiple resources of DaemonSets + // (one per nodeGroup), so we should use a default manifest + existing/mutated spec, + // and the rest will be updated later + mf.Core.DaemonSet = defaultManifests.Core.DaemonSet.DeepCopy() + for _, ds := range existingMF.daemonSets { + if ds.daemonSetError == nil { + // use the spec from one of the existing so we won't end up with + // diffs that derives from default values applied by the API server + mf.Core.DaemonSet.Spec = *ds.daemonSet.Spec.DeepCopy() + } + } + // Clear volumes and volume mounts before rendering to avoid duplicates + // The Render() function will add them back based on the configuration + clearVolumesAndVolumeMounts(mf.Core.DaemonSet) + + o = objectstate.ObjectState{Error: existingMF.errs.Metrics.Service} + if o.IsNotFoundError() { + mf.Metrics.Service = defaultManifests.Metrics.Service.DeepCopy() + } + + var err error + mf.Core, err = mf.Core.Render(options.UpdaterDaemon{ + Namespace: namespace, + DaemonSet: options.DaemonSet{ + Verbose: 2, + NotificationEnable: true, + UpdateInterval: 10 * time.Second, + }, + }) + if err != nil { + return mf, err + } + return mf, err +} + func (em *ExistingManifests) State(mf Manifests) []objectstate.ObjectState { ret := []objectstate.ObjectState{ { @@ -372,3 +469,11 @@ func getObject(ctx context.Context, cli client.Client, key client.ObjectKey, obj *err = cli.Get(ctx, key, obj) return *err == nil } + +func clearVolumesAndVolumeMounts(ds *appsv1.DaemonSet) { + podSpec := &ds.Spec.Template.Spec + podSpec.Volumes = nil + for i := range podSpec.Containers { + podSpec.Containers[i].VolumeMounts = nil + } +} diff --git a/pkg/objectupdate/rte/rte.go b/pkg/objectupdate/rte/rte.go index 8a2a305988..5cfae0921b 100644 --- a/pkg/objectupdate/rte/rte.go +++ b/pkg/objectupdate/rte/rte.go @@ -21,7 +21,6 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" - "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/klog/v2" @@ -38,6 +37,7 @@ import ( "github.com/openshift-kni/numaresources-operator/pkg/hash" "github.com/openshift-kni/numaresources-operator/pkg/objectupdate/envvar" objtls "github.com/openshift-kni/numaresources-operator/pkg/objectupdate/tls" + "github.com/openshift-kni/numaresources-operator/pkg/objectupdate/volume" ) // these should be provided by a deployer API @@ -47,6 +47,8 @@ const ( pfpStatusMountName = "run-pfpstatus" pfpStatusDir = "/run/pfpstatus" + + rteConfigVolumeName = "rte-config-volume" ) func DaemonSetUserImageSettings(ds *appsv1.DaemonSet, userImageSpec, builtinImageSpec string, builtinPullPolicy corev1.PullPolicy) error { @@ -225,7 +227,9 @@ func DaemonSetArgs(ds *appsv1.DaemonSet, conf nropv1.NodeGroupConfig, metricsTLS // TODO: these don't really belong here, but OTOH adding the status file without having set // the volume doesn't work either. We need a deeper refactoring in this area. - AddVolumeMountMemory(podSpec, cnt, pfpStatusMountName, pfpStatusDir, 8*_MiB) + if err := AddVolumeMountMemory(podSpec, cnt, pfpStatusMountName, pfpStatusDir, 8*_MiB); err != nil { + return fmt.Errorf("failed to add volume mount memory: %w", err) + } envvar.SetForContainer(cnt, envvar.PFPStatusDump, envvar.PFPStatusDirDefault) } else { // TODO: ditto @@ -239,10 +243,12 @@ func DaemonSetArgs(ds *appsv1.DaemonSet, conf nropv1.NodeGroupConfig, metricsTLS } func DaemonSetTolerations(ds *appsv1.DaemonSet, userTolerations []corev1.Toleration) { + podSpec := &ds.Spec.Template.Spec // shortcut + // cleanup undesired toleration + podSpec.Tolerations = nil if len(userTolerations) == 0 { return } - podSpec := &ds.Spec.Template.Spec // shortcut podSpec.Tolerations = nropv1.CloneTolerations(userTolerations) } @@ -251,10 +257,24 @@ func ContainerConfig(ds *appsv1.DaemonSet, name string) error { if cnt == nil { return fmt.Errorf("cannot find container data for %q", MainContainerName) } - k8swgrteupdate.ContainerConfig(&ds.Spec.Template.Spec, cnt, name) + podSpec := &ds.Spec.Template.Spec + if !hasVolume(podSpec, rteConfigVolumeName) { + k8swgrteupdate.ContainerConfig(podSpec, cnt, name) + // when configMap volume is rendered, it's default mode is not set, so we need to set it. + setConfigMapVolumeDefaultMode(podSpec, rteConfigVolumeName) + } return nil } +func setConfigMapVolumeDefaultMode(podSpec *corev1.PodSpec, volumeName string) { + for i := range podSpec.Volumes { + vol := &podSpec.Volumes[i] + if vol.Name == volumeName && vol.ConfigMap != nil && vol.ConfigMap.DefaultMode == nil { + var defaultMode int32 = corev1.ConfigMapVolumeSourceDefaultMode + vol.ConfigMap.DefaultMode = &defaultMode + } + } +} func AllContainersTerminationMessagePolicy(ds *appsv1.DaemonSet) { for idx := range ds.Spec.Template.Spec.Containers { cnt := &ds.Spec.Template.Spec.Containers[idx] @@ -264,37 +284,53 @@ func AllContainersTerminationMessagePolicy(ds *appsv1.DaemonSet) { } } -func AddVolumeMountMemory(podSpec *corev1.PodSpec, cnt *corev1.Container, mountName, dirName string, sizeMiB int64) { - cnt.VolumeMounts = append(cnt.VolumeMounts, - corev1.VolumeMount{ - Name: mountName, - MountPath: dirName, - }, - corev1.VolumeMount{ - MountPath: "/etc/secrets/rte/", - Name: "rte-metrics-service-cert", - ReadOnly: true, - }, - ) - podSpec.Volumes = append(podSpec.Volumes, - corev1.Volume{ - Name: mountName, - VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{ - Medium: corev1.StorageMediumMemory, - SizeLimit: resource.NewQuantity(sizeMiB, resource.BinarySI), - }, - }, - }, - corev1.Volume{ - Name: "rte-metrics-service-cert", - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: "rte-metrics-service-cert", - }, - }, - }, - ) +// hasVolumeMount checks if a container already has a volume mount with the given name +func hasVolumeMount(cnt *corev1.Container, volumeName string) bool { + for _, vm := range cnt.VolumeMounts { + if vm.Name == volumeName { + return true + } + } + return false +} + +// hasVolume checks if a pod spec already has a volume with the given name +func hasVolume(podSpec *corev1.PodSpec, volumeName string) bool { + for _, v := range podSpec.Volumes { + if v.Name == volumeName { + return true + } + } + return false +} + +func AddVolumeMountMemory(podSpec *corev1.PodSpec, cnt *corev1.Container, mountName, dirName string, sizeMiB int64) error { + // Add the requested memory volume mount + if !hasVolumeMount(cnt, mountName) && !hasVolume(podSpec, mountName) { + volume.AddMemoryVolume(podSpec, cnt, mountName, dirName, sizeMiB) + } + + // Add the metrics certificate volume mount only if it doesn't already exist + metricsVolumeName := "rte-metrics-service-cert" + if !hasVolumeMount(cnt, metricsVolumeName) && !hasVolume(podSpec, metricsVolumeName) { + volume.AddSecret(podSpec, cnt, metricsVolumeName, "/etc/secrets/rte/", metricsVolumeName, volume.DefaultMode, false, true) + } + + // Add host-sys volume + rteSysVolumeName := "host-sys" + if !hasVolume(podSpec, rteSysVolumeName) && !hasVolumeMount(cnt, rteSysVolumeName) { + hostPathType := corev1.HostPathDirectory + volume.AddHostPath(podSpec, cnt, rteSysVolumeName, "/host-sys", "/sys", &hostPathType, true) + } + + // Add host-podresources volume + hostPodresourcesName := "host-podresources" + if !hasVolume(podSpec, hostPodresourcesName) && !hasVolumeMount(cnt, hostPodresourcesName) { + hostPathType := corev1.HostPathDirectory + volume.AddHostPath(podSpec, cnt, hostPodresourcesName, "/host-podresources", "/var/lib/kubelet/pod-resources", &hostPathType, false) + } + + return nil } func SecurityContextConstraint(scc *securityv1.SecurityContextConstraints, legacyRTEContext bool) { diff --git a/pkg/objectupdate/rte/rte_test.go b/pkg/objectupdate/rte/rte_test.go index 282f264816..447461adb4 100644 --- a/pkg/objectupdate/rte/rte_test.go +++ b/pkg/objectupdate/rte/rte_test.go @@ -195,7 +195,7 @@ func TestUpdateDaemonSetTolerations(t *testing.T) { { name: "defaults", conf: nropv1.NodeGroupConfig{}, - expectedTols: []corev1.Toleration{}, + expectedTols: nil, }, { name: "add tolerations", diff --git a/pkg/objectupdate/volume/volume.go b/pkg/objectupdate/volume/volume.go new file mode 100644 index 0000000000..de629efe5d --- /dev/null +++ b/pkg/objectupdate/volume/volume.go @@ -0,0 +1,176 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * Copyright 2025 Red Hat, Inc. + */ + +package volume + +import ( + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/klog/v2" +) + +// VolumeType represents the type of volume to create +type VolumeType string + +const ( + TypeConfigMap VolumeType = "configmap" + TypeSecret VolumeType = "secret" + TypeHostPath VolumeType = "hostpath" + TypeEmptyDir VolumeType = "emptydir" +) + +const ( + DefaultMode = 420 +) + +// Options holds the configuration for creating volumes and volume mounts +type Options struct { + // Common fields + VolumeName string + MountPath string + ReadOnly bool + SubPath string + Type VolumeType + + // ConfigMap/Secret specific + ResourceName string + DefaultMode int32 + Optional bool + + // HostPath specific + HostPath string + HostPathType *corev1.HostPathType + + // EmptyDir specific + SizeLimit *resource.Quantity + Medium corev1.StorageMedium +} + +// Add adds a volume to the pod spec and a corresponding volume mount to the container +func Add(podSpec *corev1.PodSpec, container *corev1.Container, opts Options) { + // Create volume based on type + volume := corev1.Volume{ + Name: opts.VolumeName, + } + + switch opts.Type { + case TypeConfigMap: + volume.VolumeSource = corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: opts.ResourceName, + }, + Optional: &opts.Optional, + DefaultMode: &opts.DefaultMode, + }, + } + case TypeSecret: + volume.VolumeSource = corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: opts.ResourceName, + Optional: &opts.Optional, + DefaultMode: &opts.DefaultMode, + }, + } + case TypeHostPath: + volume.VolumeSource = corev1.VolumeSource{ + HostPath: &corev1.HostPathVolumeSource{ + Path: opts.HostPath, + Type: opts.HostPathType, + }, + } + case TypeEmptyDir: + emptyDirSource := &corev1.EmptyDirVolumeSource{} + if opts.Medium != "" { + emptyDirSource.Medium = opts.Medium + } + if opts.SizeLimit != nil { + emptyDirSource.SizeLimit = opts.SizeLimit + } + volume.VolumeSource = corev1.VolumeSource{ + EmptyDir: emptyDirSource, + } + default: + klog.InfoS("unsupported volume type, skipping", "type", opts.Type) + return + } + podSpec.Volumes = append(podSpec.Volumes, volume) + + // Add volume mount to container + volumeMount := corev1.VolumeMount{ + Name: opts.VolumeName, + MountPath: opts.MountPath, + ReadOnly: opts.ReadOnly, + } + if opts.SubPath != "" { + volumeMount.SubPath = opts.SubPath + } + container.VolumeMounts = append(container.VolumeMounts, volumeMount) +} + +// AddHostPath adds a HostPath volume with the specified path and type +func AddHostPath(podSpec *corev1.PodSpec, container *corev1.Container, volumeName, mountPath, hostPath string, hostPathType *corev1.HostPathType, readOnly bool) { + Add(podSpec, container, Options{ + VolumeName: volumeName, + MountPath: mountPath, + ReadOnly: readOnly, + Type: TypeHostPath, + HostPath: hostPath, + HostPathType: hostPathType, + }) +} + +// AddEmptyDir adds an EmptyDir volume with the specified options +func AddEmptyDir(podSpec *corev1.PodSpec, container *corev1.Container, volumeName, mountPath string, medium corev1.StorageMedium, sizeLimit *resource.Quantity) { + Add(podSpec, container, Options{ + VolumeName: volumeName, + MountPath: mountPath, + Type: TypeEmptyDir, + Medium: medium, + SizeLimit: sizeLimit, + }) +} + +// AddSecret adds a Secret volume with the specified options +func AddSecret(podSpec *corev1.PodSpec, container *corev1.Container, volumeName, mountPath, secretName string, defaultMode int32, optional bool, readOnly bool) { + Add(podSpec, container, Options{ + VolumeName: volumeName, + MountPath: mountPath, + ReadOnly: readOnly, + Type: TypeSecret, + ResourceName: secretName, + DefaultMode: defaultMode, + Optional: optional, + }) +} + +// AddConfigMap adds a ConfigMap volume with the specified options +func AddConfigMap(podSpec *corev1.PodSpec, container *corev1.Container, volumeName, mountPath, configMapName string, defaultMode int32, optional bool, readOnly bool) { + Add(podSpec, container, Options{ + VolumeName: volumeName, + MountPath: mountPath, + ReadOnly: readOnly, + Type: TypeConfigMap, + ResourceName: configMapName, + DefaultMode: defaultMode, + Optional: optional, + }) +} + +// AddMemoryVolume adds an EmptyDir volume with memory storage +func AddMemoryVolume(podSpec *corev1.PodSpec, container *corev1.Container, volumeName, mountPath string, sizeMiB int64) { + AddEmptyDir(podSpec, container, volumeName, mountPath, corev1.StorageMediumMemory, resource.NewQuantity(sizeMiB, resource.BinarySI)) +} diff --git a/pkg/objectupdate/volume/volume_test.go b/pkg/objectupdate/volume/volume_test.go new file mode 100644 index 0000000000..60127ec761 --- /dev/null +++ b/pkg/objectupdate/volume/volume_test.go @@ -0,0 +1,398 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * Copyright 2025 Red Hat, Inc. + */ + +package volume + +import ( + "testing" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" +) + +func TestAdd_ConfigMap(t *testing.T) { + podSpec := &corev1.PodSpec{} + container := &corev1.Container{} + + opts := Options{ + VolumeName: "test-config", + MountPath: "/etc/config", + ReadOnly: true, + Type: TypeConfigMap, + ResourceName: "my-configmap", + DefaultMode: DefaultMode, + Optional: true, + } + + Add(podSpec, container, opts) + + // Verify volume mount + if len(container.VolumeMounts) != 1 { + t.Fatalf("expected 1 volume mount, got %d", len(container.VolumeMounts)) + } + + vm := container.VolumeMounts[0] + if vm.Name != "test-config" { + t.Errorf("expected volume mount name 'test-config', got '%s'", vm.Name) + } + if vm.MountPath != "/etc/config" { + t.Errorf("expected mount path '/etc/config', got '%s'", vm.MountPath) + } + if !vm.ReadOnly { + t.Errorf("expected readonly to be true") + } + + // Verify volume + if len(podSpec.Volumes) != 1 { + t.Fatalf("expected 1 volume, got %d", len(podSpec.Volumes)) + } + + vol := podSpec.Volumes[0] + if vol.Name != "test-config" { + t.Errorf("expected volume name 'test-config', got '%s'", vol.Name) + } + if vol.ConfigMap == nil { + t.Fatal("expected ConfigMap volume source") + } + if vol.ConfigMap.Name != "my-configmap" { + t.Errorf("expected configmap name 'my-configmap', got '%s'", vol.ConfigMap.Name) + } + if vol.ConfigMap.Optional == nil || !*vol.ConfigMap.Optional { + t.Errorf("expected optional to be true") + } + if vol.ConfigMap.DefaultMode == nil || *vol.ConfigMap.DefaultMode != DefaultMode { + t.Errorf("expected default mode 420, got %v", vol.ConfigMap.DefaultMode) + } +} + +func TestAdd_Secret(t *testing.T) { + podSpec := &corev1.PodSpec{} + container := &corev1.Container{} + + opts := Options{ + VolumeName: "test-secret", + MountPath: "/etc/secrets", + ReadOnly: true, + Type: TypeSecret, + ResourceName: "my-secret", + DefaultMode: 400, + Optional: false, + } + + Add(podSpec, container, opts) + + // Verify volume + vol := podSpec.Volumes[0] + if vol.Secret == nil { + t.Fatal("expected Secret volume source") + } + if vol.Secret.SecretName != "my-secret" { + t.Errorf("expected secret name 'my-secret', got '%s'", vol.Secret.SecretName) + } + if vol.Secret.Optional == nil || *vol.Secret.Optional { + t.Errorf("expected optional to be false") + } + if vol.Secret.DefaultMode == nil || *vol.Secret.DefaultMode != 400 { + t.Errorf("expected default mode 400, got %v", vol.Secret.DefaultMode) + } +} + +func TestAdd_HostPath(t *testing.T) { + podSpec := &corev1.PodSpec{} + container := &corev1.Container{} + hostPathType := corev1.HostPathDirectory + + opts := Options{ + VolumeName: "test-hostpath", + MountPath: "/host/data", + ReadOnly: true, + Type: TypeHostPath, + HostPath: "/var/lib/data", + HostPathType: &hostPathType, + } + + Add(podSpec, container, opts) + + // Verify volume + vol := podSpec.Volumes[0] + if vol.HostPath == nil { + t.Fatal("expected HostPath volume source") + } + if vol.HostPath.Path != "/var/lib/data" { + t.Errorf("expected host path '/var/lib/data', got '%s'", vol.HostPath.Path) + } + if vol.HostPath.Type == nil || *vol.HostPath.Type != corev1.HostPathDirectory { + t.Errorf("expected host path type Directory, got %v", vol.HostPath.Type) + } +} + +func TestAdd_EmptyDir(t *testing.T) { + podSpec := &corev1.PodSpec{} + container := &corev1.Container{} + sizeLimit := resource.MustParse("1Gi") + + opts := Options{ + VolumeName: "test-emptydir", + MountPath: "/tmp", + Type: TypeEmptyDir, + Medium: corev1.StorageMediumMemory, + SizeLimit: &sizeLimit, + } + + Add(podSpec, container, opts) + + // Verify volume + vol := podSpec.Volumes[0] + if vol.EmptyDir == nil { + t.Fatal("expected EmptyDir volume source") + } + if vol.EmptyDir.Medium != corev1.StorageMediumMemory { + t.Errorf("expected medium Memory, got '%s'", vol.EmptyDir.Medium) + } + if vol.EmptyDir.SizeLimit == nil || !vol.EmptyDir.SizeLimit.Equal(sizeLimit) { + t.Errorf("expected size limit 1Gi, got %v", vol.EmptyDir.SizeLimit) + } +} + +func TestAdd_EmptyDirNoOptions(t *testing.T) { + podSpec := &corev1.PodSpec{} + container := &corev1.Container{} + + opts := Options{ + VolumeName: "test-emptydir", + MountPath: "/tmp", + Type: TypeEmptyDir, + } + + Add(podSpec, container, opts) + + // Verify volume + vol := podSpec.Volumes[0] + if vol.EmptyDir == nil { + t.Fatal("expected EmptyDir volume source") + } + if vol.EmptyDir.Medium != "" { + t.Errorf("expected no medium, got '%s'", vol.EmptyDir.Medium) + } + if vol.EmptyDir.SizeLimit != nil { + t.Errorf("expected no size limit, got %v", vol.EmptyDir.SizeLimit) + } +} + +func TestAdd_WithSubPath(t *testing.T) { + podSpec := &corev1.PodSpec{} + container := &corev1.Container{} + + opts := Options{ + VolumeName: "test-config", + MountPath: "/etc/config", + SubPath: "config.yaml", + Type: TypeConfigMap, + ResourceName: "my-configmap", + DefaultMode: DefaultMode, + Optional: true, + } + + Add(podSpec, container, opts) + + // Verify volume mount has subpath + vm := container.VolumeMounts[0] + if vm.SubPath != "config.yaml" { + t.Errorf("expected subpath 'config.yaml', got '%s'", vm.SubPath) + } +} + +func TestAdd_UnsupportedType(t *testing.T) { + podSpec := &corev1.PodSpec{} + container := &corev1.Container{} + + opts := Options{ + VolumeName: "test-volume", + MountPath: "/test", + Type: VolumeType("unsupported"), + } + + Add(podSpec, container, opts) +} + +func TestAddHostPath(t *testing.T) { + podSpec := &corev1.PodSpec{} + container := &corev1.Container{} + hostPathType := corev1.HostPathDirectory + + AddHostPath(podSpec, container, "test-host", "/host/test", "/test", &hostPathType, true) + + // Verify volume mount + vm := container.VolumeMounts[0] + if vm.Name != "test-host" { + t.Errorf("expected volume mount name 'test-host', got '%s'", vm.Name) + } + if vm.MountPath != "/host/test" { + t.Errorf("expected mount path '/host/test', got '%s'", vm.MountPath) + } + if !vm.ReadOnly { + t.Errorf("expected readonly to be true") + } + + // Verify volume + vol := podSpec.Volumes[0] + if vol.HostPath.Path != "/test" { + t.Errorf("expected host path '/test', got '%s'", vol.HostPath.Path) + } +} + +func TestAddEmptyDir(t *testing.T) { + podSpec := &corev1.PodSpec{} + container := &corev1.Container{} + sizeLimit := resource.MustParse("512Mi") + + AddEmptyDir(podSpec, container, "test-empty", "/tmp", corev1.StorageMediumDefault, &sizeLimit) + + // Verify volume + vol := podSpec.Volumes[0] + if vol.EmptyDir.Medium != corev1.StorageMediumDefault { + t.Errorf("expected medium Default, got '%s'", vol.EmptyDir.Medium) + } + if !vol.EmptyDir.SizeLimit.Equal(sizeLimit) { + t.Errorf("expected size limit 512Mi, got %v", vol.EmptyDir.SizeLimit) + } +} + +func TestAddSecret(t *testing.T) { + podSpec := &corev1.PodSpec{} + container := &corev1.Container{} + + AddSecret(podSpec, container, "test-secret", "/secrets", "my-secret", 600, true, true) + + // Verify volume mount + vm := container.VolumeMounts[0] + if !vm.ReadOnly { + t.Errorf("expected readonly to be true") + } + + // Verify volume + vol := podSpec.Volumes[0] + if vol.Secret.SecretName != "my-secret" { + t.Errorf("expected secret name 'my-secret', got '%s'", vol.Secret.SecretName) + } + if *vol.Secret.DefaultMode != 600 { + t.Errorf("expected default mode 600, got %d", *vol.Secret.DefaultMode) + } + if !*vol.Secret.Optional { + t.Errorf("expected optional to be true") + } +} + +func TestAddConfigMap(t *testing.T) { + podSpec := &corev1.PodSpec{} + container := &corev1.Container{} + + AddConfigMap(podSpec, container, "test-config", "/config", "my-config", 644, false, false) + + // Verify volume mount + vm := container.VolumeMounts[0] + if vm.ReadOnly { + t.Errorf("expected readonly to be false") + } + + // Verify volume + vol := podSpec.Volumes[0] + if vol.ConfigMap.Name != "my-config" { + t.Errorf("expected configmap name 'my-config', got '%s'", vol.ConfigMap.Name) + } + if *vol.ConfigMap.DefaultMode != 644 { + t.Errorf("expected default mode 644, got %d", *vol.ConfigMap.DefaultMode) + } + if *vol.ConfigMap.Optional { + t.Errorf("expected optional to be false") + } +} + +func TestAddMemoryVolume(t *testing.T) { + podSpec := &corev1.PodSpec{} + container := &corev1.Container{} + + AddMemoryVolume(podSpec, container, "test-memory", "/memory", 256) + + // Verify volume + vol := podSpec.Volumes[0] + if vol.EmptyDir == nil { + t.Fatal("expected EmptyDir volume source") + } + if vol.EmptyDir.Medium != corev1.StorageMediumMemory { + t.Errorf("expected medium Memory, got '%s'", vol.EmptyDir.Medium) + } + + expectedSize := resource.NewQuantity(256, resource.BinarySI) + if !vol.EmptyDir.SizeLimit.Equal(*expectedSize) { + t.Errorf("expected size limit 256 bytes, got %v", vol.EmptyDir.SizeLimit) + } +} + +func TestMultipleVolumes(t *testing.T) { + podSpec := &corev1.PodSpec{} + container := &corev1.Container{} + + // Add multiple volumes + AddConfigMap(podSpec, container, "config1", "/config1", "cm1", DefaultMode, true, false) + + AddSecret(podSpec, container, "secret1", "/secret1", "s1", 400, false, true) + + hostPathType := corev1.HostPathDirectory + AddHostPath(podSpec, container, "host1", "/host1", "/var/data", &hostPathType, true) + + // Verify we have 3 volumes and 3 volume mounts + if len(podSpec.Volumes) != 3 { + t.Errorf("expected 3 volumes, got %d", len(podSpec.Volumes)) + } + if len(container.VolumeMounts) != 3 { + t.Errorf("expected 3 volume mounts, got %d", len(container.VolumeMounts)) + } + + // Verify volume names + volumeNames := make(map[string]bool) + for _, vol := range podSpec.Volumes { + volumeNames[vol.Name] = true + } + + expectedNames := []string{"config1", "secret1", "host1"} + for _, name := range expectedNames { + if !volumeNames[name] { + t.Errorf("expected volume '%s' not found", name) + } + } +} + +func TestVolumeTypes(t *testing.T) { + tests := []struct { + name string + vt VolumeType + str string + }{ + {"ConfigMap", TypeConfigMap, "configmap"}, + {"Secret", TypeSecret, "secret"}, + {"HostPath", TypeHostPath, "hostpath"}, + {"EmptyDir", TypeEmptyDir, "emptydir"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if string(tt.vt) != tt.str { + t.Errorf("expected volume type '%s', got '%s'", tt.str, string(tt.vt)) + } + }) + } +} diff --git a/test/e2e/serial/tests/reconcile_noop.go b/test/e2e/serial/tests/reconcile_noop.go new file mode 100644 index 0000000000..cd0a7b3eaa --- /dev/null +++ b/test/e2e/serial/tests/reconcile_noop.go @@ -0,0 +1,149 @@ +/* + * Copyright 2026 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package tests + +import ( + "context" + "fmt" + "net" + "regexp" + "strconv" + "strings" + "time" + + corev1 "k8s.io/api/core/v1" + "k8s.io/klog/v2" + + "sigs.k8s.io/controller-runtime/pkg/client" + + nropv1 "github.com/openshift-kni/numaresources-operator/api/v1" + "github.com/openshift-kni/numaresources-operator/internal/remoteexec" + "github.com/openshift-kni/numaresources-operator/pkg/apply" + "github.com/openshift-kni/numaresources-operator/test/e2e/label" + e2eclient "github.com/openshift-kni/numaresources-operator/test/internal/clients" + "github.com/openshift-kni/numaresources-operator/test/internal/deploy" + "github.com/openshift-kni/numaresources-operator/test/internal/objects" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +const ( + e2eNoopReconcileAnnotation = "numaresources.openshift.io/e2e-noop-reconcile" + managerMetricsPort = "8080" + managerMetricsAddr = "127.0.0.1" +) + +// parseApplyClientUpdatesScalar returns the value of numaresources_operator_apply_client_updates_total. +func parseApplyClientUpdatesScalar(metricsText string) (float64, error) { + metricsText = strings.ReplaceAll(metricsText, "\r", "") + name := regexp.QuoteMeta(apply.MetricApplyClientUpdatesTotal) + // Line-anchored metric sample: NAME optional{label="pairs"} WS VALUE + // (?m)^ start of a line (multiline) + // (?:\{[^}]*\})? optional label set for this counter (empty or any labels) + // \s+ whitespace before the numeric sample + // ([0-9.eE+-]+) counter value, including scientific notation + re := regexp.MustCompile(`(?m)^` + name + `(?:\{[^}]*\})?\s+([0-9.eE+-]+)`) + m := re.FindStringSubmatch(metricsText) + if len(m) < 2 { + return 0, fmt.Errorf("metric %s not found in metrics output", apply.MetricApplyClientUpdatesTotal) + } + return strconv.ParseFloat(m[1], 64) +} + +func fetchManagerMetricsFromPod(ctx context.Context, pod *corev1.Pod) string { + GinkgoHelper() + endpoint := net.JoinHostPort(managerMetricsAddr, managerMetricsPort) + key := client.ObjectKeyFromObject(pod) + cmd := []string{ + "/bin/curl", "-k", + fmt.Sprintf("https://%s/metrics", endpoint), + } + klog.V(2).InfoS("executing command", "args", cmd, "pod", key.String()) + stdout, stderr, err := remoteexec.CommandOnPod(ctx, e2eclient.K8sClient, pod, cmd...) + Expect(err).ToNot(HaveOccurred(), "exec curl on pod=%q: %v; stderr=%q", key.String(), err, stderr) + Expect(stdout).NotTo(BeEmpty()) + return string(stdout) +} + +var _ = Describe("[serial] numaresources reconcile no-op updates", Serial, Label(label.Tier2, "feature:nonreg"), func() { + var nropKey client.ObjectKey + + BeforeEach(func() { + Expect(e2eclient.ClientsEnabled).To(BeTrue(), "failed to create kubernetes client") + nropKey = objects.NROObjectKey() + }) + + It("should not call apply Update() for operands when only NUMAResourcesOperator metadata changes", func(ctx context.Context) { + nro := &nropv1.NUMAResourcesOperator{} + Expect(e2eclient.Client.Get(ctx, nropKey, nro)).To(Succeed(), "get NUMAResourcesOperator") + + if len(nro.Status.DaemonSets) == 0 { + Skip("no RTE DaemonSets in status yet") + } + + managerPod, err := deploy.FindNUMAResourcesOperatorPod(ctx, e2eclient.Client, nro) + Expect(err).ToNot(HaveOccurred()) + + beforeText := fetchManagerMetricsFromPod(ctx, managerPod) + beforeTotal, err := parseApplyClientUpdatesScalar(beforeText) + Expect(err).ToNot(HaveOccurred()) + + baseForPatch := nro.DeepCopy() + if nro.Annotations == nil { + nro.Annotations = map[string]string{} + } + nro.Annotations[e2eNoopReconcileAnnotation] = fmt.Sprintf("%d", time.Now().UnixNano()) + Expect(e2eclient.Client.Patch(ctx, nro, client.MergeFrom(baseForPatch))).To(Succeed(), "patch NRO annotation") + + defer func() { + cleanup := &nropv1.NUMAResourcesOperator{} + if err := e2eclient.Client.Get(ctx, nropKey, cleanup); err != nil { + return + } + base := cleanup.DeepCopy() + if cleanup.Annotations == nil { + return + } + delete(cleanup.Annotations, e2eNoopReconcileAnnotation) + if len(cleanup.Annotations) == 0 { + cleanup.Annotations = nil + } + _ = e2eclient.Client.Patch(ctx, cleanup, client.MergeFrom(base)) + }() + + Consistently(func() error { + afterText := fetchManagerMetricsFromPod(ctx, managerPod) + afterTotal, err := parseApplyClientUpdatesScalar(afterText) + if err != nil { + return err + } + delta := afterTotal - beforeTotal + if delta > 0 { + return fmt.Errorf("%s increased during metadata-only reconcile (delta=%v)", apply.MetricApplyClientUpdatesTotal, delta) + } + return nil + }).WithTimeout(90*time.Second).WithPolling(5*time.Second).Should(Succeed(), + "pkg/apply ApplyObject should not invoke client.Update when spec is unchanged") + + By("checking the NRO object itself was updated (annotation patch persisted)") + nroAfter := &nropv1.NUMAResourcesOperator{} + Expect(e2eclient.Client.Get(ctx, nropKey, nroAfter)).To(Succeed()) + Expect(nroAfter.GetAnnotations()).To(HaveKey(e2eNoopReconcileAnnotation)) + Expect(nroAfter.GetUID()).To(Equal(nro.GetUID())) + }) +})