From 09da24ddf756732d6b4d993827d59a1e002d5ebd Mon Sep 17 00:00:00 2001 From: Ce Gao Date: Thu, 19 Sep 2019 10:31:47 +0800 Subject: [PATCH 1/7] feat: Add more output Signed-off-by: Ce Gao --- test/e2e/v1alpha3/run-e2e-experiment.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/e2e/v1alpha3/run-e2e-experiment.go b/test/e2e/v1alpha3/run-e2e-experiment.go index 6750b9a38f3..d9cd937659d 100644 --- a/test/e2e/v1alpha3/run-e2e-experiment.go +++ b/test/e2e/v1alpha3/run-e2e-experiment.go @@ -69,10 +69,18 @@ func main() { log.Fatal("Get Experiment error ", err) } 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) + 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) time.Sleep(20 * time.Second) } From 81e99b54c0d90335d96e3eb5bb07cef9323c0e64 Mon Sep 17 00:00:00 2001 From: Ce Gao Date: Thu, 19 Sep 2019 10:55:34 +0800 Subject: [PATCH 2/7] feat: Update Signed-off-by: Ce Gao --- test/e2e/v1alpha3/run-e2e-experiment.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/e2e/v1alpha3/run-e2e-experiment.go b/test/e2e/v1alpha3/run-e2e-experiment.go index d9cd937659d..f36f90d72b6 100644 --- a/test/e2e/v1alpha3/run-e2e-experiment.go +++ b/test/e2e/v1alpha3/run-e2e-experiment.go @@ -68,10 +68,6 @@ func main() { if err != nil { log.Fatal("Get Experiment error ", err) } - if exp.IsCompleted() { - log.Printf("Experiment %v finished", exp.Name) - break - } 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.`, @@ -81,6 +77,11 @@ func main() { 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) + + if exp.IsCompleted() { + log.Printf("Experiment %v finished", exp.Name) + break + } time.Sleep(20 * time.Second) } From bc0793d0e2c8fb48850d18396b86de1224852e08 Mon Sep 17 00:00:00 2001 From: Ce Gao Date: Thu, 19 Sep 2019 11:59:18 +0800 Subject: [PATCH 3/7] fix: Add suggestion Signed-off-by: Ce Gao --- pkg/util/v1alpha3/katibclient/katib_client.go | 15 +++++++++++++++ test/e2e/v1alpha3/run-e2e-experiment.go | 9 ++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/pkg/util/v1alpha3/katibclient/katib_client.go b/pkg/util/v1alpha3/katibclient/katib_client.go index 223d2684cdf..df1253dcc7b 100644 --- a/pkg/util/v1alpha3/katibclient/katib_client.go +++ b/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{} diff --git a/test/e2e/v1alpha3/run-e2e-experiment.go b/test/e2e/v1alpha3/run-e2e-experiment.go index f36f90d72b6..5b6074dc903 100644 --- a/test/e2e/v1alpha3/run-e2e-experiment.go +++ b/test/e2e/v1alpha3/run-e2e-experiment.go @@ -68,6 +68,10 @@ func main() { if err != nil { log.Fatal("Get Experiment error ", err) } + suggestion, err := kclient.GetSuggestion(exp.Name, exp.Namespace) + 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.`, @@ -77,7 +81,10 @@ func main() { 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) - + 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("Experiment %v finished", exp.Name) break From bb853af017f42b203cc49db7bac81b71d50d3885 Mon Sep 17 00:00:00 2001 From: Ce Gao Date: Thu, 19 Sep 2019 12:18:52 +0800 Subject: [PATCH 4/7] feat: generate Signed-off-by: Ce Gao --- .../v1alpha3/experiment/manifest/generator.go | 15 ----------- .../v1alpha3/util/katibclient/katibclient.go | 27 ++++++++++++++++--- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/pkg/mock/v1alpha3/experiment/manifest/generator.go b/pkg/mock/v1alpha3/experiment/manifest/generator.go index 80be9da7257..60de2c9620b 100644 --- a/pkg/mock/v1alpha3/experiment/manifest/generator.go +++ b/pkg/mock/v1alpha3/experiment/manifest/generator.go @@ -35,21 +35,6 @@ func (m *MockGenerator) EXPECT() *MockGeneratorMockRecorder { return m.recorder } -// GetMetricsCollectorManifest mocks base method -func (m *MockGenerator) GetMetricsCollectorManifest(arg0, arg1, arg2, arg3 string, arg4 []string, arg5 *v1alpha30.MetricsCollectorSpec) (string, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetMetricsCollectorManifest", arg0, arg1, arg2, arg3, arg4, arg5) - ret0, _ := ret[0].(string) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// GetMetricsCollectorManifest indicates an expected call of GetMetricsCollectorManifest -func (mr *MockGeneratorMockRecorder) GetMetricsCollectorManifest(arg0, arg1, arg2, arg3, arg4, arg5 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMetricsCollectorManifest", reflect.TypeOf((*MockGenerator)(nil).GetMetricsCollectorManifest), arg0, arg1, arg2, arg3, arg4, arg5) -} - // GetRunSpec mocks base method func (m *MockGenerator) GetRunSpec(arg0 *v1alpha30.Experiment, arg1, arg2, arg3 string) (string, error) { m.ctrl.T.Helper() diff --git a/pkg/mock/v1alpha3/util/katibclient/katibclient.go b/pkg/mock/v1alpha3/util/katibclient/katibclient.go index 412e1243777..c446ee3853c 100644 --- a/pkg/mock/v1alpha3/util/katibclient/katibclient.go +++ b/pkg/mock/v1alpha3/util/katibclient/katibclient.go @@ -7,7 +7,8 @@ package mock import ( gomock "github.com/golang/mock/gomock" v1alpha3 "github.com/kubeflow/katib/pkg/apis/controller/experiments/v1alpha3" - v1alpha30 "github.com/kubeflow/katib/pkg/apis/controller/trials/v1alpha3" + v1alpha30 "github.com/kubeflow/katib/pkg/apis/controller/suggestions/v1alpha3" + v1alpha31 "github.com/kubeflow/katib/pkg/apis/controller/trials/v1alpha3" reflect "reflect" client "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -132,15 +133,35 @@ func (mr *MockClientMockRecorder) GetExperimentList(arg0 ...interface{}) *gomock return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetExperimentList", reflect.TypeOf((*MockClient)(nil).GetExperimentList), arg0...) } +// GetSuggestion mocks base method +func (m *MockClient) GetSuggestion(arg0 string, arg1 ...string) (*v1alpha30.Suggestion, error) { + m.ctrl.T.Helper() + varargs := []interface{}{arg0} + for _, a := range arg1 { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "GetSuggestion", varargs...) + ret0, _ := ret[0].(*v1alpha30.Suggestion) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetSuggestion indicates an expected call of GetSuggestion +func (mr *MockClientMockRecorder) GetSuggestion(arg0 interface{}, arg1 ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{arg0}, arg1...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSuggestion", reflect.TypeOf((*MockClient)(nil).GetSuggestion), varargs...) +} + // GetTrialList mocks base method -func (m *MockClient) GetTrialList(arg0 string, arg1 ...string) (*v1alpha30.TrialList, error) { +func (m *MockClient) GetTrialList(arg0 string, arg1 ...string) (*v1alpha31.TrialList, error) { m.ctrl.T.Helper() varargs := []interface{}{arg0} for _, a := range arg1 { varargs = append(varargs, a) } ret := m.ctrl.Call(m, "GetTrialList", varargs...) - ret0, _ := ret[0].(*v1alpha30.TrialList) + ret0, _ := ret[0].(*v1alpha31.TrialList) ret1, _ := ret[1].(error) return ret0, ret1 } From badbd14cdfc4a1d9b6708f17139f8c32a22639bc Mon Sep 17 00:00:00 2001 From: Ce Gao Date: Thu, 19 Sep 2019 12:22:34 +0800 Subject: [PATCH 5/7] fix: Reorder Signed-off-by: Ce Gao --- test/e2e/v1alpha3/run-e2e-experiment.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/test/e2e/v1alpha3/run-e2e-experiment.go b/test/e2e/v1alpha3/run-e2e-experiment.go index 5b6074dc903..f7887cb576e 100644 --- a/test/e2e/v1alpha3/run-e2e-experiment.go +++ b/test/e2e/v1alpha3/run-e2e-experiment.go @@ -68,10 +68,6 @@ func main() { if err != nil { log.Fatal("Get Experiment error ", err) } - suggestion, err := kclient.GetSuggestion(exp.Name, exp.Namespace) - 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.`, @@ -81,10 +77,16 @@ func main() { 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) - log.Printf("Suggestion %s's conditions: %v", suggestion.Name, - suggestion.Status.Conditions) - log.Printf("Suggestion %s's suggestions: %v", suggestion.Name, - suggestion.Status.Suggestions) + + 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("Experiment %v finished", exp.Name) break From 5f17910a63bcc20adbfd48598446b3c1094a998e Mon Sep 17 00:00:00 2001 From: Ce Gao Date: Thu, 19 Sep 2019 13:32:24 +0800 Subject: [PATCH 6/7] fix: output error Signed-off-by: Ce Gao --- pkg/controller.v1alpha3/suggestion/suggestion_controller.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/controller.v1alpha3/suggestion/suggestion_controller.go b/pkg/controller.v1alpha3/suggestion/suggestion_controller.go index 7cce4e71903..f0a2b1bcbbe 100644 --- a/pkg/controller.v1alpha3/suggestion/suggestion_controller.go +++ b/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 From b7c048f9cdbc9a827234914fdde64c7826522581 Mon Sep 17 00:00:00 2001 From: Ce Gao Date: Thu, 19 Sep 2019 13:35:57 +0800 Subject: [PATCH 7/7] fix: Fix test cases Signed-off-by: Ce Gao --- .../experiment/experiment_controller_test.go | 36 ------------------- .../experiment/validator/validator_test.go | 12 ------- 2 files changed, 48 deletions(-) diff --git a/pkg/controller.v1alpha3/experiment/experiment_controller_test.go b/pkg/controller.v1alpha3/experiment/experiment_controller_test.go index abbe31cb111..7a4edf65ce4 100644 --- a/pkg/controller.v1alpha3/experiment/experiment_controller_test.go +++ b/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()) diff --git a/pkg/webhook/v1alpha3/experiment/validator/validator_test.go b/pkg/webhook/v1alpha3/experiment/validator/validator_test.go index c06e6bcb78a..bc13bb6769d 100644 --- a/pkg/webhook/v1alpha3/experiment/validator/validator_test.go +++ b/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,