Skip to content

Commit

Permalink
feat(experiment): Add test cases (#556)
Browse files Browse the repository at this point in the history
* fix: Remove useless code

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* feat(experiment): Abstract manager client

Signed-off-by: Ce Gao <gaoce@caicloud.io>

* feat: Add basic test cases

Signed-off-by: Ce Gao <gaoce@caicloud.io>
  • Loading branch information
gaocegege authored and k8s-ci-robot committed May 20, 2019
1 parent f71f0e8 commit 22ac009
Show file tree
Hide file tree
Showing 17 changed files with 447 additions and 173 deletions.
23 changes: 9 additions & 14 deletions pkg/controller/v1alpha2/experiment/experiment_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (
experimentsv1alpha2 "github.com/kubeflow/katib/pkg/api/operators/apis/experiment/v1alpha2"
trialsv1alpha2 "github.com/kubeflow/katib/pkg/api/operators/apis/trial/v1alpha2"
"github.com/kubeflow/katib/pkg/controller/v1alpha2/consts"
"github.com/kubeflow/katib/pkg/controller/v1alpha2/experiment/managerclient"
"github.com/kubeflow/katib/pkg/controller/v1alpha2/experiment/manifest"
"github.com/kubeflow/katib/pkg/controller/v1alpha2/experiment/suggestion"
suggestionfake "github.com/kubeflow/katib/pkg/controller/v1alpha2/experiment/suggestion/fake"
Expand All @@ -64,17 +65,14 @@ func Add(mgr manager.Manager) error {
// newReconciler returns a new reconcile.Reconciler
func newReconciler(mgr manager.Manager) reconcile.Reconciler {
r := &ReconcileExperiment{
Client: mgr.GetClient(),
scheme: mgr.GetScheme(),
Client: mgr.GetClient(),
scheme: mgr.GetScheme(),
ManagerClient: managerclient.New(),
}
imp := viper.GetString(consts.ConfigExperimentSuggestionName)
r.Suggestion = newSuggestion(imp)

generator, err := manifest.New()
if err != nil {
panic(err)
}
r.Generator = generator
r.Generator = manifest.New(r.Client)
r.updateStatusHandler = r.updateStatus
return r
}
Expand Down Expand Up @@ -145,17 +143,13 @@ func addWebhook(mgr manager.Manager) error {
if err != nil {
return err
}
validator, err := newExperimentValidator()
if err != nil {
return err
}
validatingWebhook, err := builder.NewWebhookBuilder().
Name("validating.experiment.kubeflow.org").
Validating().
Operations(admissionregistrationv1beta1.Create, admissionregistrationv1beta1.Update).
WithManager(mgr).
ForType(&experimentsv1alpha2.Experiment{}).
Handlers(validator).
Handlers(newExperimentValidator(mgr.GetClient())).
Build()
if err != nil {
return err
Expand Down Expand Up @@ -197,6 +191,7 @@ type ReconcileExperiment struct {

suggestion.Suggestion
manifest.Generator
managerclient.ManagerClient
// updateStatusHandler is defined for test purpose.
updateStatusHandler updateStatusFunc
}
Expand Down Expand Up @@ -243,7 +238,7 @@ func (r *ReconcileExperiment) Reconcile(request reconcile.Request) (reconcile.Re
msg := "Experiment is created"
instance.MarkExperimentStatusCreated(util.ExperimentCreatedReason, msg)

err = util.CreateExperimentInDB(instance)
err = r.CreateExperimentInDB(instance)
if err != nil {
logger.Error(err, "Create experiment in DB error")
return reconcile.Result{}, err
Expand All @@ -259,7 +254,7 @@ func (r *ReconcileExperiment) Reconcile(request reconcile.Request) (reconcile.Re

if !equality.Semantic.DeepEqual(original.Status, instance.Status) {
//assuming that only status change
err = util.UpdateExperimentStatusInDB(instance)
err = r.UpdateExperimentStatusInDB(instance)
if err != nil {
logger.Error(err, "Update experiment status in DB error")
return reconcile.Result{}, err
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/*
Copyright 2019 The Kubernetes 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 experiment

import (
stdlog "log"
"os"
"path/filepath"
"sync"
"testing"

"github.com/onsi/gomega"
"k8s.io/client-go/kubernetes/scheme"
"k8s.io/client-go/rest"
"sigs.k8s.io/controller-runtime/pkg/envtest"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

"github.com/kubeflow/katib/pkg/api/operators/apis"
)

var cfg *rest.Config

func TestMain(m *testing.M) {
t := &envtest.Environment{
CRDDirectoryPaths: []string{
filepath.Join("..", "..", "..", "..", "manifests", "v1alpha2", "katib-controller"),
filepath.Join("..", "..", "..", "..", "test", "unit", "v1alpha2", "crds"),
},
}
apis.AddToScheme(scheme.Scheme)

var err error
if cfg, err = t.Start(); err != nil {
stdlog.Fatal(err)
}

code := m.Run()
t.Stop()
os.Exit(code)
}

// SetupTestReconcile returns a reconcile.Reconcile implementation that delegates to inner and
// writes the request to requests after Reconcile is finished.
func SetupTestReconcile(inner reconcile.Reconciler) (reconcile.Reconciler, chan reconcile.Request) {
requests := make(chan reconcile.Request)
fn := reconcile.Func(func(req reconcile.Request) (reconcile.Result, error) {
result, err := inner.Reconcile(req)
requests <- req
return result, err
})
return fn, requests
}

// StartTestManager adds recFn
func StartTestManager(mgr manager.Manager, g *gomega.GomegaWithT) (chan struct{}, *sync.WaitGroup) {
stop := make(chan struct{})
wg := &sync.WaitGroup{}
wg.Add(1)
go func() {
defer wg.Done()
g.Expect(mgr.Start(stop)).NotTo(gomega.HaveOccurred())
}()
return stop, wg
}
103 changes: 103 additions & 0 deletions pkg/controller/v1alpha2/experiment/experiment_controller_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
package experiment

import (
"context"
"testing"
"time"

"github.com/golang/mock/gomock"
"github.com/onsi/gomega"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"sigs.k8s.io/controller-runtime/pkg/manager"
"sigs.k8s.io/controller-runtime/pkg/reconcile"
logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"

experimentsv1alpha2 "github.com/kubeflow/katib/pkg/api/operators/apis/experiment/v1alpha2"
managerclientmock "github.com/kubeflow/katib/pkg/mock/v1alpha2/experiment/managerclient"
manifestmock "github.com/kubeflow/katib/pkg/mock/v1alpha2/experiment/manifest"
suggestionmock "github.com/kubeflow/katib/pkg/mock/v1alpha2/experiment/suggestion"
)

const (
experimentName = "foo"
namespace = "default"

timeout = time.Second * 20
)

var expectedRequest = reconcile.Request{NamespacedName: types.NamespacedName{Name: experimentName, Namespace: namespace}}
var trialKey = types.NamespacedName{Name: "test", Namespace: namespace}

func init() {
logf.SetLogger(logf.ZapLogger(true))
}

func TestCreateExperiment(t *testing.T) {
g := gomega.NewGomegaWithT(t)
instance := newFakeInstance()

mockCtrl := gomock.NewController(t)
defer mockCtrl.Finish()
mc := managerclientmock.NewMockManagerClient(mockCtrl)
mc.EXPECT().CreateExperimentInDB(gomock.Any()).Return(nil).AnyTimes()
mc.EXPECT().UpdateExperimentStatusInDB(gomock.Any()).Return(nil).AnyTimes()

mockCtrl2 := gomock.NewController(t)
defer mockCtrl2.Finish()
suggestion := suggestionmock.NewMockSuggestion(mockCtrl)

mockCtrl3 := gomock.NewController(t)
defer mockCtrl3.Finish()
generator := manifestmock.NewMockGenerator(mockCtrl)

// Setup the Manager and Controller. Wrap the Controller Reconcile function so it writes each request to a
// channel when it is finished.
mgr, err := manager.New(cfg, manager.Options{})
g.Expect(err).NotTo(gomega.HaveOccurred())
c := mgr.GetClient()

recFn, requests := SetupTestReconcile(&ReconcileExperiment{
Client: mgr.GetClient(),
scheme: mgr.GetScheme(),
ManagerClient: mc,
Suggestion: suggestion,
Generator: generator,
updateStatusHandler: func(instance *experimentsv1alpha2.Experiment) error {
if !instance.IsCreated() {
t.Errorf("Expected got condition created")
}
return nil
},
})
g.Expect(add(mgr, recFn)).NotTo(gomega.HaveOccurred())

stopMgr, mgrStopped := StartTestManager(mgr, g)

defer func() {
close(stopMgr)
mgrStopped.Wait()
}()

// Create the Trial object and expect the Reconcile and Deployment to be created
err = c.Create(context.TODO(), instance)
// The instance object may not be a valid object because it might be missing some required fields.
// Please modify the instance object by adding required fields and then remove the following if statement.
if apierrors.IsInvalid(err) {
t.Logf("failed to create object, got an invalid object error: %v", err)
return
}
g.Expect(err).NotTo(gomega.HaveOccurred())
defer c.Delete(context.TODO(), instance)
g.Eventually(requests, timeout).Should(gomega.Receive(gomega.Equal(expectedRequest)))
}

func newFakeInstance() *experimentsv1alpha2.Experiment {
return &experimentsv1alpha2.Experiment{
ObjectMeta: metav1.ObjectMeta{
Name: experimentName,
Namespace: namespace,
},
}
}
7 changes: 3 additions & 4 deletions pkg/controller/v1alpha2/experiment/experiment_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
experimentsv1alpha2 "github.com/kubeflow/katib/pkg/api/operators/apis/experiment/v1alpha2"
trialsv1alpha2 "github.com/kubeflow/katib/pkg/api/operators/apis/trial/v1alpha2"
apiv1alpha2 "github.com/kubeflow/katib/pkg/api/v1alpha2"
"github.com/kubeflow/katib/pkg/controller/v1alpha2/experiment/util"
)

func (r *ReconcileExperiment) createTrialInstance(expInstance *experimentsv1alpha2.Experiment, trialInstance *apiv1alpha2.Trial) error {
Expand All @@ -39,8 +38,8 @@ func (r *ReconcileExperiment) createTrialInstance(expInstance *experimentsv1alph
if trialInstance.Spec != nil && trialInstance.Spec.ParameterAssignments != nil {
for _, p := range trialInstance.Spec.ParameterAssignments.Assignments {
hps = append(hps, p)
pa := common.ParameterAssignment {
Name: p.Name,
pa := common.ParameterAssignment{
Name: p.Name,
Value: p.Value,
}
trial.Spec.ParameterAssignments = append(trial.Spec.ParameterAssignments, pa)
Expand Down Expand Up @@ -85,7 +84,7 @@ func (r *ReconcileExperiment) createTrialInstance(expInstance *experimentsv1alph
func (r *ReconcileExperiment) updateFinalizers(instance *experimentsv1alpha2.Experiment, finalizers []string) (reconcile.Result, error) {
logger := log.WithValues("Experiment", types.NamespacedName{Name: instance.Name, Namespace: instance.Namespace})
if instance.GetDeletionTimestamp() != nil {
if err := util.DeleteExperimentInDB(instance); err != nil {
if err := r.DeleteExperimentInDB(instance); err != nil {
logger.Error(err, "Fail to delete data in DB")
return reconcile.Result{}, err
}
Expand Down
Loading

0 comments on commit 22ac009

Please sign in to comment.