From afbfabc9d8ee66197fa44aa9b1cfc9819bbf8a3c Mon Sep 17 00:00:00 2001 From: Shereen Haj Date: Tue, 21 Jan 2025 12:31:09 +0200 Subject: [PATCH] add namespacedname helper pkg Minor adjustments and cleanups that wraps helper functions for NamespacedName under api/v1/helper/namespacedname. Signed-off-by: Shereen Haj --- .../helper/namespacedname/namespacedname.go | 40 ++++++++ .../namespacedname/namespacedname_test.go | 98 +++++++++++++++++++ .../v1/namespacedname.go | 11 --- .../numaresourcesoperator_controller.go | 3 +- internal/wait/job.go | 14 ++- .../objectstate/sched/sched.go | 6 +- test/e2e/serial/tests/configuration.go | 3 +- .../e2e/serial/tests/scheduler_cache_stall.go | 3 +- 8 files changed, 152 insertions(+), 26 deletions(-) create mode 100644 api/numaresourcesoperator/v1/helper/namespacedname/namespacedname.go create mode 100644 api/numaresourcesoperator/v1/helper/namespacedname/namespacedname_test.go diff --git a/api/numaresourcesoperator/v1/helper/namespacedname/namespacedname.go b/api/numaresourcesoperator/v1/helper/namespacedname/namespacedname.go new file mode 100644 index 000000000..7c90fe623 --- /dev/null +++ b/api/numaresourcesoperator/v1/helper/namespacedname/namespacedname.go @@ -0,0 +1,40 @@ +/* + * Copyright 2025 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 namespacedname + +import ( + "sigs.k8s.io/controller-runtime/pkg/client" + + nropv1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1" +) + +func AsObjectKey(nn nropv1.NamespacedName) client.ObjectKey { + return client.ObjectKey{ + Namespace: nn.Namespace, + Name: nn.Name, + } +} + +func FromObject(obj client.Object) nropv1.NamespacedName { + if obj == nil { + return nropv1.NamespacedName{} + } + return nropv1.NamespacedName{ + Namespace: obj.GetNamespace(), + Name: obj.GetName(), + } +} diff --git a/api/numaresourcesoperator/v1/helper/namespacedname/namespacedname_test.go b/api/numaresourcesoperator/v1/helper/namespacedname/namespacedname_test.go new file mode 100644 index 000000000..46f03445a --- /dev/null +++ b/api/numaresourcesoperator/v1/helper/namespacedname/namespacedname_test.go @@ -0,0 +1,98 @@ +/* + * Copyright 2025 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 namespacedname + +import ( + "reflect" + "testing" + + appsv1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + v1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1" +) + +func TestAsObjectKey(t *testing.T) { + tests := []struct { + name string + nname v1.NamespacedName + expected client.ObjectKey + }{ + { + name: "empty", + expected: client.ObjectKey{}, + }, + { + name: "full", + nname: v1.NamespacedName{ + Namespace: "ns", + Name: "name", + }, + expected: client.ObjectKey{Namespace: "ns", Name: "name"}, + }, + { + name: "partial", + nname: v1.NamespacedName{ + Name: "name", + }, + expected: client.ObjectKey{Name: "name"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := AsObjectKey(tt.nname) + if !reflect.DeepEqual(got, tt.expected) { + t.Errorf("got = %v, expected %v", got, tt.expected) + } + }) + } +} + +func TestFromObject(t *testing.T) { + tests := []struct { + name string + obj client.Object + expected v1.NamespacedName + }{ + { + name: "empty", + expected: v1.NamespacedName{}, + }, + { + name: "client object", + obj: &appsv1.DaemonSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "ds", + Namespace: "ns", + }, + }, + expected: v1.NamespacedName{ + Namespace: "ns", + Name: "ds", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := FromObject(tt.obj) + if !reflect.DeepEqual(got, tt.expected) { + t.Errorf("got = %v, expected %v", got, tt.expected) + } + }) + } +} diff --git a/api/numaresourcesoperator/v1/namespacedname.go b/api/numaresourcesoperator/v1/namespacedname.go index 4489da662..c572711f0 100644 --- a/api/numaresourcesoperator/v1/namespacedname.go +++ b/api/numaresourcesoperator/v1/namespacedname.go @@ -16,10 +16,6 @@ package v1 -import ( - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -) - // This is borrowed from the kubernetes source, because controller-gen // complains about the kube native type: // encountered struct field "Namespace" without JSON tag in type "NamespacedName" @@ -40,10 +36,3 @@ const ( func (n NamespacedName) String() string { return n.Namespace + string(Separator) + n.Name } - -func NamespacedNameFromObject(obj metav1.Object) NamespacedName { - return NamespacedName{ - Namespace: obj.GetNamespace(), - Name: obj.GetName(), - } -} diff --git a/controllers/numaresourcesoperator_controller.go b/controllers/numaresourcesoperator_controller.go index 25454e477..40a8b5c16 100644 --- a/controllers/numaresourcesoperator_controller.go +++ b/controllers/numaresourcesoperator_controller.go @@ -52,6 +52,7 @@ import ( apimanifests "github.com/k8stopologyawareschedwg/deployer/pkg/manifests/api" k8swgrteupdate "github.com/k8stopologyawareschedwg/deployer/pkg/objectupdate/rte" nropv1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1" + "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1/helper/namespacedname" 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/dangling" @@ -551,7 +552,7 @@ func (r *NUMAResourcesOperatorReconciler) syncNUMAResourcesOperatorResources(ctx if err != nil { return err } - dsPoolPairs = append(dsPoolPairs, poolDaemonSet{poolName, nropv1.NamespacedNameFromObject(gdm.DaemonSet)}) + dsPoolPairs = append(dsPoolPairs, poolDaemonSet{poolName, namespacedname.FromObject(gdm.DaemonSet)}) return nil }) diff --git a/internal/wait/job.go b/internal/wait/job.go index 8cb982851..d23cd0097 100644 --- a/internal/wait/job.go +++ b/internal/wait/job.go @@ -24,14 +24,12 @@ import ( k8swait "k8s.io/apimachinery/pkg/util/wait" "k8s.io/klog/v2" - "sigs.k8s.io/controller-runtime/pkg/client" + v1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1" + "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1/helper/namespacedname" ) -func (wt Waiter) ForJobCompleted(ctx context.Context, jobNamespace, jobName string) (*batchv1.Job, error) { - jobKey := client.ObjectKey{ - Namespace: jobNamespace, - Name: jobName, - } +func (wt Waiter) ForJobCompleted(ctx context.Context, nname v1.NamespacedName) (*batchv1.Job, error) { + jobKey := namespacedname.AsObjectKey(nname) updatedJob := batchv1.Job{} immediate := true err := k8swait.PollUntilContextTimeout(ctx, wt.PollInterval, wt.PollTimeout, immediate, func(aContext context.Context) (bool, error) { @@ -40,10 +38,10 @@ func (wt Waiter) ForJobCompleted(ctx context.Context, jobNamespace, jobName stri return false, err } if !isJobCompleted(updatedJob) { - klog.Infof("%s/%s not yet completed (succeeded=%d)", jobNamespace, jobName, updatedJob.Status.Succeeded) + klog.Infof("%s not yet completed (succeeded=%d)", nname, updatedJob.Status.Succeeded) return false, nil } - klog.Infof("%s/%s completed! (succeeded=%d)", jobNamespace, jobName, updatedJob.Status.Succeeded) + klog.Infof("%s completed! (succeeded=%d)", nname, updatedJob.Status.Succeeded) return true, nil }) return &updatedJob, err diff --git a/pkg/numaresourcesscheduler/objectstate/sched/sched.go b/pkg/numaresourcesscheduler/objectstate/sched/sched.go index d3cc9a96e..ccf7ae837 100644 --- a/pkg/numaresourcesscheduler/objectstate/sched/sched.go +++ b/pkg/numaresourcesscheduler/objectstate/sched/sched.go @@ -28,6 +28,7 @@ import ( "github.com/k8stopologyawareschedwg/deployer/pkg/manifests" nropv1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1" + "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1/helper/namespacedname" schedmanifests "github.com/openshift-kni/numaresources-operator/pkg/numaresourcesscheduler/manifests/sched" "github.com/openshift-kni/numaresources-operator/pkg/objectstate" "github.com/openshift-kni/numaresources-operator/pkg/objectstate/compare" @@ -160,10 +161,7 @@ func FromClient(ctx context.Context, cli client.Client, mf schedmanifests.Manife } func DeploymentNamespacedNameFromObject(obj client.Object) (nropv1.NamespacedName, bool) { - res := nropv1.NamespacedName{ - Namespace: obj.GetNamespace(), - Name: obj.GetName(), - } + res := namespacedname.FromObject(obj) _, ok := obj.(*appsv1.Deployment) return res, ok } diff --git a/test/e2e/serial/tests/configuration.go b/test/e2e/serial/tests/configuration.go index 807cd3370..127b295a7 100644 --- a/test/e2e/serial/tests/configuration.go +++ b/test/e2e/serial/tests/configuration.go @@ -52,6 +52,7 @@ import ( machineconfigv1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" nropv1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1" + "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1/helper/namespacedname" nropmcp "github.com/openshift-kni/numaresources-operator/internal/machineconfigpools" intnrt "github.com/openshift-kni/numaresources-operator/internal/noderesourcetopology" intobjs "github.com/openshift-kni/numaresources-operator/internal/objects" @@ -1400,7 +1401,7 @@ func createTAEDeployment(fxt *e2efixture.Fixture, ctx context.Context, name, sch func daemonSetListToNamespacedNameList(dss []*appsv1.DaemonSet) []nropv1.NamespacedName { ret := make([]nropv1.NamespacedName, 0, len(dss)) for _, ds := range dss { - ret = append(ret, (nropv1.NamespacedNameFromObject(ds))) + ret = append(ret, namespacedname.FromObject(ds)) } return ret } diff --git a/test/e2e/serial/tests/scheduler_cache_stall.go b/test/e2e/serial/tests/scheduler_cache_stall.go index 2e4546ba1..0a0c64656 100644 --- a/test/e2e/serial/tests/scheduler_cache_stall.go +++ b/test/e2e/serial/tests/scheduler_cache_stall.go @@ -34,6 +34,7 @@ import ( nrtv1alpha2 "github.com/k8stopologyawareschedwg/noderesourcetopology-api/pkg/apis/topology/v1alpha2" nropv1 "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1" + "github.com/openshift-kni/numaresources-operator/api/numaresourcesoperator/v1/helper/namespacedname" intnrt "github.com/openshift-kni/numaresources-operator/internal/noderesourcetopology" e2ereslist "github.com/openshift-kni/numaresources-operator/internal/resourcelist" "github.com/openshift-kni/numaresources-operator/internal/wait" @@ -133,7 +134,7 @@ var _ = Describe("[serial][scheduler][cache] scheduler cache stall", Label("sche err := fxt.Client.Create(context.TODO(), idleJob) // will be removed by the fixture Expect(err).ToNot(HaveOccurred()) - _, err = wait.With(fxt.Client).Interval(3*time.Second).Timeout(30*time.Second).ForJobCompleted(context.TODO(), idleJob.Namespace, idleJob.Name) + _, err = wait.With(fxt.Client).Interval(3*time.Second).Timeout(30*time.Second).ForJobCompleted(context.TODO(), namespacedname.FromObject(idleJob)) Expect(err).ToNot(HaveOccurred()) // ensure foreign pods are reported