remove unused scripts

improve status_test from tensorflow

pkg/apis/mpi/v1/constants.go

@@ -17,8 +17,6 @@ package v1
 import common "github.com/kubeflow/common/pkg/apis/common/v1"
 
 const (
-	// EnvKubeflowNamespace is ENV for kubeflow namespace specified by user.
-	EnvKubeflowNamespace = "KUBEFLOW_NAMESPACE"
 	// DefaultPortName is name of the port used to communicate between Master and Workers.
 	DefaultPortName = "mpi-port"
 	// DefaultContainerName is the name of the MPIJob container.

pkg/common/util/client.go

@@ -15,18 +15,9 @@
 package util
 
 import (
-	"os"
-
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
-func HomeDir() string {
-	if h := os.Getenv("HOME"); h != "" {
-		return h
-	}
-	return os.Getenv("USERPROFILE") // windows
-}
-
 // TODO (Jeffwan@): Find an elegant way to either use delegatingReader or directly use clientss
 // GetDelegatingClientFromClient try to extract client reader from client, client
 // reader reads cluster info from api client.

pkg/common/util/scheduler.go

@@ -33,3 +33,12 @@ func IsGangSchedulerSet(replicas map[commonv1.ReplicaType]*commonv1.ReplicaSpec,
 
 	return false
 }
+
+func GetSchedulerName(replicas map[commonv1.ReplicaType]*commonv1.ReplicaSpec) string {
+	for _, spec := range replicas {
+		if len(spec.Template.Spec.SchedulerName) > 0 {
+			return spec.Template.Spec.SchedulerName
+		}
+	}
+	return ""
+}

pkg/common/util/util.go

@@ -50,11 +50,3 @@ func GetReplicaTypes(specs map[commonv1.ReplicaType]*commonv1.ReplicaSpec) []com
 	}
 	return keys
 }
-func GetSchedulerName(replicas map[commonv1.ReplicaType]*commonv1.ReplicaSpec) string {
-	for _, spec := range replicas {
-		if len(spec.Template.Spec.SchedulerName) > 0 {
-			return spec.Template.Spec.SchedulerName
-		}
-	}
-	return ""
-}

pkg/common/util/v1/testutil/service.go

@@ -69,7 +69,7 @@ func NewServiceList(count int32, job metav1.Object, typ string, refs []metav1.Ow
 	return services
 }
 
-func SetServicesV2(client client.Client, job metav1.Object, typ string, activeWorkerServices int32,
+func SetServices(client client.Client, job metav1.Object, typ string, activeWorkerServices int32,
 	refs []metav1.OwnerReference, basicLabels map[string]string) {
 	ctx := context.Background()
 	for _, svc := range NewServiceList(activeWorkerServices, job, typ, refs) {

pkg/config/config.go

@@ -1,3 +1,17 @@
+// Copyright 2021 The Kubeflow 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 config
 
 // Config is the global configuration for the training operator.

pkg/controller.v1/register_controller.go

@@ -18,6 +18,8 @@ import (
 	"fmt"
 	"strings"
 
+	"sigs.k8s.io/controller-runtime/pkg/manager"
+
 	mpiv1 "github.com/kubeflow/training-operator/pkg/apis/mpi/v1"
 	mxnetv1 "github.com/kubeflow/training-operator/pkg/apis/mxnet/v1"
 	pytorchv1 "github.com/kubeflow/training-operator/pkg/apis/pytorch/v1"
@@ -28,7 +30,6 @@ import (
 	pytorchcontroller "github.com/kubeflow/training-operator/pkg/controller.v1/pytorch"
 	tensorflowcontroller "github.com/kubeflow/training-operator/pkg/controller.v1/tensorflow"
 	xgboostcontroller "github.com/kubeflow/training-operator/pkg/controller.v1/xgboost"
-	"sigs.k8s.io/controller-runtime/pkg/manager"
 )
 
 const ErrTemplateSchemeNotSupported = "scheme %s is not supported yet"

pkg/controller.v1/register_controller_test.go

@@ -0,0 +1,76 @@
+// Copyright 2022 The Kubeflow 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 controller_v1
+
+import (
+	mpiv1 "github.com/kubeflow/training-operator/pkg/apis/mpi/v1"
+	tfv1 "github.com/kubeflow/training-operator/pkg/apis/tensorflow/v1"
+	"testing"
+)
+
+func TestEnabledSchemes(t *testing.T) {
+	testES := EnabledSchemes{}
+
+	if testES.String() != "" {
+		t.Errorf("empty EnabledSchemes converted no-empty string %s", testES.String())
+	}
+
+	if !testES.Empty() {
+		t.Error("Empty method returned false for empty EnabledSchemes")
+	}
+
+	if testES.Set("TFJob") != nil {
+		t.Error("failed to restore TFJob")
+	} else {
+		stored := false
+		for _, kind := range testES {
+			if kind == tfv1.Kind {
+				stored = true
+			}
+		}
+		if !stored {
+			t.Errorf("%s not successfully registered", tfv1.Kind)
+		}
+	}
+
+	if testES.Set("mpijob") != nil {
+		t.Error("failed to restore PyTorchJob(pytorchjob)")
+	} else {
+		stored := false
+		for _, kind := range testES {
+			if kind == mpiv1.Kind {
+				stored = true
+			}
+		}
+		if !stored {
+			t.Errorf("%s not successfully registered", mpiv1.Kind)
+		}
+	}
+
+	dummyJob := "dummyjob"
+	if testES.Set(dummyJob) == nil {
+		t.Errorf("successfully registerd non-supported job %s", dummyJob)
+	}
+
+	if testES.Empty() {
+		t.Error("Empty method returned true for non-empty EnabledSchemes")
+	}
+
+	es2 := EnabledSchemes{}
+	es2.FillAll()
+	if es2.Empty() {
+		t.Error("Empty method returned true for fully registered EnabledSchemes")
+	}
+}

pkg/controller.v1/tensorflow/job_test.go

@@ -268,8 +268,8 @@ var _ = Describe("TFJob controller", func() {
 					tc.pendingPSPods, tc.activePSPods, tc.succeededPSPods, tc.failedPSPods,
 					nil, refs, basicLabels)
 
-				testutil.SetServicesV2(testK8sClient, tc.tfJob, testutil.LabelWorker, tc.activeWorkerServices, refs, basicLabels)
-				testutil.SetServicesV2(testK8sClient, tc.tfJob, testutil.LabelPS, tc.activePSServices, refs, basicLabels)
+				testutil.SetServices(testK8sClient, tc.tfJob, testutil.LabelWorker, tc.activeWorkerServices, refs, basicLabels)
+				testutil.SetServices(testK8sClient, tc.tfJob, testutil.LabelPS, tc.activePSServices, refs, basicLabels)
 
 				podList := &corev1.PodList{}
 				Expect(testK8sClient.List(ctx, podList, listOpt)).Should(Succeed())
@@ -385,8 +385,8 @@ var _ = Describe("TFJob controller", func() {
 					tc.pendingPSPods, tc.activePSPods, tc.succeededPSPods, tc.failedPSPods,
 					nil, refs, basicLabels)
 
-				testutil.SetServicesV2(testK8sClient, tc.tfJob, testutil.LabelWorker, tc.activeWorkerServices, refs, basicLabels)
-				testutil.SetServicesV2(testK8sClient, tc.tfJob, testutil.LabelPS, tc.activePSServices, refs, basicLabels)
+				testutil.SetServices(testK8sClient, tc.tfJob, testutil.LabelWorker, tc.activeWorkerServices, refs, basicLabels)
+				testutil.SetServices(testK8sClient, tc.tfJob, testutil.LabelPS, tc.activePSServices, refs, basicLabels)
 
 				podList := &corev1.PodList{}
 				Expect(testK8sClient.List(ctx, podList, listOpt)).Should(Succeed())
@@ -497,8 +497,8 @@ var _ = Describe("TFJob controller", func() {
 					tc.pendingPSPods, tc.activePSPods, tc.succeededPSPods, tc.failedPSPods,
 					tc.restartCounts, refs, basicLabels)
 
-				testutil.SetServicesV2(testK8sClient, tc.tfJob, testutil.LabelWorker, tc.activeWorkerServices, refs, basicLabels)
-				testutil.SetServicesV2(testK8sClient, tc.tfJob, testutil.LabelPS, tc.activePSServices, refs, basicLabels)
+				testutil.SetServices(testK8sClient, tc.tfJob, testutil.LabelWorker, tc.activeWorkerServices, refs, basicLabels)
+				testutil.SetServices(testK8sClient, tc.tfJob, testutil.LabelPS, tc.activePSServices, refs, basicLabels)
 
 				podList := &corev1.PodList{}
 				Expect(testK8sClient.List(ctx, podList, listOpt)).Should(Succeed())

pkg/controller.v1/tensorflow/status_test.go

@@ -488,6 +488,8 @@ func setStatusForTest(tfJob *tfv1.TFJob, rtype commonv1.ReplicaType, failed, suc
 	default:
 		fmt.Println("wrong type")
 	}
+	Expect(typ).ShouldNot(Equal(""))
+
 	refs := []metav1.OwnerReference{
 		*reconciler.GenOwnerReference(tfJob),
 	}
@@ -500,9 +502,11 @@ func setStatusForTest(tfJob *tfv1.TFJob, rtype commonv1.ReplicaType, failed, suc
 			pod.Labels[k] = v
 		}
 		po := &corev1.Pod{}
-		_ = client.Create(ctx, pod)
+		Expect(client.Create(ctx, pod)).Should(Succeed())
+
 		key := genKeyFromJob(pod)
 		Eventually(func() error {
+			po = &corev1.Pod{}
 			if err := client.Get(ctx, key, po); err != nil {
 				return err
 			}
@@ -511,7 +515,7 @@ func setStatusForTest(tfJob *tfv1.TFJob, rtype commonv1.ReplicaType, failed, suc
 			if worker0Completed == true && rtype == tfv1.TFReplicaTypeWorker && index == 0 {
 				po.Status.ContainerStatuses = []corev1.ContainerStatus{
 					{
-						Name: tfv1.DefaultContainerName,
+						Name: reconciler.GetDefaultContainerName(),
 						State: corev1.ContainerState{
 							Terminated: &corev1.ContainerStateTerminated{
 								ExitCode: int32(0), // exit with 0
@@ -528,16 +532,18 @@ func setStatusForTest(tfJob *tfv1.TFJob, rtype commonv1.ReplicaType, failed, suc
 
 		index++
 	}
+
 	for i = 0; i < failed; i++ {
 		pod := testutil.NewPod(tfJob, typ, index, refs)
 		for k, v := range basicLabels {
 			pod.Labels[k] = v
 		}
 		po := &corev1.Pod{}
-		_ = client.Create(ctx, pod)
+		Expect(client.Create(ctx, pod)).Should(Succeed())
+
 		key := genKeyFromJob(pod)
 		Eventually(func() error {
-
+			po = &corev1.Pod{}
 			if err := client.Get(ctx, key, po); err != nil {
 				return err
 			}
@@ -547,7 +553,7 @@ func setStatusForTest(tfJob *tfv1.TFJob, rtype commonv1.ReplicaType, failed, suc
 				if po.Status.ContainerStatuses == nil {
 					po.Status.ContainerStatuses = []corev1.ContainerStatus{
 						{
-							Name: tfv1.DefaultContainerName,
+							Name: reconciler.GetDefaultContainerName(),
 							State: corev1.ContainerState{
 								Terminated: &corev1.ContainerStateTerminated{
 									ExitCode: int32(130), // 130 is a retryable code
@@ -564,15 +570,18 @@ func setStatusForTest(tfJob *tfv1.TFJob, rtype commonv1.ReplicaType, failed, suc
 		updateJobReplicaStatuses(&tfJob.Status, rtype, po)
 		index++
 	}
+
 	for i = 0; i < active; i++ {
 		pod := testutil.NewPod(tfJob, typ, index, refs)
 		for k, v := range basicLabels {
 			pod.Labels[k] = v
 		}
 		po := &corev1.Pod{}
 		Expect(client.Create(ctx, pod)).Should(Succeed())
+
 		key := genKeyFromJob(pod)
 		Eventually(func() error {
+			po = &corev1.Pod{}
 			if err := client.Get(ctx, key, po); err != nil {
 				return err
 			}

pkg/controller.v1/tensorflow/tfjob_controller_test.go

@@ -187,8 +187,8 @@ var _ = Describe("TFJob controller", func() {
 				testutil.SetPodsStatuses(testK8sClient, tfJob, testutil.LabelWorker, tc.pendingWorkerPods, tc.activeWorkerPods, tc.succeededWorkerPods, tc.failedWorkerPods, nil, refs, basicLabels)
 				testutil.SetPodsStatuses(testK8sClient, tfJob, testutil.LabelPS, tc.pendingPSPods, tc.activePSPods, tc.succeededPSPods, tc.failedPSPods, nil, refs, basicLabels)
 
-				testutil.SetServicesV2(testK8sClient, tfJob, testutil.LabelWorker, tc.activeWorkerServices, refs, basicLabels)
-				testutil.SetServicesV2(testK8sClient, tfJob, testutil.LabelPS, tc.activePSServices, refs, basicLabels)
+				testutil.SetServices(testK8sClient, tfJob, testutil.LabelWorker, tc.activeWorkerServices, refs, basicLabels)
+				testutil.SetServices(testK8sClient, tfJob, testutil.LabelPS, tc.activePSServices, refs, basicLabels)
 
 				totalPodNumber := int(tc.pendingWorkerPods + tc.activeWorkerPods + tc.succeededWorkerPods + tc.failedWorkerPods + tc.pendingPSPods + tc.activePSPods + tc.succeededPSPods + tc.failedPSPods)
 				totalServiceNumber := int(tc.activeWorkerServices + tc.activePSServices)