feat: Add more output in e2e test for debug purpose and fix test cases

pkg/controller.v1alpha3/experiment/experiment_controller_test.go

@@ -168,42 +168,6 @@ spec:
           containers:
             - name: tensorflow
               image: kubeflow/tf-dist-mnist-test:1.0`, nil).AnyTimes()
-	generator.EXPECT().GetMetricsCollectorManifest(
-		gomock.Any(), gomock.Any(),
-		gomock.Any(), gomock.Any(),
-		gomock.Any(), gomock.Any()).Return(`apiVersion: batch/v1beta1
-kind: CronJob
-metadata:
-  name: test
-  namespace: default
-spec:
-  schedule: "*/1 * * * *"
-  successfulJobsHistoryLimit: 0
-  failedJobsHistoryLimit: 1
-  jobTemplate:
-    spec:
-      backoffLimit: 0
-      template:
-        spec:
-          serviceAccountName: metrics-collector
-          containers:
-          - name: test
-            image: katib/metrics-collector
-            args:
-            - "./metricscollector.v1alpha3"
-            - "-e"
-            - "teste"
-            - "-t"
-            - "test"
-            - "-k"
-            - "TFJob"
-            - "-n"
-            - "default"
-            - "-m"
-            - "test"
-            - "-mn"
-            - "test"
-          restartPolicy: Never`, nil).AnyTimes()
 
 	mgr, err := manager.New(cfg, manager.Options{})
 	g.Expect(err).NotTo(gomega.HaveOccurred())

pkg/controller.v1alpha3/suggestion/suggestion_controller.go

@@ -18,6 +18,7 @@ package suggestion
 
 import (
 	"context"
+	"fmt"
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -193,7 +194,7 @@ func (r *ReconcileSuggestion) ReconcileSuggestion(instance *suggestionsv1alpha3.
 	if !instance.IsRunning() {
 		if err = r.ValidateAlgorithmSettings(instance, experiment); err != nil {
 			logger.Error(err, "Marking suggestion failed as algorithm settings validation failed")
-			msg := "Suggestion has failed because Algorithm settings validation failed"
+			msg := fmt.Sprintf("Validation failed: %v", err)
 			instance.MarkSuggestionStatusFailed(SuggestionFailedReason, msg)
 			// return nil since it is a terminal condition
 			return nil

pkg/util/v1alpha3/katibclient/katib_client.go

@@ -27,6 +27,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
 
 	experimentsv1alpha3 "github.com/kubeflow/katib/pkg/apis/controller/experiments/v1alpha3"
+	suggestionsv1alpha3 "github.com/kubeflow/katib/pkg/apis/controller/suggestions/v1alpha3"
 	trialsv1alpha3 "github.com/kubeflow/katib/pkg/apis/controller/trials/v1alpha3"
 	"github.com/kubeflow/katib/pkg/controller.v1alpha3/consts"
 )
@@ -40,6 +41,7 @@ type Client interface {
 	GetConfigMap(name string, namespace ...string) (map[string]string, error)
 	GetTrialList(name string, namespace ...string) (*trialsv1alpha3.TrialList, error)
 	GetTrialTemplates(namespace ...string) (map[string]string, error)
+	GetSuggestion(name string, namespace ...string) (*suggestionsv1alpha3.Suggestion, error)
 	UpdateTrialTemplates(newTrialTemplates map[string]string, namespace ...string) error
 }
 
@@ -60,6 +62,7 @@ func NewClient(options client.Options) (Client, error) {
 	}
 	experimentsv1alpha3.AddToScheme(scheme.Scheme)
 	trialsv1alpha3.AddToScheme(scheme.Scheme)
+	suggestionsv1alpha3.AddToScheme(scheme.Scheme)
 	cl, err := client.New(cfg, options)
 	return &KatibClient{
 		client: cl,
@@ -82,6 +85,18 @@ func (k *KatibClient) GetExperimentList(namespace ...string) (*experimentsv1alph
 
 }
 
+func (k *KatibClient) GetSuggestion(name string, namespace ...string) (
+	*suggestionsv1alpha3.Suggestion, error) {
+	ns := getNamespace(namespace...)
+	suggestion := &suggestionsv1alpha3.Suggestion{}
+
+	if err := k.client.Get(context.TODO(), types.NamespacedName{Name: name, Namespace: ns}, suggestion); err != nil {
+		return nil, err
+	}
+	return suggestion, nil
+
+}
+
 func (k *KatibClient) GetTrialList(name string, namespace ...string) (*trialsv1alpha3.TrialList, error) {
 	ns := getNamespace(namespace...)
 	trialList := &trialsv1alpha3.TrialList{}

pkg/webhook/v1alpha3/experiment/validator/validator_test.go

@@ -97,19 +97,7 @@ metadata:
   name: "fake-trial"
   namespace: fakens`
 
-	metricsCollectorTemplate := `apiVersion: batch/v1beta1
-kind: CronJob
-metadata:
-  name: fake-trial
-  namespace: fakens
-spec:
-  schedule: "*/1 * * * *"`
-
 	p.EXPECT().GetRunSpec(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(trialJobTemplate, nil).AnyTimes()
-	p.EXPECT().GetMetricsCollectorManifest(
-		gomock.Any(), gomock.Any(), gomock.Any(),
-		gomock.Any(), gomock.Any(), gomock.Any()).
-		Return(metricsCollectorTemplate, nil).AnyTimes()
 	mc.EXPECT().PreCheckRegisterExperimentInDB(gomock.Any()).Return(
 		&api_pb.PreCheckRegisterExperimentReply{
 			CanRegister: true,

test/e2e/v1alpha3/run-e2e-experiment.go

@@ -68,11 +68,29 @@ func main() {
 		if err != nil {
 			log.Fatal("Get Experiment error ", err)
 		}
+		log.Printf("Waiting for Experiment %s to finish.", exp.Name)
+		log.Printf(`Experiment %s's trials: %d trials, %d pending trials,
+%d running trials, %d killed trials, %d succeeded trials, %d failed trials.`,
+			exp.Name,
+			exp.Status.Trials, exp.Status.TrialsPending, exp.Status.TrialsRunning,
+			exp.Status.TrialsKilled, exp.Status.TrialsSucceeded, exp.Status.TrialsFailed)
+		log.Printf("Optimal Trial for Experiment %s: %v", exp.Name,
+			exp.Status.CurrentOptimalTrial)
+		log.Printf("Experiment %s's conditions: %v", exp.Name, exp.Status.Conditions)
+
+		suggestion, err := kclient.GetSuggestion(exp.Name, exp.Namespace)
+		if err != nil {
+			log.Printf("Get Suggestion error: %v", err)
+		} else {
+			log.Printf("Suggestion %s's conditions: %v", suggestion.Name,
+				suggestion.Status.Conditions)
+			log.Printf("Suggestion %s's suggestions: %v", suggestion.Name,
+				suggestion.Status.Suggestions)
+		}
 		if exp.IsCompleted() {
-			log.Printf("Job %v finished", exp.Name)
+			log.Printf("Experiment %v finished", exp.Name)
 			break
 		}
-		log.Printf("Waiting for job %v to finish. [ %v trials running %v succeeded ]", exp.Name, exp.Status.TrialsRunning, exp.Status.TrialsSucceeded)
 		time.Sleep(20 * time.Second)
 	}