Skip to content

Commit

Permalink
chore: Separate ReconcilerFunc from ContextFunc
Browse files Browse the repository at this point in the history
  • Loading branch information
irvinlim committed Apr 16, 2022
1 parent 98cec48 commit 1c8f331
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 38 deletions.
10 changes: 7 additions & 3 deletions pkg/execution/controllers/jobcontroller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ type Context struct {
controllercontext.Context
podInformer coreinformers.PodInformer
jobInformer executioninformers.JobInformer
HasSynced []cache.InformerSynced
hasSynced []cache.InformerSynced
queue workqueue.RateLimitingInterface
recorder record.EventRecorder
tasks tasks.ExecutorFactory
Expand Down Expand Up @@ -88,7 +88,7 @@ func NewContextWithRecorder(context controllercontext.Context, recorder record.E
// Bind informers.
c.podInformer = c.Informers().Kubernetes().Core().V1().Pods()
c.jobInformer = c.Informers().Furiko().Execution().V1alpha1().Jobs()
c.HasSynced = []cache.InformerSynced{
c.hasSynced = []cache.InformerSynced{
c.podInformer.Informer().HasSynced,
c.jobInformer.Informer().HasSynced,
}
Expand All @@ -99,6 +99,10 @@ func NewContextWithRecorder(context controllercontext.Context, recorder record.E
return c
}

func (c *Context) GetHasSynced() []cache.InformerSynced {
return c.hasSynced
}

func NewController(
ctrlContext controllercontext.Context,
concurrency *configv1alpha1.Concurrency,
Expand All @@ -121,7 +125,7 @@ func (c *Controller) Run(ctx context.Context) error {
klog.InfoS("jobcontroller: starting controller")

// Wait for cache sync up to a timeout.
if ok := cache.WaitForNamedCacheSync(controllerName, ctx.Done(), c.HasSynced...); !ok {
if ok := cache.WaitForNamedCacheSync(controllerName, ctx.Done(), c.hasSynced...); !ok {
klog.Error("jobcontroller: cache sync timeout")
return controllerutil.ErrWaitForCacheSyncTimeout
}
Expand Down
12 changes: 5 additions & 7 deletions pkg/execution/controllers/jobcontroller/reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (

"k8s.io/apimachinery/pkg/runtime"
ktesting "k8s.io/client-go/testing"
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/tools/record"
"k8s.io/utils/pointer"

Expand All @@ -39,12 +38,11 @@ import (

func TestReconciler(t *testing.T) {
test := runtimetesting.ReconcilerTest{
ReconcilerFunc: func(c controllercontext.Context) (reconciler.Reconciler, []cache.InformerSynced) {
ctrlCtx := jobcontroller.NewContextWithRecorder(c, &record.FakeRecorder{})
recon := jobcontroller.NewReconciler(ctrlCtx, &configv1alpha1.Concurrency{
Workers: 1,
})
return recon, ctrlCtx.HasSynced
ContextFunc: func(c controllercontext.Context) runtimetesting.ControllerContext {
return jobcontroller.NewContextWithRecorder(c, &record.FakeRecorder{})
},
ReconcilerFunc: func(c runtimetesting.ControllerContext) reconciler.Reconciler {
return jobcontroller.NewReconciler(c.(*jobcontroller.Context), runtimetesting.ReconcilerDefaultConcurrency)
},
Now: testutils.Mktime(now),
}
Expand Down
10 changes: 7 additions & 3 deletions pkg/execution/controllers/jobqueuecontroller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ type Context struct {
controllercontext.Context
jobInformer executioninformers.JobInformer
jobconfigInformer executioninformers.JobConfigInformer
HasSynced []cache.InformerSynced
hasSynced []cache.InformerSynced
jobConfigQueue workqueue.RateLimitingInterface
independentQueue workqueue.RateLimitingInterface
recorder record.EventRecorder
Expand All @@ -82,7 +82,7 @@ func NewContextWithRecorder(context controllercontext.Context, recorder record.E
// Bind informers.
c.jobInformer = c.Informers().Furiko().Execution().V1alpha1().Jobs()
c.jobconfigInformer = c.Informers().Furiko().Execution().V1alpha1().JobConfigs()
c.HasSynced = []cache.InformerSynced{
c.hasSynced = []cache.InformerSynced{
c.jobInformer.Informer().HasSynced,
c.jobconfigInformer.Informer().HasSynced,
}
Expand All @@ -96,6 +96,10 @@ func NewContextWithRecorder(context controllercontext.Context, recorder record.E
return c
}

func (c *Context) GetHasSynced() []cache.InformerSynced {
return c.hasSynced
}

func NewController(
ctrlContext controllercontext.Context,
concurrency *configv1alpha1.Concurrency,
Expand Down Expand Up @@ -125,7 +129,7 @@ func (c *Controller) Run(ctx context.Context) error {
defer utilruntime.HandleCrash()
klog.InfoS("jobqueuecontroller: starting controller")

if ok := cache.WaitForNamedCacheSync(controllerName, ctx.Done(), c.HasSynced...); !ok {
if ok := cache.WaitForNamedCacheSync(controllerName, ctx.Done(), c.hasSynced...); !ok {
klog.Error("jobqueuecontroller: cache sync timeout")
return controllerutil.ErrWaitForCacheSyncTimeout
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,8 @@ package jobqueuecontroller_test
import (
"testing"

"k8s.io/client-go/tools/cache"
"k8s.io/client-go/tools/record"

configv1alpha1 "github.com/furiko-io/furiko/apis/config/v1alpha1"
"github.com/furiko-io/furiko/pkg/execution/controllers/jobqueuecontroller"
"github.com/furiko-io/furiko/pkg/runtime/controllercontext"
"github.com/furiko-io/furiko/pkg/runtime/reconciler"
Expand All @@ -32,12 +30,14 @@ import (

func TestIndependentReconciler(t *testing.T) {
test := runtimetesting.ReconcilerTest{
ReconcilerFunc: func(c controllercontext.Context) (reconciler.Reconciler, []cache.InformerSynced) {
ctrlCtx := jobqueuecontroller.NewContextWithRecorder(c, &record.FakeRecorder{})
recon := jobqueuecontroller.NewIndependentReconciler(ctrlCtx, &configv1alpha1.Concurrency{
Workers: 1,
})
return recon, ctrlCtx.HasSynced
ContextFunc: func(c controllercontext.Context) runtimetesting.ControllerContext {
return jobqueuecontroller.NewContextWithRecorder(c, &record.FakeRecorder{})
},
ReconcilerFunc: func(c runtimetesting.ControllerContext) reconciler.Reconciler {
return jobqueuecontroller.NewIndependentReconciler(
c.(*jobqueuecontroller.Context),
runtimetesting.ReconcilerDefaultConcurrency,
)
},
Now: testutils.Mktime(now),
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,8 @@ import (
"testing"

"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/tools/record"

configv1alpha1 "github.com/furiko-io/furiko/apis/config/v1alpha1"
"github.com/furiko-io/furiko/pkg/execution/controllers/jobqueuecontroller"
"github.com/furiko-io/furiko/pkg/execution/stores/activejobstore"
"github.com/furiko-io/furiko/pkg/runtime/controllercontext"
Expand All @@ -35,12 +33,14 @@ import (

func TestPerJobConfigReconciler(t *testing.T) {
test := runtimetesting.ReconcilerTest{
ReconcilerFunc: func(c controllercontext.Context) (reconciler.Reconciler, []cache.InformerSynced) {
ctrlCtx := jobqueuecontroller.NewContextWithRecorder(c, &record.FakeRecorder{})
recon := jobqueuecontroller.NewPerConfigReconciler(ctrlCtx, &configv1alpha1.Concurrency{
Workers: 1,
})
return recon, ctrlCtx.HasSynced
ContextFunc: func(c controllercontext.Context) runtimetesting.ControllerContext {
return jobqueuecontroller.NewContextWithRecorder(c, &record.FakeRecorder{})
},
ReconcilerFunc: func(c runtimetesting.ControllerContext) reconciler.Reconciler {
return jobqueuecontroller.NewPerConfigReconciler(
c.(*jobqueuecontroller.Context),
runtimetesting.ReconcilerDefaultConcurrency,
)
},
Now: testutils.Mktime(now),
Stores: []mock.StoreFactory{
Expand Down
35 changes: 26 additions & 9 deletions pkg/runtime/testing/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,20 @@ const (
defaultReconcilerTestTimeout = time.Second
)

// ControllerContext is implemented by controller contexts used in reconciler tests.
type ControllerContext interface {
controllercontext.Context
GetHasSynced() []cache.InformerSynced
}

// ReconcilerTest encapsulates multiple test cases, providing a standard
// framework for testing standard Reconcilers.
type ReconcilerTest struct {
// Returns a new controller context.
ContextFunc func(c controllercontext.Context) ControllerContext

// Returns the reconciler to test and informer synced methods to wait for.
ReconcilerFunc func(c controllercontext.Context) (reconciler.Reconciler, []cache.InformerSynced)
ReconcilerFunc func(c ControllerContext) reconciler.Reconciler

// Sets the current time.
Now time.Time
Expand Down Expand Up @@ -92,7 +101,7 @@ type ReconcilerTestCase struct {
Reactors CombinedReactors

// Defines additional assertion functions.
Assert func(t assert.TestingT, tt ReconcilerTestCase)
Assert func(t assert.TestingT, tt ReconcilerTestCase, ctrlContext ControllerContext)

// List of actions that we expect to see.
WantActions CombinedActions
Expand Down Expand Up @@ -143,17 +152,17 @@ func (r *ReconcilerTest) RunTestCase(t testinginterface.T, tt ReconcilerTestCase

// Initialize context.
c := mock.NewContext()
r.setupContext(c, tt)
ctrlContext := r.setupContext(c, tt)

// Initialize reconciler.
recon, hasSynced := r.ReconcilerFunc(c)
recon := r.ReconcilerFunc(ctrlContext)

// Start the context.
err := c.Start(ctx)
assert.NoError(t, err)

// Initialize fixtures.
target, err := r.initClientset(ctx, c.MockClientsets(), tt, hasSynced)
target, err := r.initClientset(ctx, c.MockClientsets(), tt, ctrlContext.GetHasSynced())
if err != nil {
t.Fatalf("cannot initialize fixtures: %v", err)
}
Expand All @@ -164,10 +173,10 @@ func (r *ReconcilerTest) RunTestCase(t testinginterface.T, tt ReconcilerTestCase
}

// Compare actions.
r.assertResult(t, tt, c.MockClientsets())
r.assertResult(t, tt, c.MockClientsets(), ctrlContext)
}

func (r *ReconcilerTest) setupContext(c *mock.Context, tt ReconcilerTestCase) {
func (r *ReconcilerTest) setupContext(c *mock.Context, tt ReconcilerTestCase) ControllerContext {
// Set up configs.
c.MockConfigs().SetConfigs(tt.Configs)

Expand All @@ -181,6 +190,9 @@ func (r *ReconcilerTest) setupContext(c *mock.Context, tt ReconcilerTestCase) {
}
fakeClock := clock.NewFakeClock(now)
ktime.Clock = fakeClock

// Initialize context.
return r.ContextFunc(c)
}

// initClientset performs initialization of all fixtures and reactors with the given clientsets.
Expand Down Expand Up @@ -281,13 +293,18 @@ func (r *ReconcilerTest) initializeFixture(
return err
}

func (r *ReconcilerTest) assertResult(t testinginterface.T, tt ReconcilerTestCase, client *mock.Clientsets) {
func (r *ReconcilerTest) assertResult(
t testinginterface.T,
tt ReconcilerTestCase,
client *mock.Clientsets,
ctrlContext ControllerContext,
) {
// Compare actions.
CompareActions(t, tt.WantActions.Kubernetes, client.KubernetesMock().Actions())
CompareActions(t, tt.WantActions.Furiko, client.FurikoMock().Actions())

// Run custom assertions.
if tt.Assert != nil {
tt.Assert(t, tt)
tt.Assert(t, tt, ctrlContext)
}
}

0 comments on commit 1c8f331

Please sign in to comment.