Merge pull request kubernetes#949 from alculquicondor/util-cleanup

Simplify tests setup

main.go

@@ -218,7 +218,7 @@ func setupControllers(ctx context.Context, mgr ctrl.Manager, cCache *cache.Cache
 	err := jobframework.ForEachIntegration(func(name string, cb jobframework.IntegrationCallbacks) error {
 		log := setupLog.WithValues("jobFrameworkName", name)
 		if isFrameworkEnabled(cfg, name) && crds.Has(name) {
-			if err := cb.NewReconciler(mgr.GetScheme(),
+			if err := cb.NewReconciler(
 				mgr.GetClient(),
 				mgr.GetEventRecorderFor(fmt.Sprintf("%s-%s-controller", name, constants.KueueName)),
 				opts...,

pkg/controller/core/clusterqueue_controller_test.go

@@ -17,14 +17,11 @@ limitations under the License.
 package core
 
 import (
-	"context"
 	"testing"
 
-	"github.com/go-logr/logr/testr"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	ctrl "sigs.k8s.io/controller-runtime"
 
 	kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1"
 	"sigs.k8s.io/kueue/pkg/cache"
@@ -167,10 +164,7 @@ func TestUpdateCqStatusIfChanged(t *testing.T) {
 			cq.Status = tc.cqStatus
 			lq := utiltesting.MakeLocalQueue(lqName, "").
 				ClusterQueue(cqName).Obj()
-			log := testr.NewWithOptions(t, testr.Options{
-				Verbosity: 2,
-			})
-			ctx := ctrl.LoggerInto(context.Background(), log)
+			ctx, log := utiltesting.ContextWithLog(t)
 
 			cl := utiltesting.NewClientBuilder().WithLists(defaultWls).WithObjects(lq, cq).WithStatusSubresource(lq, cq).
 				Build()

pkg/controller/jobframework/integrationmanager.go

@@ -38,7 +38,7 @@ type JobReconcilerInterface interface {
 	SetupWithManager(mgr ctrl.Manager) error
 }
 
-type ReconcilerFactory func(scheme *runtime.Scheme, client client.Client, record record.EventRecorder, opts ...Option) JobReconcilerInterface
+type ReconcilerFactory func(client client.Client, record record.EventRecorder, opts ...Option) JobReconcilerInterface
 
 // IntegrationCallbacks groups a set of callbacks used to integrate a new framework.
 type IntegrationCallbacks struct {

pkg/controller/jobframework/integrationmanager_test.go

@@ -32,7 +32,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-func testNewReconciler(*runtime.Scheme, client.Client, record.EventRecorder, ...Option) JobReconcilerInterface {
+func testNewReconciler(client.Client, record.EventRecorder, ...Option) JobReconcilerInterface {
 	return nil
 }
 
@@ -257,9 +257,8 @@ func TestForEach(t *testing.T) {
 }
 
 func TestGetCallbacksForOwner(t *testing.T) {
-
 	dontManage := IntegrationCallbacks{
-		NewReconciler: func(*runtime.Scheme, client.Client, record.EventRecorder, ...Option) JobReconcilerInterface {
+		NewReconciler: func(client.Client, record.EventRecorder, ...Option) JobReconcilerInterface {
 			panic("not implemented")
 		},
 		SetupWebhook: func(ctrl.Manager, ...Option) error { panic("not implemented") },

pkg/controller/jobframework/reconciler.go

@@ -22,7 +22,6 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/tools/record"
@@ -51,7 +50,6 @@ var (
 // JobReconciler reconciles a GenericJob object
 type JobReconciler struct {
 	client                     client.Client
-	scheme                     *runtime.Scheme
 	record                     record.EventRecorder
 	manageJobsWithoutQueueName bool
 	waitForPodsReady           bool
@@ -85,7 +83,6 @@ func WithWaitForPodsReady(f bool) Option {
 var DefaultOptions = Options{}
 
 func NewReconciler(
-	scheme *runtime.Scheme,
 	client client.Client,
 	record record.EventRecorder,
 	opts ...Option) *JobReconciler {
@@ -95,7 +92,6 @@ func NewReconciler(
 	}
 
 	return &JobReconciler{
-		scheme:                     scheme,
 		client:                     client,
 		record:                     record,
 		manageJobsWithoutQueueName: options.ManageJobsWithoutQueueName,
@@ -465,7 +461,7 @@ func (r *JobReconciler) constructWorkload(ctx context.Context, job GenericJob, o
 	wl.Spec.PriorityClassName = priorityClassName
 	wl.Spec.Priority = &p
 
-	if err := ctrl.SetControllerReference(object, wl, r.scheme); err != nil {
+	if err := ctrl.SetControllerReference(object, wl, r.client.Scheme()); err != nil {
 		return nil, err
 	}
 	return wl, nil
@@ -593,9 +589,9 @@ func getPodSetsInfoFromWorkload(wl *kueue.Workload) []PodSetInfo {
 // newJob should return a new empty job.
 // newWorkloadHandler it's optional, if added it should return a new workload event handler.
 func NewGenericReconciler(newJob func() GenericJob, newWorkloadHandler func(client.Client) handler.EventHandler) ReconcilerFactory {
-	return func(scheme *runtime.Scheme, client client.Client, record record.EventRecorder, opts ...Option) JobReconcilerInterface {
+	return func(client client.Client, record record.EventRecorder, opts ...Option) JobReconcilerInterface {
 		return &genericReconciler{
-			jr:                 NewReconciler(scheme, client, record, opts...),
+			jr:                 NewReconciler(client, record, opts...),
 			newJob:             newJob,
 			newWorkloadHandler: newWorkloadHandler,
 		}

pkg/controller/jobframework/reconciler_test.go

@@ -93,7 +93,7 @@ func TestIsParentJobManaged(t *testing.T) {
 				builder = builder.WithObjects(tc.parentJob)
 			}
 			cl := builder.Build()
-			r := NewReconciler(nil, cl, nil)
+			r := NewReconciler(cl, nil)
 			got, gotErr := r.IsParentJobManaged(context.Background(), tc.job, jobNamespace)
 			if tc.wantManaged != got {
 				t.Errorf("Unexpected response from isParentManaged want: %v,got: %v", tc.wantManaged, got)

pkg/scheduler/preemption/preemption_test.go

@@ -23,15 +23,13 @@ import (
 	"testing"
 	"time"
 
-	"github.com/go-logr/logr/testr"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/tools/record"
-	ctrl "sigs.k8s.io/controller-runtime"
 
 	kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1"
 	"sigs.k8s.io/kueue/pkg/cache"
@@ -745,10 +743,7 @@ func TestPreemption(t *testing.T) {
 	}
 	for name, tc := range cases {
 		t.Run(name, func(t *testing.T) {
-			log := testr.NewWithOptions(t, testr.Options{
-				Verbosity: 2,
-			})
-			ctx := ctrl.LoggerInto(context.Background(), log)
+			ctx, _ := utiltesting.ContextWithLog(t)
 			cl := utiltesting.NewClientBuilder().
 				WithLists(&kueue.WorkloadList{Items: tc.admitted}).
 				Build()

pkg/scheduler/scheduler_test.go

@@ -24,7 +24,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/go-logr/logr/testr"
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	corev1 "k8s.io/api/core/v1"
@@ -34,7 +33,6 @@ import (
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/utils/pointer"
-	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1"
@@ -857,10 +855,7 @@ func TestSchedule(t *testing.T) {
 			if tc.enablePartialAdmission {
 				defer features.SetFeatureGateDuringTest(t, features.PartialAdmission, true)()
 			}
-			log := testr.NewWithOptions(t, testr.Options{
-				Verbosity: 2,
-			})
-			ctx := ctrl.LoggerInto(context.Background(), log)
+			ctx, _ := utiltesting.ContextWithLog(t)
 			scheme := runtime.NewScheme()
 
 			allQueues := append(queues, tc.additionalLocalQueues...)
@@ -1142,10 +1137,7 @@ func TestRequeueAndUpdate(t *testing.T) {
 
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
-			log := testr.NewWithOptions(t, testr.Options{
-				Verbosity: 2,
-			})
-			ctx := ctrl.LoggerInto(context.Background(), log)
+			ctx, log := utiltesting.ContextWithLog(t)
 			scheme := runtime.NewScheme()
 
 			objs := []client.Object{w1, q1, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "ns1"}}}

pkg/util/testing/context.go

@@ -0,0 +1,33 @@
+/*
+Copyright 2023 The Kubernetes Authors.
+
+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 testing
+
+import (
+	"context"
+	"testing"
+
+	"github.com/go-logr/logr"
+	"github.com/go-logr/logr/testr"
+	ctrl "sigs.k8s.io/controller-runtime"
+)
+
+func ContextWithLog(t *testing.T) (context.Context, logr.Logger) {
+	logger := testr.NewWithOptions(t, testr.Options{
+		Verbosity: 2,
+	})
+	return ctrl.LoggerInto(context.Background(), logger), logger
+}

pkg/util/testingjobs/job/wrappers.go

@@ -49,7 +49,6 @@ func MakeJob(name, ns string) *JobWrapper {
 						{
 							Name:      "c",
 							Image:     "pause",
-							Command:   []string{},
 							Resources: corev1.ResourceRequirements{Requests: corev1.ResourceList{}},
 						},
 					},
@@ -133,8 +132,7 @@ func (j *JobWrapper) Request(r corev1.ResourceName, v string) *JobWrapper {
 	return j
 }
 
-func (j *JobWrapper) Image(name string, image string, args []string) *JobWrapper {
-	j.Spec.Template.Spec.Containers[0].Name = name
+func (j *JobWrapper) Image(image string, args []string) *JobWrapper {
 	j.Spec.Template.Spec.Containers[0].Image = image
 	j.Spec.Template.Spec.Containers[0].Args = args
 	return j

test/e2e/e2e_test.go

@@ -126,7 +126,7 @@ var _ = ginkgo.Describe("Kueue", func() {
 
 		ginkgo.It("Should unsuspend a job and set nodeSelectors", func() {
 			// Use a binary that ends.
-			sampleJob = (&testingjob.JobWrapper{Job: *sampleJob}).Image("sleep", "gcr.io/k8s-staging-perf-tests/sleep:v0.0.3", []string{"5s"}).Obj()
+			sampleJob = (&testingjob.JobWrapper{Job: *sampleJob}).Image("gcr.io/k8s-staging-perf-tests/sleep:v0.0.3", []string{"5s"}).Obj()
 			gomega.Expect(k8sClient.Create(ctx, sampleJob)).Should(gomega.Succeed())
 
 			createdWorkload := &kueue.Workload{}

test/integration/controller/job/suite_test.go

@@ -60,7 +60,6 @@ func TestAPIs(t *testing.T) {
 func managerSetup(opts ...jobframework.Option) framework.ManagerSetup {
 	return func(mgr manager.Manager, ctx context.Context) {
 		reconciler := job.NewReconciler(
-			mgr.GetScheme(),
 			mgr.GetClient(),
 			mgr.GetEventRecorderFor(constants.JobControllerName),
 			opts...)
@@ -86,7 +85,7 @@ func managerAndSchedulerSetup(opts ...jobframework.Option) framework.ManagerSetu
 
 		err = job.SetupIndexes(ctx, mgr.GetFieldIndexer())
 		gomega.Expect(err).NotTo(gomega.HaveOccurred())
-		err = job.NewReconciler(mgr.GetScheme(), mgr.GetClient(),
+		err = job.NewReconciler(mgr.GetClient(),
 			mgr.GetEventRecorderFor(constants.JobControllerName), opts...).SetupWithManager(mgr)
 		gomega.Expect(err).NotTo(gomega.HaveOccurred())
 		err = job.SetupWebhook(mgr, opts...)

test/integration/controller/jobset/suite_test.go

@@ -60,7 +60,6 @@ func TestAPIs(t *testing.T) {
 func managerSetup(opts ...jobframework.Option) framework.ManagerSetup {
 	return func(mgr manager.Manager, ctx context.Context) {
 		reconciler := jobset.NewReconciler(
-			mgr.GetScheme(),
 			mgr.GetClient(),
 			mgr.GetEventRecorderFor(constants.JobControllerName),
 			opts...)
@@ -86,7 +85,7 @@ func managerAndSchedulerSetup(opts ...jobframework.Option) framework.ManagerSetu
 
 		err = jobset.SetupIndexes(ctx, mgr.GetFieldIndexer())
 		gomega.Expect(err).NotTo(gomega.HaveOccurred())
-		err = jobset.NewReconciler(mgr.GetScheme(), mgr.GetClient(),
+		err = jobset.NewReconciler(mgr.GetClient(),
 			mgr.GetEventRecorderFor(constants.JobControllerName), opts...).SetupWithManager(mgr)
 		gomega.Expect(err).NotTo(gomega.HaveOccurred())
 		err = jobset.SetupJobSetWebhook(mgr, opts...)

test/integration/controller/mpijob/suite_test.go

@@ -61,7 +61,6 @@ func TestAPIs(t *testing.T) {
 func managerSetup(setupJobManager bool, opts ...jobframework.Option) framework.ManagerSetup {
 	return func(mgr manager.Manager, ctx context.Context) {
 		reconciler := mpijob.NewReconciler(
-			mgr.GetScheme(),
 			mgr.GetClient(),
 			mgr.GetEventRecorderFor(constants.JobControllerName),
 			opts...)
@@ -74,7 +73,6 @@ func managerSetup(setupJobManager bool, opts ...jobframework.Option) framework.M
 
 		if setupJobManager {
 			jobReconciler := job.NewReconciler(
-				mgr.GetScheme(),
 				mgr.GetClient(),
 				mgr.GetEventRecorderFor(constants.JobControllerName),
 				opts...)
@@ -101,7 +99,7 @@ func managerAndSchedulerSetup(opts ...jobframework.Option) framework.ManagerSetu
 
 		err = mpijob.SetupIndexes(ctx, mgr.GetFieldIndexer())
 		gomega.Expect(err).NotTo(gomega.HaveOccurred())
-		err = mpijob.NewReconciler(mgr.GetScheme(), mgr.GetClient(),
+		err = mpijob.NewReconciler(mgr.GetClient(),
 			mgr.GetEventRecorderFor(constants.JobControllerName), opts...).SetupWithManager(mgr)
 		gomega.Expect(err).NotTo(gomega.HaveOccurred())
 		err = mpijob.SetupMPIJobWebhook(mgr, opts...)

test/integration/controller/rayjob/suite_test.go

@@ -60,7 +60,6 @@ func TestAPIs(t *testing.T) {
 func managerSetup(opts ...jobframework.Option) framework.ManagerSetup {
 	return func(mgr manager.Manager, ctx context.Context) {
 		reconciler := rayjob.NewReconciler(
-			mgr.GetScheme(),
 			mgr.GetClient(),
 			mgr.GetEventRecorderFor(constants.JobControllerName),
 			opts...)
@@ -86,7 +85,7 @@ func managerAndSchedulerSetup(opts ...jobframework.Option) framework.ManagerSetu
 
 		err = rayjob.SetupIndexes(ctx, mgr.GetFieldIndexer())
 		gomega.Expect(err).NotTo(gomega.HaveOccurred())
-		err = rayjob.NewReconciler(mgr.GetScheme(), mgr.GetClient(),
+		err = rayjob.NewReconciler(mgr.GetClient(),
 			mgr.GetEventRecorderFor(constants.JobControllerName), opts...).SetupWithManager(mgr)
 		gomega.Expect(err).NotTo(gomega.HaveOccurred())
 		err = rayjob.SetupRayJobWebhook(mgr, opts...)