From 81d00f364788c87a2a973fb538e674a3c1354ff2 Mon Sep 17 00:00:00 2001 From: googs1025 Date: Sat, 30 Mar 2024 16:54:08 +0800 Subject: [PATCH 1/8] test: add pod controller unit test --- pkg/controllers/pod_controller_test.go | 430 +++++++++++++++++++++++++ pkg/util/testing/wrappers.go | 65 ++++ 2 files changed, 495 insertions(+) create mode 100644 pkg/controllers/pod_controller_test.go diff --git a/pkg/controllers/pod_controller_test.go b/pkg/controllers/pod_controller_test.go new file mode 100644 index 000000000..7d89d4fc7 --- /dev/null +++ b/pkg/controllers/pod_controller_test.go @@ -0,0 +1,430 @@ +/* +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 + htcp://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 controllers + +import ( + "context" + "errors" + "fmt" + "sort" + "strconv" + "testing" + + "github.com/stretchr/testify/assert" + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" + "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + + jobset "sigs.k8s.io/jobset/api/jobset/v1alpha2" + "sigs.k8s.io/jobset/pkg/constants" + testutils "sigs.k8s.io/jobset/pkg/util/testing" +) + +func TestValidatePodPlacements(t *testing.T) { + var ( + jobSetName = "test-jobset" + ns = "default" + leaderPodWrapper = makePod(&makePodArgs{ + jobSetName: jobSetName, + replicatedJobName: "replicated-job-1", + jobName: "test-jobset-replicated-job-1-test-job-0", + podName: "test-jobset-replicated-job-1-test-job-0-0", + ns: ns, + nodeName: "test-node", + jobIdx: 0}) + podWrapper = makePod(&makePodArgs{ + jobSetName: jobSetName, + replicatedJobName: "replicated-job-1", + jobName: "test-jobset-replicated-job-1-test-job-0", + podName: "test-jobset-replicated-job-1-test-job-0-1", + ns: ns, + jobIdx: 0}) + ) + tests := []struct { + name string + leaderPod corev1.Pod + podList *corev1.PodList + node *corev1.Node + wantErr error + forceClientErr bool + wantMatched bool + }{ + { + name: "topology node label not found", + leaderPod: leaderPodWrapper.AddAnnotation(jobset.ExclusiveKey, "test-node-topologyKey"). + AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + podList: &corev1.PodList{ + Items: []corev1.Pod{ + leaderPodWrapper.AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + podWrapper.NodeSelector(map[string]string{"test-node-topologyKey": "topologyDomain"}).Obj(), + }, + }, + node: &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-node", + }, + }, + wantMatched: false, + wantErr: fmt.Errorf("node does not have topology label: %s", "test-node-topologyKey"), + }, + { + name: "valid pod placements with matched", + leaderPod: leaderPodWrapper.AddAnnotation(jobset.ExclusiveKey, "test-node-topologyKey"). + AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + podList: &corev1.PodList{ + Items: []corev1.Pod{ + leaderPodWrapper.NodeSelector(map[string]string{"test-node-topologyKey": "topologyDomain"}). + AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + podWrapper.NodeSelector(map[string]string{"test-node-topologyKey": "topologyDomain"}).Obj(), + }, + }, + node: &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-node", + Labels: map[string]string{"test-node-topologyKey": "topologyDomain"}, + }, + }, + wantMatched: true, + wantErr: nil, + }, + { + name: "valid pod placements with pod nodeSelector is nil", + leaderPod: leaderPodWrapper.AddAnnotation(jobset.ExclusiveKey, "test-node-topologyKey"). + AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + podList: &corev1.PodList{ + Items: []corev1.Pod{ + leaderPodWrapper.NodeSelector(map[string]string{ + "test-node-topologyKey": "topologyDomain", + }).AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + podWrapper.NodeSelector(nil).Obj(), + }, + }, + node: &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-node", + Labels: map[string]string{"test-node-topologyKey": "topologyDomain"}, + }, + }, + wantMatched: false, + wantErr: fmt.Errorf("pod %s nodeSelector is nil", "test-jobset-replicated-job-1-test-job-0-1"), + }, + { + name: "valid pod placements with pod nodeSelector is missing key", + leaderPod: leaderPodWrapper.AddAnnotation(jobset.ExclusiveKey, "test-node-topologyKey"). + AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + podList: &corev1.PodList{ + Items: []corev1.Pod{ + leaderPodWrapper.AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + podWrapper.NodeSelector(map[string]string{}).Obj(), + }, + }, + node: &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-node", + Labels: map[string]string{"test-node-topologyKey": "topologyDomain"}, + }, + }, + wantMatched: false, + wantErr: fmt.Errorf("pod %s nodeSelector is missing key: %s", + "test-jobset-replicated-job-1-test-job-0-1", "test-node-topologyKey"), + }, + { + name: "valid pod placements with followerTopology != leaderTopology", + leaderPod: leaderPodWrapper.AddAnnotation(jobset.ExclusiveKey, "test-node-topologyKey"). + AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + podList: &corev1.PodList{ + Items: []corev1.Pod{ + leaderPodWrapper.NodeSelector(map[string]string{"test-node-topologyKey": "topologyDomain"}). + AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + podWrapper.NodeSelector(map[string]string{"test-node-topologyKey": "topologyDomain1"}).Obj(), + }, + }, + node: &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-node", + Labels: map[string]string{"test-node-topologyKey": "topologyDomain"}, + }, + }, + wantMatched: false, + wantErr: fmt.Errorf("follower topology %q != leader topology %q", + "topologyDomain1", "topologyDomain"), + }, + { + name: "valid pod placements with get node error", + leaderPod: leaderPodWrapper.AddAnnotation(jobset.ExclusiveKey, "test-node-topologyKey"). + AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + podList: &corev1.PodList{ + Items: []corev1.Pod{ + leaderPodWrapper.NodeSelector(map[string]string{"test-node-topologyKey": "topologyDomain"}). + AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + podWrapper.NodeSelector(map[string]string{"test-node-topologyKey": "topologyDomain1"}).Obj(), + }, + }, + node: &corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-node", + Labels: map[string]string{"test-node-topologyKey": "topologyDomain"}, + }, + }, + wantMatched: false, + forceClientErr: true, + wantErr: errors.Join(errors.New("example error")), + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + fc := makeFakeClient(interceptor.Funcs{ + Get: func(ctx context.Context, client client.WithWatch, + key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + node, ok := obj.(*corev1.Node) + if !ok { + return nil + } + if tc.forceClientErr || node == nil { + return errors.Join(errors.New("example error")) + } + *node = *tc.node + return nil + }}) + r := &PodReconciler{ + Client: fc.client, + Scheme: fc.scheme, + Record: fc.record, + } + gotMatched, gotErr := r.validatePodPlacements(context.Background(), &tc.leaderPod, tc.podList) + assert.Equal(t, tc.wantErr, gotErr) + assert.Equal(t, tc.wantMatched, gotMatched) + }) + } +} + +func TestDeleteFollowerPods(t *testing.T) { + var ( + jobSetName = "test-jobset" + ns = "default" + leaderPodWrapper = makePod(&makePodArgs{ + jobSetName: jobSetName, + replicatedJobName: "replicated-job-1", + jobName: "test-jobset-replicated-job-1-test-job-0", + podName: "test-jobset-replicated-job-1-test-job-0-0", + ns: ns, + nodeName: "test-node", + jobIdx: 0}) + followerPodWrapper = makePod(&makePodArgs{ + jobSetName: jobSetName, + replicatedJobName: "replicated-job-1", + jobName: "test-jobset-replicated-job-1-test-job-0", + podName: "test-jobset-replicated-job-1-test-job-0-1", + ns: ns, + jobIdx: 0}) + wantPodWrapper = makePod(&makePodArgs{ + jobSetName: jobSetName, + replicatedJobName: "replicated-job-1", + jobName: "test-jobset-replicated-job-1-test-job-0", + podName: "test-jobset-replicated-job-1-test-job-0-1", + ns: ns, + jobIdx: 0}) + ) + tests := []struct { + name string + pods []corev1.Pod + wantPodsDeleted []corev1.Pod + wantErr error + forceClientErr bool + }{ + { + name: "delete follower pods", + pods: []corev1.Pod{ + leaderPodWrapper.AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + followerPodWrapper.Obj(), + }, + wantPodsDeleted: []corev1.Pod{ + wantPodWrapper.ResourceVersion("999").SetConditions([]corev1.PodCondition{{ + Type: corev1.DisruptionTarget, + Status: corev1.ConditionTrue, + Reason: constants.ExclusivePlacementViolationReason, + LastTransitionTime: metav1.Now(), + Message: constants.ExclusivePlacementViolationMessage, + }, + }).Obj(), + }, + }, + { + name: "delete follower pods with pod conditions status is false", + pods: []corev1.Pod{ + leaderPodWrapper.AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + followerPodWrapper.SetConditions([]corev1.PodCondition{{ + Type: corev1.DisruptionTarget, + Status: corev1.ConditionFalse, + Reason: constants.ExclusivePlacementViolationReason, + LastTransitionTime: metav1.Now(), + Message: constants.ExclusivePlacementViolationMessage, + }, + }).Obj(), + }, + wantPodsDeleted: []corev1.Pod{ + wantPodWrapper.ResourceVersion("999").SetConditions([]corev1.PodCondition{{ + Type: corev1.DisruptionTarget, + Status: corev1.ConditionTrue, + Reason: constants.ExclusivePlacementViolationReason, + LastTransitionTime: metav1.Now(), + Message: constants.ExclusivePlacementViolationMessage, + }, + }).Obj(), + }, + }, + { + name: "delete follower pods with update pod status error", + pods: []corev1.Pod{ + leaderPodWrapper.AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + followerPodWrapper.SetConditions([]corev1.PodCondition{{ + Type: corev1.DisruptionTarget, + Status: corev1.ConditionFalse, + Reason: constants.ExclusivePlacementViolationReason, + LastTransitionTime: metav1.Now().Rfc3339Copy(), + Message: constants.ExclusivePlacementViolationMessage, + }, + }).Obj(), + }, + wantPodsDeleted: nil, + forceClientErr: true, + wantErr: errors.Join(errors.Join(errors.New("example error"))), + }, + { + name: "delete follower pods with delete error", + pods: []corev1.Pod{ + leaderPodWrapper.AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + followerPodWrapper.SetConditions([]corev1.PodCondition{{ + Type: corev1.DisruptionTarget, + Status: corev1.ConditionTrue, + Reason: constants.ExclusivePlacementViolationReason, + Message: constants.ExclusivePlacementViolationMessage}, + }).Obj(), + }, + wantPodsDeleted: nil, + forceClientErr: true, + wantErr: errors.Join(errors.Join(errors.New("example error"))), + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var gotPodsDeleted []corev1.Pod + fc := makeFakeClient(interceptor.Funcs{ + Delete: func(ctx context.Context, client client.WithWatch, + obj client.Object, opts ...client.DeleteOption) error { + pod, ok := obj.(*corev1.Pod) + if !ok { + return nil + } + if tc.forceClientErr || pod == nil { + return errors.Join(errors.New("example error")) + } + // This is to solve the problem that the timestamps do not match. There will be a slight gap. + pod.Status.Conditions[0].LastTransitionTime = + tc.wantPodsDeleted[0].Status.Conditions[0].LastTransitionTime + gotPodsDeleted = append(gotPodsDeleted, *pod) + return nil + }, + SubResourceUpdate: func(ctx context.Context, client client.Client, + subResourceName string, obj client.Object, opts ...client.SubResourceUpdateOption) error { + pod, ok := obj.(*corev1.Pod) + if !ok { + return nil + } + if tc.forceClientErr || pod == nil { + return errors.Join(errors.New("example error")) + } + return nil + }, + }, &tc.pods[0], &tc.pods[1]) + r := &PodReconciler{ + Client: fc.client, + Scheme: fc.scheme, + Record: fc.record, + } + + gotErr := r.deleteFollowerPods(context.Background(), tc.pods) + assert.Equal(t, gotErr, tc.wantErr) + sort.Slice(gotPodsDeleted, func(i, j int) bool { + return gotPodsDeleted[i].Name < gotPodsDeleted[j].Name + }) + sort.Slice(tc.wantPodsDeleted, func(i, j int) bool { + return tc.wantPodsDeleted[i].Name < tc.wantPodsDeleted[j].Name + }) + + if !assert.Equal(t, gotPodsDeleted, tc.wantPodsDeleted) { + t.Errorf("deleteFollowerPods() did not make the expected pod deletion calls") + } + }) + } +} + +type makePodArgs struct { + jobSetName string + replicatedJobName string + jobName string + podName string + nodeName string + ns string + jobIdx int +} + +// Helper function to create a Pod for unit testing. +func makePod(args *makePodArgs) *testutils.PodWrapper { + labels := map[string]string{ + jobset.JobSetNameKey: args.jobSetName, + jobset.ReplicatedJobNameKey: args.replicatedJobName, + jobset.JobIndexKey: strconv.Itoa(args.jobIdx), + jobset.JobKey: jobHashKey(args.ns, args.jobName), + } + annotations := map[string]string{ + jobset.JobSetNameKey: args.jobSetName, + jobset.JobIndexKey: strconv.Itoa(args.jobIdx), + jobset.JobKey: jobHashKey(args.ns, args.jobName), + } + return testutils.MakePod(args.podName, args.ns).Labels(labels).Annotations(annotations) +} + +type fakeClient struct { + client client.WithWatch + scheme *runtime.Scheme + record record.EventRecorder +} + +func makeFakeClient(interceptor interceptor.Funcs, initObjs ...client.Object) *fakeClient { + scheme := runtime.NewScheme() + utilruntime.Must(jobset.AddToScheme(scheme)) + utilruntime.Must(corev1.AddToScheme(scheme)) + utilruntime.Must(batchv1.AddToScheme(scheme)) + + eventBroadcaster := record.NewBroadcaster() + recorder := eventBroadcaster.NewRecorder(scheme, corev1.EventSource{Component: "jobset-test-reconciler"}) + fc := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(initObjs...). + WithInterceptorFuncs(interceptor). + WithStatusSubresource(initObjs...). + Build() + return &fakeClient{ + client: fc, + scheme: scheme, + record: recorder, + } +} diff --git a/pkg/util/testing/wrappers.go b/pkg/util/testing/wrappers.go index d12fbc05c..77a99cd7d 100644 --- a/pkg/util/testing/wrappers.go +++ b/pkg/util/testing/wrappers.go @@ -369,3 +369,68 @@ func (j *JobWrapper) NodeSelector(nodeSelector map[string]string) *JobWrapper { func (j *JobWrapper) Obj() *batchv1.Job { return &j.Job } + +// PodWrapper wraps a Pod. +type PodWrapper struct { + corev1.Pod +} + +// MakePod creates a wrapper for a Pod. +func MakePod(podName, ns string) *PodWrapper { + return &PodWrapper{ + corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: podName, + Namespace: ns, + }, + Spec: corev1.PodSpec{}, + }, + } +} + +// AddAnnotation add a pod annotation. +func (p *PodWrapper) AddAnnotation(key, value string) *PodWrapper { + p.ObjectMeta.Annotations[key] = value + return p +} + +// AddLabel add a pod label. +func (p *PodWrapper) AddLabel(key, value string) *PodWrapper { + p.ObjectMeta.Labels[key] = value + return p +} + +// Annotations sets the pod annotations. +func (p *PodWrapper) Annotations(annotations map[string]string) *PodWrapper { + p.ObjectMeta.Annotations = annotations + return p +} + +// Labels sets the pod labels. +func (p *PodWrapper) Labels(labels map[string]string) *PodWrapper { + p.ObjectMeta.Labels = labels + return p +} + +// ResourceVersion sets the pod.metadata.resourceVersion. +func (p *PodWrapper) ResourceVersion(resourceVersion string) *PodWrapper { + p.ObjectMeta.ResourceVersion = resourceVersion + return p +} + +// SetConditions sets the value of the pod.status.conditions. +func (p *PodWrapper) SetConditions(conditions []corev1.PodCondition) *PodWrapper { + p.Status.Conditions = conditions + return p +} + +// NodeSelector sets the value of the pod.spec.nodeSelector. +func (p *PodWrapper) NodeSelector(nodeSelector map[string]string) *PodWrapper { + p.Spec.NodeSelector = nodeSelector + return p +} + +// Obj returns the wrapped Pod. +func (p *PodWrapper) Obj() corev1.Pod { + return p.Pod +} From ce146ef8a540dadd683d02fe91df893f8206dd4d Mon Sep 17 00:00:00 2001 From: googs1025 Date: Tue, 16 Apr 2024 16:02:03 +0800 Subject: [PATCH 2/8] add pod_controller ut --- pkg/controllers/pod_controller_test.go | 37 ++++++++++++-------------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/pkg/controllers/pod_controller_test.go b/pkg/controllers/pod_controller_test.go index 7d89d4fc7..371251764 100644 --- a/pkg/controllers/pod_controller_test.go +++ b/pkg/controllers/pod_controller_test.go @@ -56,6 +56,7 @@ func TestValidatePodPlacements(t *testing.T) { podName: "test-jobset-replicated-job-1-test-job-0-1", ns: ns, jobIdx: 0}) + nodeSelector = map[string]string{"test-node-topologyKey": "topologyDomain"} ) tests := []struct { name string @@ -73,7 +74,7 @@ func TestValidatePodPlacements(t *testing.T) { podList: &corev1.PodList{ Items: []corev1.Pod{ leaderPodWrapper.AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), - podWrapper.NodeSelector(map[string]string{"test-node-topologyKey": "topologyDomain"}).Obj(), + podWrapper.NodeSelector(nodeSelector).Obj(), }, }, node: &corev1.Node{ @@ -81,8 +82,7 @@ func TestValidatePodPlacements(t *testing.T) { Name: "test-node", }, }, - wantMatched: false, - wantErr: fmt.Errorf("node does not have topology label: %s", "test-node-topologyKey"), + wantErr: fmt.Errorf("node does not have topology label: %s", "test-node-topologyKey"), }, { name: "valid pod placements with matched", @@ -90,22 +90,21 @@ func TestValidatePodPlacements(t *testing.T) { AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), podList: &corev1.PodList{ Items: []corev1.Pod{ - leaderPodWrapper.NodeSelector(map[string]string{"test-node-topologyKey": "topologyDomain"}). + leaderPodWrapper.NodeSelector(nodeSelector). AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), - podWrapper.NodeSelector(map[string]string{"test-node-topologyKey": "topologyDomain"}).Obj(), + podWrapper.NodeSelector(nodeSelector).Obj(), }, }, node: &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: "test-node", - Labels: map[string]string{"test-node-topologyKey": "topologyDomain"}, + Labels: nodeSelector, }, }, wantMatched: true, - wantErr: nil, }, { - name: "valid pod placements with pod nodeSelector is nil", + name: "follower pod nodeSelector is nil", leaderPod: leaderPodWrapper.AddAnnotation(jobset.ExclusiveKey, "test-node-topologyKey"). AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), podList: &corev1.PodList{ @@ -119,14 +118,14 @@ func TestValidatePodPlacements(t *testing.T) { node: &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: "test-node", - Labels: map[string]string{"test-node-topologyKey": "topologyDomain"}, + Labels: nodeSelector, }, }, wantMatched: false, wantErr: fmt.Errorf("pod %s nodeSelector is nil", "test-jobset-replicated-job-1-test-job-0-1"), }, { - name: "valid pod placements with pod nodeSelector is missing key", + name: "valid pod placements with pod nodeSelector is empty", leaderPod: leaderPodWrapper.AddAnnotation(jobset.ExclusiveKey, "test-node-topologyKey"). AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), podList: &corev1.PodList{ @@ -138,10 +137,9 @@ func TestValidatePodPlacements(t *testing.T) { node: &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: "test-node", - Labels: map[string]string{"test-node-topologyKey": "topologyDomain"}, + Labels: nodeSelector, }, }, - wantMatched: false, wantErr: fmt.Errorf("pod %s nodeSelector is missing key: %s", "test-jobset-replicated-job-1-test-job-0-1", "test-node-topologyKey"), }, @@ -151,7 +149,7 @@ func TestValidatePodPlacements(t *testing.T) { AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), podList: &corev1.PodList{ Items: []corev1.Pod{ - leaderPodWrapper.NodeSelector(map[string]string{"test-node-topologyKey": "topologyDomain"}). + leaderPodWrapper.NodeSelector(nodeSelector). AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), podWrapper.NodeSelector(map[string]string{"test-node-topologyKey": "topologyDomain1"}).Obj(), }, @@ -159,10 +157,9 @@ func TestValidatePodPlacements(t *testing.T) { node: &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: "test-node", - Labels: map[string]string{"test-node-topologyKey": "topologyDomain"}, + Labels: nodeSelector, }, }, - wantMatched: false, wantErr: fmt.Errorf("follower topology %q != leader topology %q", "topologyDomain1", "topologyDomain"), }, @@ -172,7 +169,7 @@ func TestValidatePodPlacements(t *testing.T) { AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), podList: &corev1.PodList{ Items: []corev1.Pod{ - leaderPodWrapper.NodeSelector(map[string]string{"test-node-topologyKey": "topologyDomain"}). + leaderPodWrapper.NodeSelector(nodeSelector). AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), podWrapper.NodeSelector(map[string]string{"test-node-topologyKey": "topologyDomain1"}).Obj(), }, @@ -180,10 +177,9 @@ func TestValidatePodPlacements(t *testing.T) { node: &corev1.Node{ ObjectMeta: metav1.ObjectMeta{ Name: "test-node", - Labels: map[string]string{"test-node-topologyKey": "topologyDomain"}, + Labels: nodeSelector, }, }, - wantMatched: false, forceClientErr: true, wantErr: errors.Join(errors.New("example error")), }, @@ -298,7 +294,7 @@ func TestDeleteFollowerPods(t *testing.T) { Type: corev1.DisruptionTarget, Status: corev1.ConditionFalse, Reason: constants.ExclusivePlacementViolationReason, - LastTransitionTime: metav1.Now().Rfc3339Copy(), + LastTransitionTime: metav1.Now(), Message: constants.ExclusivePlacementViolationMessage, }, }).Obj(), @@ -336,7 +332,8 @@ func TestDeleteFollowerPods(t *testing.T) { if tc.forceClientErr || pod == nil { return errors.Join(errors.New("example error")) } - // This is to solve the problem that the timestamps do not match. There will be a slight gap. + // This is to solve the problem that the timestamps do not match. + // There will be a slight gap. pod.Status.Conditions[0].LastTransitionTime = tc.wantPodsDeleted[0].Status.Conditions[0].LastTransitionTime gotPodsDeleted = append(gotPodsDeleted, *pod) From cc22f125e3105b20ff8d249e562fd21647815c0a Mon Sep 17 00:00:00 2001 From: googs1025 Date: Tue, 23 Apr 2024 09:15:28 +0800 Subject: [PATCH 3/8] ut test for pod controller: Reduce duplicate code --- pkg/controllers/pod_controller_test.go | 95 ++++++++++++-------------- 1 file changed, 43 insertions(+), 52 deletions(-) diff --git a/pkg/controllers/pod_controller_test.go b/pkg/controllers/pod_controller_test.go index 371251764..80295d011 100644 --- a/pkg/controllers/pod_controller_test.go +++ b/pkg/controllers/pod_controller_test.go @@ -48,15 +48,16 @@ func TestValidatePodPlacements(t *testing.T) { podName: "test-jobset-replicated-job-1-test-job-0-0", ns: ns, nodeName: "test-node", - jobIdx: 0}) - podWrapper = makePod(&makePodArgs{ + jobIdx: 0}).AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0") + followerPodWrapper = makePod(&makePodArgs{ jobSetName: jobSetName, replicatedJobName: "replicated-job-1", jobName: "test-jobset-replicated-job-1-test-job-0", podName: "test-jobset-replicated-job-1-test-job-0-1", ns: ns, jobIdx: 0}) - nodeSelector = map[string]string{"test-node-topologyKey": "topologyDomain"} + testTopologyKey = "test-node-topologyKey" + nodeSelector = map[string]string{testTopologyKey: "topologyDomain"} ) tests := []struct { name string @@ -68,13 +69,12 @@ func TestValidatePodPlacements(t *testing.T) { wantMatched bool }{ { - name: "topology node label not found", - leaderPod: leaderPodWrapper.AddAnnotation(jobset.ExclusiveKey, "test-node-topologyKey"). - AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + name: "topology node label not found", + leaderPod: leaderPodWrapper.AddAnnotation(jobset.ExclusiveKey, testTopologyKey).Obj(), podList: &corev1.PodList{ Items: []corev1.Pod{ - leaderPodWrapper.AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), - podWrapper.NodeSelector(nodeSelector).Obj(), + leaderPodWrapper.Obj(), + followerPodWrapper.NodeSelector(nodeSelector).Obj(), }, }, node: &corev1.Node{ @@ -82,17 +82,15 @@ func TestValidatePodPlacements(t *testing.T) { Name: "test-node", }, }, - wantErr: fmt.Errorf("node does not have topology label: %s", "test-node-topologyKey"), + wantErr: fmt.Errorf("node does not have topology label: %s", testTopologyKey), }, { - name: "valid pod placements with matched", - leaderPod: leaderPodWrapper.AddAnnotation(jobset.ExclusiveKey, "test-node-topologyKey"). - AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + name: "valid pod placements", + leaderPod: leaderPodWrapper.AddAnnotation(jobset.ExclusiveKey, testTopologyKey).Obj(), podList: &corev1.PodList{ Items: []corev1.Pod{ - leaderPodWrapper.NodeSelector(nodeSelector). - AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), - podWrapper.NodeSelector(nodeSelector).Obj(), + leaderPodWrapper.NodeSelector(nodeSelector).Obj(), + followerPodWrapper.NodeSelector(nodeSelector).Obj(), }, }, node: &corev1.Node{ @@ -104,15 +102,14 @@ func TestValidatePodPlacements(t *testing.T) { wantMatched: true, }, { - name: "follower pod nodeSelector is nil", - leaderPod: leaderPodWrapper.AddAnnotation(jobset.ExclusiveKey, "test-node-topologyKey"). - AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + name: "follower pod nodeSelector is nil", + leaderPod: leaderPodWrapper.AddAnnotation(jobset.ExclusiveKey, testTopologyKey).Obj(), podList: &corev1.PodList{ Items: []corev1.Pod{ leaderPodWrapper.NodeSelector(map[string]string{ - "test-node-topologyKey": "topologyDomain", - }).AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), - podWrapper.NodeSelector(nil).Obj(), + testTopologyKey: "topologyDomain", + }).Obj(), + followerPodWrapper.NodeSelector(nil).Obj(), }, }, node: &corev1.Node{ @@ -121,17 +118,15 @@ func TestValidatePodPlacements(t *testing.T) { Labels: nodeSelector, }, }, - wantMatched: false, - wantErr: fmt.Errorf("pod %s nodeSelector is nil", "test-jobset-replicated-job-1-test-job-0-1"), + wantErr: fmt.Errorf("pod %s nodeSelector is nil", "test-jobset-replicated-job-1-test-job-0-1"), }, { - name: "valid pod placements with pod nodeSelector is empty", - leaderPod: leaderPodWrapper.AddAnnotation(jobset.ExclusiveKey, "test-node-topologyKey"). - AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + name: "follower pod nodeSelector is empty", + leaderPod: leaderPodWrapper.AddAnnotation(jobset.ExclusiveKey, testTopologyKey).Obj(), podList: &corev1.PodList{ Items: []corev1.Pod{ - leaderPodWrapper.AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), - podWrapper.NodeSelector(map[string]string{}).Obj(), + leaderPodWrapper.Obj(), + followerPodWrapper.NodeSelector(map[string]string{}).Obj(), }, }, node: &corev1.Node{ @@ -141,17 +136,15 @@ func TestValidatePodPlacements(t *testing.T) { }, }, wantErr: fmt.Errorf("pod %s nodeSelector is missing key: %s", - "test-jobset-replicated-job-1-test-job-0-1", "test-node-topologyKey"), + "test-jobset-replicated-job-1-test-job-0-1", testTopologyKey), }, { - name: "valid pod placements with followerTopology != leaderTopology", - leaderPod: leaderPodWrapper.AddAnnotation(jobset.ExclusiveKey, "test-node-topologyKey"). - AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + name: "followerTopology != leaderTopology", + leaderPod: leaderPodWrapper.Obj(), podList: &corev1.PodList{ Items: []corev1.Pod{ - leaderPodWrapper.NodeSelector(nodeSelector). - AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), - podWrapper.NodeSelector(map[string]string{"test-node-topologyKey": "topologyDomain1"}).Obj(), + leaderPodWrapper.NodeSelector(nodeSelector).Obj(), + followerPodWrapper.NodeSelector(map[string]string{testTopologyKey: "topologyDomain1"}).Obj(), }, }, node: &corev1.Node{ @@ -164,14 +157,12 @@ func TestValidatePodPlacements(t *testing.T) { "topologyDomain1", "topologyDomain"), }, { - name: "valid pod placements with get node error", - leaderPod: leaderPodWrapper.AddAnnotation(jobset.ExclusiveKey, "test-node-topologyKey"). - AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + name: "get node error", + leaderPod: leaderPodWrapper.AddAnnotation(jobset.ExclusiveKey, testTopologyKey).Obj(), podList: &corev1.PodList{ Items: []corev1.Pod{ - leaderPodWrapper.NodeSelector(nodeSelector). - AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), - podWrapper.NodeSelector(map[string]string{"test-node-topologyKey": "topologyDomain1"}).Obj(), + leaderPodWrapper.NodeSelector(nodeSelector).Obj(), + followerPodWrapper.NodeSelector(map[string]string{testTopologyKey: "topologyDomain"}).Obj(), }, }, node: &corev1.Node{ @@ -196,6 +187,7 @@ func TestValidatePodPlacements(t *testing.T) { if tc.forceClientErr || node == nil { return errors.Join(errors.New("example error")) } + // Set returned node value to be the node defined in the test case. *node = *tc.node return nil }}) @@ -222,7 +214,8 @@ func TestDeleteFollowerPods(t *testing.T) { podName: "test-jobset-replicated-job-1-test-job-0-0", ns: ns, nodeName: "test-node", - jobIdx: 0}) + jobIdx: 0}). + AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0") followerPodWrapper = makePod(&makePodArgs{ jobSetName: jobSetName, replicatedJobName: "replicated-job-1", @@ -248,7 +241,7 @@ func TestDeleteFollowerPods(t *testing.T) { { name: "delete follower pods", pods: []corev1.Pod{ - leaderPodWrapper.AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + leaderPodWrapper.Obj(), followerPodWrapper.Obj(), }, wantPodsDeleted: []corev1.Pod{ @@ -265,7 +258,7 @@ func TestDeleteFollowerPods(t *testing.T) { { name: "delete follower pods with pod conditions status is false", pods: []corev1.Pod{ - leaderPodWrapper.AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + leaderPodWrapper.Obj(), followerPodWrapper.SetConditions([]corev1.PodCondition{{ Type: corev1.DisruptionTarget, Status: corev1.ConditionFalse, @@ -289,7 +282,7 @@ func TestDeleteFollowerPods(t *testing.T) { { name: "delete follower pods with update pod status error", pods: []corev1.Pod{ - leaderPodWrapper.AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + leaderPodWrapper.Obj(), followerPodWrapper.SetConditions([]corev1.PodCondition{{ Type: corev1.DisruptionTarget, Status: corev1.ConditionFalse, @@ -299,14 +292,13 @@ func TestDeleteFollowerPods(t *testing.T) { }, }).Obj(), }, - wantPodsDeleted: nil, - forceClientErr: true, - wantErr: errors.Join(errors.Join(errors.New("example error"))), + forceClientErr: true, + wantErr: errors.Join(errors.Join(errors.New("example error"))), }, { name: "delete follower pods with delete error", pods: []corev1.Pod{ - leaderPodWrapper.AddAnnotation(batchv1.JobCompletionIndexAnnotation, "0").Obj(), + leaderPodWrapper.Obj(), followerPodWrapper.SetConditions([]corev1.PodCondition{{ Type: corev1.DisruptionTarget, Status: corev1.ConditionTrue, @@ -314,9 +306,8 @@ func TestDeleteFollowerPods(t *testing.T) { Message: constants.ExclusivePlacementViolationMessage}, }).Obj(), }, - wantPodsDeleted: nil, - forceClientErr: true, - wantErr: errors.Join(errors.Join(errors.New("example error"))), + forceClientErr: true, + wantErr: errors.Join(errors.Join(errors.New("example error"))), }, } for _, tc := range tests { From 0b48d795afc3090dbcc107ebd548769a9090b6f1 Mon Sep 17 00:00:00 2001 From: googs1025 Date: Tue, 23 Apr 2024 10:35:38 +0800 Subject: [PATCH 4/8] ut: add annotation for followerPod --- pkg/controllers/pod_controller_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/controllers/pod_controller_test.go b/pkg/controllers/pod_controller_test.go index 80295d011..f81ce4546 100644 --- a/pkg/controllers/pod_controller_test.go +++ b/pkg/controllers/pod_controller_test.go @@ -55,7 +55,7 @@ func TestValidatePodPlacements(t *testing.T) { jobName: "test-jobset-replicated-job-1-test-job-0", podName: "test-jobset-replicated-job-1-test-job-0-1", ns: ns, - jobIdx: 0}) + jobIdx: 0}).AddAnnotation(batchv1.JobCompletionIndexAnnotation, "1") testTopologyKey = "test-node-topologyKey" nodeSelector = map[string]string{testTopologyKey: "topologyDomain"} ) @@ -222,14 +222,14 @@ func TestDeleteFollowerPods(t *testing.T) { jobName: "test-jobset-replicated-job-1-test-job-0", podName: "test-jobset-replicated-job-1-test-job-0-1", ns: ns, - jobIdx: 0}) + jobIdx: 0}).AddAnnotation(batchv1.JobCompletionIndexAnnotation, "1") wantPodWrapper = makePod(&makePodArgs{ jobSetName: jobSetName, replicatedJobName: "replicated-job-1", jobName: "test-jobset-replicated-job-1-test-job-0", podName: "test-jobset-replicated-job-1-test-job-0-1", ns: ns, - jobIdx: 0}) + jobIdx: 0}).AddAnnotation(batchv1.JobCompletionIndexAnnotation, "1") ) tests := []struct { name string From 7ff9a5a61542fa42b129ca377cbca144f8937fcc Mon Sep 17 00:00:00 2001 From: googs1025 Date: Wed, 24 Apr 2024 08:36:07 +0800 Subject: [PATCH 5/8] test: fix error type mismatch --- pkg/controllers/pod_controller_test.go | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/pkg/controllers/pod_controller_test.go b/pkg/controllers/pod_controller_test.go index f81ce4546..b87f333ca 100644 --- a/pkg/controllers/pod_controller_test.go +++ b/pkg/controllers/pod_controller_test.go @@ -172,7 +172,7 @@ func TestValidatePodPlacements(t *testing.T) { }, }, forceClientErr: true, - wantErr: errors.Join(errors.New("example error")), + wantErr: errors.New("example error"), }, } for _, tc := range tests { @@ -185,7 +185,7 @@ func TestValidatePodPlacements(t *testing.T) { return nil } if tc.forceClientErr || node == nil { - return errors.Join(errors.New("example error")) + return errors.New("example error") } // Set returned node value to be the node defined in the test case. *node = *tc.node @@ -197,7 +197,9 @@ func TestValidatePodPlacements(t *testing.T) { Record: fc.record, } gotMatched, gotErr := r.validatePodPlacements(context.Background(), &tc.leaderPod, tc.podList) - assert.Equal(t, tc.wantErr, gotErr) + if tc.wantErr != nil && gotErr != nil { + assert.Equal(t, tc.wantErr.Error(), gotErr.Error()) + } assert.Equal(t, tc.wantMatched, gotMatched) }) } @@ -293,7 +295,7 @@ func TestDeleteFollowerPods(t *testing.T) { }).Obj(), }, forceClientErr: true, - wantErr: errors.Join(errors.Join(errors.New("example error"))), + wantErr: errors.New("example error"), }, { name: "delete follower pods with delete error", @@ -307,7 +309,7 @@ func TestDeleteFollowerPods(t *testing.T) { }).Obj(), }, forceClientErr: true, - wantErr: errors.Join(errors.Join(errors.New("example error"))), + wantErr: errors.New("example error"), }, } for _, tc := range tests { @@ -321,7 +323,7 @@ func TestDeleteFollowerPods(t *testing.T) { return nil } if tc.forceClientErr || pod == nil { - return errors.Join(errors.New("example error")) + return errors.New("example error") } // This is to solve the problem that the timestamps do not match. // There will be a slight gap. @@ -337,7 +339,7 @@ func TestDeleteFollowerPods(t *testing.T) { return nil } if tc.forceClientErr || pod == nil { - return errors.Join(errors.New("example error")) + return errors.New("example error") } return nil }, @@ -349,7 +351,9 @@ func TestDeleteFollowerPods(t *testing.T) { } gotErr := r.deleteFollowerPods(context.Background(), tc.pods) - assert.Equal(t, gotErr, tc.wantErr) + if tc.wantErr != nil && gotErr != nil { + assert.Equal(t, tc.wantErr.Error(), gotErr.Error()) + } sort.Slice(gotPodsDeleted, func(i, j int) bool { return gotPodsDeleted[i].Name < gotPodsDeleted[j].Name }) @@ -357,7 +361,7 @@ func TestDeleteFollowerPods(t *testing.T) { return tc.wantPodsDeleted[i].Name < tc.wantPodsDeleted[j].Name }) - if !assert.Equal(t, gotPodsDeleted, tc.wantPodsDeleted) { + if !assert.Equal(t, tc.wantPodsDeleted, gotPodsDeleted) { t.Errorf("deleteFollowerPods() did not make the expected pod deletion calls") } }) From 56ae23372e975318c6743a80edf0004b96289a44 Mon Sep 17 00:00:00 2001 From: googs1025 Date: Wed, 24 Apr 2024 12:07:09 +0800 Subject: [PATCH 6/8] test: use delete count to test DeleteFollowerPods method --- pkg/controllers/pod_controller_test.go | 57 +++++--------------------- pkg/util/testing/wrappers.go | 6 --- 2 files changed, 10 insertions(+), 53 deletions(-) diff --git a/pkg/controllers/pod_controller_test.go b/pkg/controllers/pod_controller_test.go index b87f333ca..e05c2ad6c 100644 --- a/pkg/controllers/pod_controller_test.go +++ b/pkg/controllers/pod_controller_test.go @@ -17,7 +17,6 @@ import ( "context" "errors" "fmt" - "sort" "strconv" "testing" @@ -225,20 +224,13 @@ func TestDeleteFollowerPods(t *testing.T) { podName: "test-jobset-replicated-job-1-test-job-0-1", ns: ns, jobIdx: 0}).AddAnnotation(batchv1.JobCompletionIndexAnnotation, "1") - wantPodWrapper = makePod(&makePodArgs{ - jobSetName: jobSetName, - replicatedJobName: "replicated-job-1", - jobName: "test-jobset-replicated-job-1-test-job-0", - podName: "test-jobset-replicated-job-1-test-job-0-1", - ns: ns, - jobIdx: 0}).AddAnnotation(batchv1.JobCompletionIndexAnnotation, "1") ) tests := []struct { - name string - pods []corev1.Pod - wantPodsDeleted []corev1.Pod - wantErr error - forceClientErr bool + name string + pods []corev1.Pod + wantPodsDeletedCount int + wantErr error + forceClientErr bool }{ { name: "delete follower pods", @@ -246,16 +238,7 @@ func TestDeleteFollowerPods(t *testing.T) { leaderPodWrapper.Obj(), followerPodWrapper.Obj(), }, - wantPodsDeleted: []corev1.Pod{ - wantPodWrapper.ResourceVersion("999").SetConditions([]corev1.PodCondition{{ - Type: corev1.DisruptionTarget, - Status: corev1.ConditionTrue, - Reason: constants.ExclusivePlacementViolationReason, - LastTransitionTime: metav1.Now(), - Message: constants.ExclusivePlacementViolationMessage, - }, - }).Obj(), - }, + wantPodsDeletedCount: 1, }, { name: "delete follower pods with pod conditions status is false", @@ -270,16 +253,7 @@ func TestDeleteFollowerPods(t *testing.T) { }, }).Obj(), }, - wantPodsDeleted: []corev1.Pod{ - wantPodWrapper.ResourceVersion("999").SetConditions([]corev1.PodCondition{{ - Type: corev1.DisruptionTarget, - Status: corev1.ConditionTrue, - Reason: constants.ExclusivePlacementViolationReason, - LastTransitionTime: metav1.Now(), - Message: constants.ExclusivePlacementViolationMessage, - }, - }).Obj(), - }, + wantPodsDeletedCount: 1, }, { name: "delete follower pods with update pod status error", @@ -314,7 +288,7 @@ func TestDeleteFollowerPods(t *testing.T) { } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - var gotPodsDeleted []corev1.Pod + var deletedCount int fc := makeFakeClient(interceptor.Funcs{ Delete: func(ctx context.Context, client client.WithWatch, obj client.Object, opts ...client.DeleteOption) error { @@ -325,11 +299,7 @@ func TestDeleteFollowerPods(t *testing.T) { if tc.forceClientErr || pod == nil { return errors.New("example error") } - // This is to solve the problem that the timestamps do not match. - // There will be a slight gap. - pod.Status.Conditions[0].LastTransitionTime = - tc.wantPodsDeleted[0].Status.Conditions[0].LastTransitionTime - gotPodsDeleted = append(gotPodsDeleted, *pod) + deletedCount++ return nil }, SubResourceUpdate: func(ctx context.Context, client client.Client, @@ -354,14 +324,7 @@ func TestDeleteFollowerPods(t *testing.T) { if tc.wantErr != nil && gotErr != nil { assert.Equal(t, tc.wantErr.Error(), gotErr.Error()) } - sort.Slice(gotPodsDeleted, func(i, j int) bool { - return gotPodsDeleted[i].Name < gotPodsDeleted[j].Name - }) - sort.Slice(tc.wantPodsDeleted, func(i, j int) bool { - return tc.wantPodsDeleted[i].Name < tc.wantPodsDeleted[j].Name - }) - - if !assert.Equal(t, tc.wantPodsDeleted, gotPodsDeleted) { + if tc.wantPodsDeletedCount != deletedCount { t.Errorf("deleteFollowerPods() did not make the expected pod deletion calls") } }) diff --git a/pkg/util/testing/wrappers.go b/pkg/util/testing/wrappers.go index 77a99cd7d..9f94478c2 100644 --- a/pkg/util/testing/wrappers.go +++ b/pkg/util/testing/wrappers.go @@ -412,12 +412,6 @@ func (p *PodWrapper) Labels(labels map[string]string) *PodWrapper { return p } -// ResourceVersion sets the pod.metadata.resourceVersion. -func (p *PodWrapper) ResourceVersion(resourceVersion string) *PodWrapper { - p.ObjectMeta.ResourceVersion = resourceVersion - return p -} - // SetConditions sets the value of the pod.status.conditions. func (p *PodWrapper) SetConditions(conditions []corev1.PodCondition) *PodWrapper { p.Status.Conditions = conditions From 1ad62c31a32301a269760a5bcf4a68bed2180134 Mon Sep 17 00:00:00 2001 From: googs1025 Date: Thu, 25 Apr 2024 07:17:57 +0800 Subject: [PATCH 7/8] cleanup leaderPod NodeSelector --- pkg/controllers/pod_controller_test.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/pkg/controllers/pod_controller_test.go b/pkg/controllers/pod_controller_test.go index e05c2ad6c..1f95f3c21 100644 --- a/pkg/controllers/pod_controller_test.go +++ b/pkg/controllers/pod_controller_test.go @@ -88,7 +88,7 @@ func TestValidatePodPlacements(t *testing.T) { leaderPod: leaderPodWrapper.AddAnnotation(jobset.ExclusiveKey, testTopologyKey).Obj(), podList: &corev1.PodList{ Items: []corev1.Pod{ - leaderPodWrapper.NodeSelector(nodeSelector).Obj(), + leaderPodWrapper.Obj(), followerPodWrapper.NodeSelector(nodeSelector).Obj(), }, }, @@ -105,9 +105,7 @@ func TestValidatePodPlacements(t *testing.T) { leaderPod: leaderPodWrapper.AddAnnotation(jobset.ExclusiveKey, testTopologyKey).Obj(), podList: &corev1.PodList{ Items: []corev1.Pod{ - leaderPodWrapper.NodeSelector(map[string]string{ - testTopologyKey: "topologyDomain", - }).Obj(), + leaderPodWrapper.Obj(), followerPodWrapper.NodeSelector(nil).Obj(), }, }, @@ -142,7 +140,7 @@ func TestValidatePodPlacements(t *testing.T) { leaderPod: leaderPodWrapper.Obj(), podList: &corev1.PodList{ Items: []corev1.Pod{ - leaderPodWrapper.NodeSelector(nodeSelector).Obj(), + leaderPodWrapper.Obj(), followerPodWrapper.NodeSelector(map[string]string{testTopologyKey: "topologyDomain1"}).Obj(), }, }, @@ -160,7 +158,7 @@ func TestValidatePodPlacements(t *testing.T) { leaderPod: leaderPodWrapper.AddAnnotation(jobset.ExclusiveKey, testTopologyKey).Obj(), podList: &corev1.PodList{ Items: []corev1.Pod{ - leaderPodWrapper.NodeSelector(nodeSelector).Obj(), + leaderPodWrapper.Obj(), followerPodWrapper.NodeSelector(map[string]string{testTopologyKey: "topologyDomain"}).Obj(), }, }, From 9e7e98bf0e90d1c3230bbd1b33b5d27afc74938f Mon Sep 17 00:00:00 2001 From: googs1025 Date: Thu, 25 Apr 2024 12:54:14 +0800 Subject: [PATCH 8/8] cleanup useless struct --- pkg/controllers/pod_controller_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pkg/controllers/pod_controller_test.go b/pkg/controllers/pod_controller_test.go index 1f95f3c21..43b886625 100644 --- a/pkg/controllers/pod_controller_test.go +++ b/pkg/controllers/pod_controller_test.go @@ -162,12 +162,6 @@ func TestValidatePodPlacements(t *testing.T) { followerPodWrapper.NodeSelector(map[string]string{testTopologyKey: "topologyDomain"}).Obj(), }, }, - node: &corev1.Node{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-node", - Labels: nodeSelector, - }, - }, forceClientErr: true, wantErr: errors.New("example error"), },