Skip to content

Commit

Permalink
Set timeout and interval to all Eventually tests (#1846)
Browse files Browse the repository at this point in the history 
* Commonize timeout and interval for Eventually tests

Signed-off-by: Yuki Iwai <[email protected]>

* Set timeout and interval to all Eventually tests

Signed-off-by: Yuki Iwai <[email protected]>

---------

Signed-off-by: Yuki Iwai <[email protected]>
  • Loading branch information
@tenzen-y
tenzen-y authored Jul 4, 2023
1 parent 40454af commit bb628c4
Show file tree
Hide file tree
Showing 9 changed files with 74 additions and 102 deletions.
50 changes: 22 additions & 28 deletions pkg/controller.v1/mpi/mpijob_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"context"
"fmt"
"strings"
"time"

common "github.com/kubeflow/training-operator/pkg/apis/kubeflow.org/v1"
. "github.com/onsi/ginkgo/v2"
Expand All @@ -31,6 +30,7 @@ import (
ctrl "sigs.k8s.io/controller-runtime"

kubeflowv1 "github.com/kubeflow/training-operator/pkg/apis/kubeflow.org/v1"
"github.com/kubeflow/training-operator/pkg/util/testutil"
)

const (
Expand Down Expand Up @@ -129,12 +129,6 @@ func newMPIJobWithLauncher(name string, replicas *int32, pusPerReplica int64, re
}

var _ = Describe("MPIJob controller", func() {
// Define utility constants for object names and testing timeouts/durations and intervals.
const (
timeout = 10 * time.Second
interval = 1000 * time.Millisecond
)

Context("Test launcher is GPU launcher", func() {
It("Should pass GPU Launcher verification", func() {
By("By creating MPIJobs with various resource configuration")
Expand Down Expand Up @@ -194,7 +188,7 @@ var _ = Describe("MPIJob controller", func() {
}
launcherCreated.Status.Phase = corev1.PodSucceeded
return testK8sClient.Status().Update(ctx, launcherCreated)
}, timeout, interval).Should(BeNil())
}, testutil.Timeout, testutil.Interval).Should(BeNil())

created := &kubeflowv1.MPIJob{}
launcherStatus := &common.ReplicaStatus{
Expand All @@ -208,7 +202,7 @@ var _ = Describe("MPIJob controller", func() {
return false
}
return ReplicaStatusMatch(created.Status.ReplicaStatuses, kubeflowv1.MPIJobReplicaTypeLauncher, launcherStatus)
}, timeout, interval).Should(BeTrue())
}, testutil.Timeout, testutil.Interval).Should(BeTrue())
})
})

Expand Down Expand Up @@ -236,7 +230,7 @@ var _ = Describe("MPIJob controller", func() {
}
launcherCreated.Status.Phase = corev1.PodFailed
return testK8sClient.Status().Update(ctx, launcherCreated)
}, timeout, interval).Should(BeNil())
}, testutil.Timeout, testutil.Interval).Should(BeNil())

launcherStatus := &common.ReplicaStatus{
Active: 0,
Expand All @@ -250,7 +244,7 @@ var _ = Describe("MPIJob controller", func() {
return false
}
return ReplicaStatusMatch(created.Status.ReplicaStatuses, kubeflowv1.MPIJobReplicaTypeLauncher, launcherStatus)
}, timeout, interval).Should(BeTrue())
}, testutil.Timeout, testutil.Interval).Should(BeTrue())
})
})

Expand Down Expand Up @@ -280,7 +274,7 @@ var _ = Describe("MPIJob controller", func() {
}
launcherCreated.Status.Phase = corev1.PodSucceeded
return testK8sClient.Status().Update(ctx, launcherCreated)
}, timeout, interval).Should(BeNil())
}, testutil.Timeout, testutil.Interval).Should(BeNil())

created := &kubeflowv1.MPIJob{}
launcherStatus := &common.ReplicaStatus{
Expand All @@ -294,7 +288,7 @@ var _ = Describe("MPIJob controller", func() {
return false
}
return ReplicaStatusMatch(created.Status.ReplicaStatuses, kubeflowv1.MPIJobReplicaTypeWorker, launcherStatus)
}, timeout, interval).Should(BeTrue())
}, testutil.Timeout, testutil.Interval).Should(BeTrue())
})
})

Expand Down Expand Up @@ -324,7 +318,7 @@ var _ = Describe("MPIJob controller", func() {
}
launcherCreated.Status.Phase = corev1.PodRunning
return testK8sClient.Status().Update(ctx, launcherCreated)
}, timeout, interval).Should(BeNil())
}, testutil.Timeout, testutil.Interval).Should(BeNil())

for i := 0; i < int(replicas); i++ {
name := fmt.Sprintf("%s-%d", mpiJob.Name+workerSuffix, i)
Expand All @@ -340,7 +334,7 @@ var _ = Describe("MPIJob controller", func() {
}
workerCreated.Status.Phase = corev1.PodPending
return testK8sClient.Status().Update(ctx, workerCreated)
}, timeout, interval).Should(BeNil())
}, testutil.Timeout, testutil.Interval).Should(BeNil())
}

key := types.NamespacedName{
Expand All @@ -366,7 +360,7 @@ var _ = Describe("MPIJob controller", func() {
return ReplicaStatusMatch(created.Status.ReplicaStatuses, kubeflowv1.MPIJobReplicaTypeLauncher,
launcherStatus) && ReplicaStatusMatch(created.Status.ReplicaStatuses, kubeflowv1.MPIJobReplicaTypeWorker,
workerStatus)
}, timeout, interval).Should(BeTrue())
}, testutil.Timeout, testutil.Interval).Should(BeTrue())
})
})

Expand Down Expand Up @@ -396,7 +390,7 @@ var _ = Describe("MPIJob controller", func() {
}
launcherCreated.Status.Phase = corev1.PodRunning
return testK8sClient.Status().Update(ctx, launcherCreated)
}, timeout, interval).Should(BeNil())
}, testutil.Timeout, testutil.Interval).Should(BeNil())

for i := 0; i < int(replicas); i++ {
name := fmt.Sprintf("%s-%d", mpiJob.Name+workerSuffix, i)
Expand All @@ -412,7 +406,7 @@ var _ = Describe("MPIJob controller", func() {
}
workerCreated.Status.Phase = corev1.PodRunning
return testK8sClient.Status().Update(ctx, workerCreated)
}, timeout, interval).Should(BeNil())
}, testutil.Timeout, testutil.Interval).Should(BeNil())
}

key := types.NamespacedName{
Expand All @@ -438,7 +432,7 @@ var _ = Describe("MPIJob controller", func() {
return ReplicaStatusMatch(created.Status.ReplicaStatuses, kubeflowv1.MPIJobReplicaTypeLauncher,
launcherStatus) && ReplicaStatusMatch(created.Status.ReplicaStatuses, kubeflowv1.MPIJobReplicaTypeWorker,
workerStatus)
}, timeout, interval).Should(BeTrue())
}, testutil.Timeout, testutil.Interval).Should(BeTrue())
})
})

Expand Down Expand Up @@ -470,7 +464,7 @@ var _ = Describe("MPIJob controller", func() {
}
workerCreated.Status.Phase = corev1.PodRunning
return testK8sClient.Status().Update(ctx, workerCreated)
}, timeout, interval).Should(BeNil())
}, testutil.Timeout, testutil.Interval).Should(BeNil())
}

launcherKey := types.NamespacedName{
Expand All @@ -481,7 +475,7 @@ var _ = Describe("MPIJob controller", func() {
Eventually(func() bool {
err := testK8sClient.Get(ctx, launcherKey, launcher)
return err != nil
}, timeout, interval).Should(BeTrue())
}, testutil.Timeout, testutil.Interval).Should(BeTrue())

key := types.NamespacedName{
Namespace: metav1.NamespaceDefault,
Expand All @@ -506,7 +500,7 @@ var _ = Describe("MPIJob controller", func() {
return ReplicaStatusMatch(created.Status.ReplicaStatuses, kubeflowv1.MPIJobReplicaTypeLauncher,
launcherStatus) && ReplicaStatusMatch(created.Status.ReplicaStatuses, kubeflowv1.MPIJobReplicaTypeWorker,
workerStatus)
}, timeout, interval).Should(BeTrue())
}, testutil.Timeout, testutil.Interval).Should(BeTrue())
})
})

Expand Down Expand Up @@ -552,7 +546,7 @@ var _ = Describe("MPIJob controller", func() {
Eventually(func() error {
_, err := reconciler.Reconcile(ctx, req)
return err
}, timeout, interval).Should(MatchError(expectedErr))
}, testutil.Timeout, testutil.Interval).Should(MatchError(expectedErr))
})
})

Expand Down Expand Up @@ -585,7 +579,7 @@ var _ = Describe("MPIJob controller", func() {
Eventually(func() error {
_, err := reconciler.Reconcile(ctx, req)
return err
}, timeout, interval).Should(MatchError(expectedErr))
}, testutil.Timeout, testutil.Interval).Should(MatchError(expectedErr))
})
})

Expand Down Expand Up @@ -615,7 +609,7 @@ var _ = Describe("MPIJob controller", func() {
Eventually(func() error {
_, err := reconciler.Reconcile(ctx, req)
return err
}, timeout, interval).Should(MatchError(expectedErr))
}, testutil.Timeout, testutil.Interval).Should(MatchError(expectedErr))
})
})

Expand Down Expand Up @@ -645,7 +639,7 @@ var _ = Describe("MPIJob controller", func() {
Eventually(func() error {
_, err := reconciler.Reconcile(ctx, req)
return err
}, timeout, interval).Should(MatchError(expectedErr))
}, testutil.Timeout, testutil.Interval).Should(MatchError(expectedErr))
})
})

Expand Down Expand Up @@ -675,7 +669,7 @@ var _ = Describe("MPIJob controller", func() {
Eventually(func() error {
_, err := reconciler.Reconcile(ctx, req)
return err
}, timeout, interval).Should(MatchError(expectedErr))
}, testutil.Timeout, testutil.Interval).Should(MatchError(expectedErr))
})
})

Expand Down Expand Up @@ -705,7 +699,7 @@ var _ = Describe("MPIJob controller", func() {
Eventually(func() error {
_, err := reconciler.Reconcile(ctx, req)
return err
}, timeout, interval).Should(MatchError(expectedErr))
}, testutil.Timeout, testutil.Interval).Should(MatchError(expectedErr))
})
})

Expand Down
12 changes: 5 additions & 7 deletions pkg/controller.v1/paddlepaddle/paddlepaddle_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package paddle
import (
"context"
"fmt"
"time"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand All @@ -28,13 +27,12 @@ import (

commonv1 "github.com/kubeflow/training-operator/pkg/apis/kubeflow.org/v1"
kubeflowv1 "github.com/kubeflow/training-operator/pkg/apis/kubeflow.org/v1"
"github.com/kubeflow/training-operator/pkg/util/testutil"
)

var _ = Describe("PaddleJob controller", func() {
// Define utility constants for object names and testing timeouts/durations and intervals.
const (
timeout = time.Second * 10
interval = time.Millisecond * 250
expectedPort = int32(8080)
)

Expand Down Expand Up @@ -99,20 +97,20 @@ var _ = Describe("PaddleJob controller", func() {
Eventually(func() bool {
err := testK8sClient.Get(ctx, key, created)
return err == nil
}, timeout, interval).Should(BeTrue())
}, testutil.Timeout, testutil.Interval).Should(BeTrue())

masterKey := types.NamespacedName{Name: fmt.Sprintf("%s-master-0", name), Namespace: namespace}
masterPod := &corev1.Pod{}
Eventually(func() bool {
err := testK8sClient.Get(ctx, masterKey, masterPod)
return err == nil
}, timeout, interval).Should(BeTrue())
}, testutil.Timeout, testutil.Interval).Should(BeTrue())

masterSvc := &corev1.Service{}
Eventually(func() bool {
err := testK8sClient.Get(ctx, masterKey, masterSvc)
return err == nil
}, timeout, interval).Should(BeTrue())
}, testutil.Timeout, testutil.Interval).Should(BeTrue())

// Check the pod port.
Expect(masterPod.Spec.Containers[0].Ports).To(ContainElement(corev1.ContainerPort{
Expand Down Expand Up @@ -156,7 +154,7 @@ var _ = Describe("PaddleJob controller", func() {
}
return created.Status.ReplicaStatuses != nil && created.Status.
ReplicaStatuses[kubeflowv1.PaddleJobReplicaTypeMaster].Succeeded == 1
}, timeout, interval).Should(BeTrue())
}, testutil.Timeout, testutil.Interval).Should(BeTrue())
// Check if the job is succeeded.
cond := getCondition(created.Status, commonv1.JobSucceeded)
Expect(cond.Status).To(Equal(corev1.ConditionTrue))
Expand Down
22 changes: 10 additions & 12 deletions pkg/controller.v1/pytorch/pytorchjob_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ package pytorch
import (
"context"
"fmt"
"time"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand All @@ -28,13 +27,12 @@ import (

commonv1 "github.com/kubeflow/training-operator/pkg/apis/kubeflow.org/v1"
kubeflowv1 "github.com/kubeflow/training-operator/pkg/apis/kubeflow.org/v1"
"github.com/kubeflow/training-operator/pkg/util/testutil"
)

var _ = Describe("PyTorchJob controller", func() {
// Define utility constants for object names and testing timeouts/durations and intervals.
// Define utility constants for object names.
const (
timeout = time.Second * 10
interval = time.Millisecond * 250
expectedPort = int32(8080)
)

Expand Down Expand Up @@ -99,20 +97,20 @@ var _ = Describe("PyTorchJob controller", func() {
Eventually(func() bool {
err := testK8sClient.Get(ctx, key, created)
return err == nil
}, timeout, interval).Should(BeTrue())
}, testutil.Timeout, testutil.Interval).Should(BeTrue())

masterKey := types.NamespacedName{Name: fmt.Sprintf("%s-master-0", name), Namespace: namespace}
masterPod := &corev1.Pod{}
Eventually(func() bool {
err := testK8sClient.Get(ctx, masterKey, masterPod)
return err == nil
}, timeout, interval).Should(BeTrue())
}, testutil.Timeout, testutil.Interval).Should(BeTrue())

masterSvc := &corev1.Service{}
Eventually(func() bool {
err := testK8sClient.Get(ctx, masterKey, masterSvc)
return err == nil
}, timeout, interval).Should(BeTrue())
}, testutil.Timeout, testutil.Interval).Should(BeTrue())

// Check the pod port.
Expect(masterPod.Spec.Containers[0].Ports).To(ContainElement(corev1.ContainerPort{
Expand Down Expand Up @@ -159,7 +157,7 @@ var _ = Describe("PyTorchJob controller", func() {
}
return created.Status.ReplicaStatuses != nil && created.Status.
ReplicaStatuses[kubeflowv1.PyTorchJobReplicaTypeMaster].Succeeded == 1
}, timeout, interval).Should(BeTrue())
}, testutil.Timeout, testutil.Interval).Should(BeTrue())
// Check if the job is succeeded.
cond := getCondition(created.Status, commonv1.JobSucceeded)
Expect(cond.Status).To(Equal(corev1.ConditionTrue))
Expand Down Expand Up @@ -222,20 +220,20 @@ var _ = Describe("PyTorchJob controller", func() {
Eventually(func() bool {
err := testK8sClient.Get(ctx, key, created)
return err == nil
}, timeout, interval).Should(BeTrue())
}, testutil.Timeout, testutil.Interval).Should(BeTrue())

workerKey := types.NamespacedName{Name: fmt.Sprintf("%s-worker-0", name), Namespace: namespace}
pod := &corev1.Pod{}
Eventually(func() bool {
err := testK8sClient.Get(ctx, workerKey, pod)
return err == nil
}, timeout, interval).Should(BeTrue())
}, testutil.Timeout, testutil.Interval).Should(BeTrue())

svc := &corev1.Service{}
Eventually(func() bool {
err := testK8sClient.Get(ctx, workerKey, svc)
return err == nil
}, timeout, interval).Should(BeTrue())
}, testutil.Timeout, testutil.Interval).Should(BeTrue())

// Check pod port.
Expect(pod.Spec.Containers[0].Ports).To(ContainElement(corev1.ContainerPort{
Expand Down Expand Up @@ -287,7 +285,7 @@ var _ = Describe("PyTorchJob controller", func() {
}
return created.Status.ReplicaStatuses != nil && created.Status.
ReplicaStatuses[kubeflowv1.PyTorchJobReplicaTypeWorker].Succeeded == 1
}, timeout, interval).Should(BeTrue())
}, testutil.Timeout, testutil.Interval).Should(BeTrue())
// Check if the job is succeeded.
cond := getCondition(created.Status, commonv1.JobSucceeded)
Expect(cond.Status).To(Equal(corev1.ConditionTrue))
Expand Down
Loading

0 comments on commit bb628c4

Please sign in to comment.