refactor: Unify the interface (kubeflow#568)

Signed-off-by: Ce Gao <[email protected]>

pkg/controller/v1alpha2/experiment/experiment_util.go

@@ -71,7 +71,7 @@ func (r *ReconcileExperiment) createTrialInstance(expInstance *experimentsv1alph
 		logger.Error(err, "Error getting metrics collector manifest")
 		return err
 	}
-	trial.Spec.MetricsCollectorSpec = mcSpec.String()
+	trial.Spec.MetricsCollectorSpec = mcSpec
 
 	if err := r.Create(context.TODO(), trial); err != nil {
 		logger.Error(err, "Trial create error", "Trial name", trial.Name)

pkg/controller/v1alpha2/experiment/manifest/generator.go

@@ -25,7 +25,7 @@ type Generator interface {
 	InjectClient(c client.Client)
 	GetRunSpec(e *experimentsv1alpha2.Experiment, experiment, trial, namespace string) (string, error)
 	GetRunSpecWithHyperParameters(e *experimentsv1alpha2.Experiment, experiment, trial, namespace string, hps []*apiv1alpha2.ParameterAssignment) (string, error)
-	GetMetricsCollectorManifest(experimentName string, trialName string, jobKind string, namespace string, metricNames []string, mcs *experimentsv1alpha2.MetricsCollectorSpec) (*bytes.Buffer, error)
+	GetMetricsCollectorManifest(experimentName string, trialName string, jobKind string, namespace string, metricNames []string, mcs *experimentsv1alpha2.MetricsCollectorSpec) (string, error)
 }
 
 // DefaultGenerator is the default implementation of Generator.
@@ -45,7 +45,7 @@ func (g *DefaultGenerator) InjectClient(c client.Client) {
 	g.client.InjectClient(c)
 }
 
-func (g *DefaultGenerator) GetMetricsCollectorManifest(experimentName string, trialName string, jobKind string, namespace string, metricNames []string, mcs *experimentsv1alpha2.MetricsCollectorSpec) (*bytes.Buffer, error) {
+func (g *DefaultGenerator) GetMetricsCollectorManifest(experimentName string, trialName string, jobKind string, namespace string, metricNames []string, mcs *experimentsv1alpha2.MetricsCollectorSpec) (string, error) {
 	var mtp *template.Template = nil
 	var err error
 	tmpValues := map[string]string{
@@ -65,23 +65,23 @@ func (g *DefaultGenerator) GetMetricsCollectorManifest(experimentName string, tr
 		}
 		mtl, err := g.client.GetMetricsCollectorTemplates()
 		if err != nil {
-			return nil, err
+			return "", err
 		}
 		if mt, ok := mtl[mctp]; !ok {
-			return nil, fmt.Errorf("No MetricsCollector template name %s", mctp)
+			return "", fmt.Errorf("No MetricsCollector template name %s", mctp)
 		} else {
 			mtp, err = template.New("MetricsCollector").Parse(mt)
 		}
 	}
 	if err != nil {
-		return nil, err
+		return "", err
 	}
 	var b bytes.Buffer
 	err = mtp.Execute(&b, tmpValues)
 	if err != nil {
-		return nil, err
+		return "", err
 	}
-	return &b, nil
+	return b.String(), nil
 }
 
 // GetRunSpec get the specification for trial.

pkg/controller/v1alpha2/experiment/manifest/generator_test.go

@@ -130,7 +130,7 @@ data:
 				- "-mn"
 				- "test"`
 
-	if expected != actual.String() {
+	if expected != actual {
 		t.Errorf("Expected %s, got %s", expected, actual)
 	}
 }

pkg/controller/v1alpha2/experiment/validator/validator.go

@@ -8,8 +8,8 @@ import (
 	batchv1beta "k8s.io/api/batch/v1beta1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	k8syaml "k8s.io/apimachinery/pkg/util/yaml"
-	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
 
 	commonapiv1alpha2 "github.com/kubeflow/katib/pkg/api/operators/apis/common/v1alpha2"
 	experimentsv1alpha2 "github.com/kubeflow/katib/pkg/api/operators/apis/experiment/v1alpha2"
@@ -179,15 +179,16 @@ func (g *DefaultValidator) validateMetricsCollector(inst *experimentsv1alpha2.Ex
 		return fmt.Errorf("Invalid spec.trialTemplate: %v.", err)
 	}
 
-	var mcjob batchv1beta.CronJob
 	mcm, err := g.GetMetricsCollectorManifest(experimentName, trialName, job.GetKind(), namespace, metricNames, inst.Spec.MetricsCollectorSpec)
 	if err != nil {
 		log.Info("getMetricsCollectorManifest error", "err", err)
 		return err
 	}
 
-	log.Info("1", "m", mcm, "instance", inst)
-	if err := k8syaml.NewYAMLOrJSONDecoder(mcm, BUFSIZE).Decode(&mcjob); err != nil {
+	buf = bytes.NewBufferString(mcm)
+
+	mcjob := &batchv1beta.CronJob{}
+	if err := k8syaml.NewYAMLOrJSONDecoder(buf, BUFSIZE).Decode(&mcjob); err != nil {
 		log.Info("MetricsCollector Yaml decode error", "err", err)
 		return err
 	}

pkg/controller/v1alpha2/experiment/validator/validator_test.go

@@ -1,7 +1,6 @@
 package validator
 
 import (
-	"bytes"
 	"database/sql"
 	"testing"
 
@@ -104,15 +103,7 @@ spec:
 	p.EXPECT().GetMetricsCollectorManifest(
 		gomock.Any(), gomock.Any(), gomock.Any(),
 		gomock.Any(), gomock.Any(), gomock.Any()).
-		Return(func() *bytes.Buffer {
-			var b bytes.Buffer
-			_, err := b.WriteString(metricsCollectorTemplate)
-			if err != nil {
-				t.Errorf("Expected nil, got %v", err)
-			}
-			println(b.String())
-			return &b
-		}(), nil).AnyTimes()
+		Return(metricsCollectorTemplate, nil).AnyTimes()
 	mc.EXPECT().GetExperimentFromDB(gomock.Any()).Return(nil, sql.ErrNoRows).AnyTimes()
 
 	tcs := []struct {