From 33e44da4742db08f1b7d864038ef1ba6212fa7c5 Mon Sep 17 00:00:00 2001 From: Talor Itzhak Date: Mon, 23 Jun 2025 12:55:06 +0300 Subject: [PATCH 1/5] objectstate: use existing object as a base Usually we're using a bare manifest as the base template for all the resources we own. The operator performs the relevant modifications on those manifests and apply them on the cluster. That usually creates unnecessary updates because the bare manifests are not containing some of the defaults which applied by the API or other controllers that present on the cluster. To fix that, we suggest to use the existing resources as the base, so those already contains all the defaults, perform the changes we care about and then apply those on the cluster. This is a different approach to accomplish: https://github.com/openshift-kni/numaresources-operator/pull/1194 Signed-off-by: Talor Itzhak --- .../numaresourcesoperator_controller.go | 20 +++- pkg/objectstate/api/api.go | 9 +- pkg/objectstate/rte/rte.go | 105 ++++++++++++++++++ pkg/objectupdate/rte/rte.go | 79 ++++++++++--- 4 files changed, 193 insertions(+), 20 deletions(-) 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/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..5e01c1b2b6 100644 --- a/pkg/objectupdate/rte/rte.go +++ b/pkg/objectupdate/rte/rte.go @@ -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 { @@ -239,10 +241,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 = []corev1.Toleration{} if len(userTolerations) == 0 { return } - podSpec := &ds.Spec.Template.Spec // shortcut podSpec.Tolerations = nropv1.CloneTolerations(userTolerations) } @@ -251,10 +255,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,17 +282,33 @@ func AllContainersTerminationMessagePolicy(ds *appsv1.DaemonSet) { } } +// 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) { + // Add the requested memory volume mount 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{ @@ -286,15 +320,32 @@ func AddVolumeMountMemory(podSpec *corev1.PodSpec, cnt *corev1.Container, mountN }, }, }, - corev1.Volume{ - Name: "rte-metrics-service-cert", - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: "rte-metrics-service-cert", + ) + + // Add the metrics certificate volume mount only if it doesn't already exist + metricsVolumeName := "rte-metrics-service-cert" + if !hasVolumeMount(cnt, metricsVolumeName) { + cnt.VolumeMounts = append(cnt.VolumeMounts, + corev1.VolumeMount{ + MountPath: "/etc/secrets/rte/", + Name: metricsVolumeName, + ReadOnly: true, + }, + ) + } + + if !hasVolume(podSpec, metricsVolumeName) { + podSpec.Volumes = append(podSpec.Volumes, + corev1.Volume{ + Name: metricsVolumeName, + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: metricsVolumeName, + }, }, }, - }, - ) + ) + } } func SecurityContextConstraint(scc *securityv1.SecurityContextConstraints, legacyRTEContext bool) { From 6ebe2f37b1539a3aedbc7f5770cf752fef7d9e3e Mon Sep 17 00:00:00 2001 From: Talor Itzhak Date: Thu, 3 Jul 2025 16:57:51 +0300 Subject: [PATCH 2/5] objectupdate: generelize volumes creation we're updating the volumes for the pod and the volume mounts for the RTE container on different stages during the operator flow. This commit generelize the update flow and make sure to set all the defaults bits to avoid any unnecessary updates calls to the server. Signed-off-by: Talor Itzhak --- .../objectstate/sched/sched.go | 20 +- pkg/objectupdate/rte/rte.go | 65 ++- pkg/objectupdate/rte/rte_test.go | 2 +- pkg/objectupdate/volume/volume.go | 176 ++++++++ pkg/objectupdate/volume/volume_test.go | 398 ++++++++++++++++++ 5 files changed, 610 insertions(+), 51 deletions(-) create mode 100644 pkg/objectupdate/volume/volume.go create mode 100644 pkg/objectupdate/volume/volume_test.go 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/objectupdate/rte/rte.go b/pkg/objectupdate/rte/rte.go index 5e01c1b2b6..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 @@ -227,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 @@ -243,7 +245,7 @@ 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 = []corev1.Toleration{} + podSpec.Tolerations = nil if len(userTolerations) == 0 { return } @@ -302,50 +304,33 @@ func hasVolume(podSpec *corev1.PodSpec, volumeName string) bool { return false } -func AddVolumeMountMemory(podSpec *corev1.PodSpec, cnt *corev1.Container, mountName, dirName string, sizeMiB int64) { +func AddVolumeMountMemory(podSpec *corev1.PodSpec, cnt *corev1.Container, mountName, dirName string, sizeMiB int64) error { // Add the requested memory volume mount - cnt.VolumeMounts = append(cnt.VolumeMounts, - corev1.VolumeMount{ - Name: mountName, - MountPath: dirName, - }, - ) - podSpec.Volumes = append(podSpec.Volumes, - corev1.Volume{ - Name: mountName, - VolumeSource: corev1.VolumeSource{ - EmptyDir: &corev1.EmptyDirVolumeSource{ - Medium: corev1.StorageMediumMemory, - SizeLimit: resource.NewQuantity(sizeMiB, resource.BinarySI), - }, - }, - }, - ) + 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) { - cnt.VolumeMounts = append(cnt.VolumeMounts, - corev1.VolumeMount{ - MountPath: "/etc/secrets/rte/", - Name: metricsVolumeName, - ReadOnly: true, - }, - ) + if !hasVolumeMount(cnt, metricsVolumeName) && !hasVolume(podSpec, metricsVolumeName) { + volume.AddSecret(podSpec, cnt, metricsVolumeName, "/etc/secrets/rte/", metricsVolumeName, volume.DefaultMode, false, true) } - if !hasVolume(podSpec, metricsVolumeName) { - podSpec.Volumes = append(podSpec.Volumes, - corev1.Volume{ - Name: metricsVolumeName, - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: metricsVolumeName, - }, - }, - }, - ) + // 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)) + } + }) + } +} From 8a92eea0e80a7ceedd0c948baefec298d2098f58 Mon Sep 17 00:00:00 2001 From: Talor Itzhak Date: Sun, 29 Mar 2026 17:44:25 +0300 Subject: [PATCH 3/5] Dockerfile: disable rhel repo in case no subscription for rhel repos is available, the image failed to build. disabling rhel repo so it will be able to pull hwdata and other deps if necessary. Signed-off-by: Talor Itzhak --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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"] From 7f58f43da37b366e9528c29832be356de2a46539 Mon Sep 17 00:00:00 2001 From: Talor Itzhak Date: Mon, 13 Apr 2026 11:22:51 +0300 Subject: [PATCH 4/5] metrics: add client update metrics This metric addition intended to be use only in tests (for time being) to be able to track when the controller is calling Update() Signed-off-by: Talor Itzhak --- pkg/apply/apply.go | 1 + pkg/apply/metrics.go | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 pkg/apply/metrics.go 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() +} From 39861f92cf922aa1119e08af28be4dfa23c030a3 Mon Sep 17 00:00:00 2001 From: Talor Itzhak Date: Mon, 13 Apr 2026 11:29:34 +0300 Subject: [PATCH 5/5] e2e: add nonreg test to make sure that changes in the objects API won't causes unnecessary updates we're adding this test to count for updates when a dummy annotation added to the CR. The result should be zero calls for updates when dummy changes are made. Signed-off-by: Talor Itzhak --- test/e2e/serial/tests/reconcile_noop.go | 149 ++++++++++++++++++++++++ 1 file changed, 149 insertions(+) create mode 100644 test/e2e/serial/tests/reconcile_noop.go 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())) + }) +})