Merge pull request #1053 from shajmakh/cleanup-minor

add namespacedname helper pkg
openshift-kni · Jan 22, 2025 · bf469c1 · bf469c1
2 parents c17907b + afbfabc
commit bf469c1
Show file tree

Hide file tree

Showing 8 changed files with 152 additions and 26 deletions.
diff --git a/api/numaresourcesoperator/v1/helper/namespacedname/namespacedname.go 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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