Repository: furiko-io/furiko
Pull request #44 (2022-04-11 17:51:45)

test(runtime): Add controllermanager tests
Adds unit tests for controllermanager.

codecov[bot]: # [Codecov](https://codecov.io/gh/furiko-io/furiko/pull/44?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=furiko-io) Report
> Merging [#44](https://codecov.io/gh/furiko-io/furiko/pull/44?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=furiko-io) (bd75fd7) into [main](https://codecov.io/gh/furiko-io/furiko/commit/650aab17f285e3fd9946b8e27673e49e2eb86804?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=furiko-io) (650aab1) will **increase** coverage by `3.02%`.
> The diff coverage is `50.00%`.

```diff
@@            Coverage Diff             @@
##             main      #44      +/-   ##
==========================================
+ Coverage   51.45%   54.47%   +3.02%     
==========================================
  Files         163      163              
  Lines        8095     8095              
==========================================
+ Hits         4165     4410     +245     
+ Misses       3731     3477     -254     
- Partials      199      208       +9     
```


| [Impacted Files](https://codecov.io/gh/furiko-io/furiko/pull/44?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=furiko-io) | Coverage Δ | |
|---|---|---|
| [cmd/execution-controller/main.go](https://codecov.io/gh/furiko-io/furiko/pull/44/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=furiko-io#diff-Y21kL2V4ZWN1dGlvbi1jb250cm9sbGVyL21haW4uZ28=) | `0.00% <0.00%> (ø)` | |
| [pkg/runtime/controllermanager/controller.go](https://codecov.io/gh/furiko-io/furiko/pull/44/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=furiko-io#diff-cGtnL3J1bnRpbWUvY29udHJvbGxlcm1hbmFnZXIvY29udHJvbGxlci5nbw==) | `91.80% <100.00%> (+91.80%)` | :arrow_up: |
| [pkg/runtime/controllercontext/context\_configs.go](https://codecov.io/gh/furiko-io/furiko/pull/44/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=furiko-io#diff-cGtnL3J1bnRpbWUvY29udHJvbGxlcmNvbnRleHQvY29udGV4dF9jb25maWdzLmdv) | `45.76% <0.00%> (+27.11%)` | :arrow_up: |
| [pkg/runtime/controllermanager/manager.go](https://codecov.io/gh/furiko-io/furiko/pull/44/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=furiko-io#diff-cGtnL3J1bnRpbWUvY29udHJvbGxlcm1hbmFnZXIvbWFuYWdlci5nbw==) | `46.42% <0.00%> (+46.42%)` | :arrow_up: |
| [pkg/runtime/leaderelection/metrics.go](https://codecov.io/gh/furiko-io/furiko/pull/44/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=furiko-io#diff-cGtnL3J1bnRpbWUvbGVhZGVyZWxlY3Rpb24vbWV0cmljcy5nbw==) | `100.00% <0.00%> (+60.00%)` | :arrow_up: |
| [pkg/runtime/leaderelection/identity.go](https://codecov.io/gh/furiko-io/furiko/pull/44/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=furiko-io#diff-cGtnL3J1bnRpbWUvbGVhZGVyZWxlY3Rpb24vaWRlbnRpdHkuZ28=) | `62.50% <0.00%> (+62.50%)` | :arrow_up: |
| [pkg/runtime/leaderelection/election.go](https://codecov.io/gh/furiko-io/furiko/pull/44/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=furiko-io#diff-cGtnL3J1bnRpbWUvbGVhZGVyZWxlY3Rpb24vZWxlY3Rpb24uZ28=) | `66.93% <0.00%> (+66.93%)` | :arrow_up: |
| [pkg/runtime/controllermanager/store.go](https://codecov.io/gh/furiko-io/furiko/pull/44/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=furiko-io#diff-cGtnL3J1bnRpbWUvY29udHJvbGxlcm1hbmFnZXIvc3RvcmUuZ28=) | `100.00% <0.00%> (+100.00%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/furiko-io/furiko/pull/44?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=furiko-io).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=furiko-io)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/furiko-io/furiko/pull/44?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=furiko-io). Last update [650aab1...bd75fd7](https://codecov.io/gh/furiko-io/furiko/pull/44?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=furiko-io). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=furiko-io).


Patch:
diff --git a/cmd/execution-controller/main.go b/cmd/execution-controller/main.go
index 2fdd9f0..5fa696b 100644
--- a/cmd/execution-controller/main.go
+++ b/cmd/execution-controller/main.go
@@ -85,7 +85,7 @@ func main() {
 
 	// Create controller manager.
 	klog.Info("setting up controller manager")
-	mgr, err := controllermanager.NewManager(
+	mgr, err := controllermanager.NewControllerManager(
 		ctrlContext,
 		options.ControllerManagerConfigSpec,
 		"execution-controller",
diff --git a/pkg/runtime/controllermanager/controller.go b/pkg/runtime/controllermanager/controller.go
index c8c617f..40fdcd9 100644
--- a/pkg/runtime/controllermanager/controller.go
+++ b/pkg/runtime/controllermanager/controller.go
@@ -53,7 +53,7 @@ type ControllerManager struct {
 	coordinator leaderelection.Coordinator
 }
 
-func NewManager(
+func NewControllerManager(
 	ctrlContext controllercontext.Context,
 	ctrlCfg configv1alpha1.ControllerManagerConfigSpec,
 	defaultLeaseName string,
@@ -175,7 +175,7 @@ func (m *ControllerManager) GetHealth() []HealthStatus {
 	healths := make([]HealthStatus, 0, len(m.controllers))
 
 	// If not yet starting (i.e. not elected), do not return false unhealthy state.
-	if m.IsStarting() {
+	if !m.IsStarting() {
 		return healths
 	}
 


Test patch:
diff --git a/pkg/runtime/controllermanager/controller_test.go b/pkg/runtime/controllermanager/controller_test.go
new file mode 100644
index 0000000..1475ac1
--- /dev/null
+++ b/pkg/runtime/controllermanager/controller_test.go
@@ -0,0 +1,340 @@
+/*
+ * Copyright 2022 The Furiko 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 controllermanager_test
+
+import (
+	"context"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"k8s.io/utils/pointer"
+
+	configv1alpha1 "github.com/furiko-io/furiko/apis/config/v1alpha1"
+	"github.com/furiko-io/furiko/pkg/runtime/controllercontext/mock"
+	"github.com/furiko-io/furiko/pkg/runtime/controllermanager"
+)
+
+type mockRunnable struct {
+	runError        bool
+	startupDuration time.Duration
+}
+
+func (m mockRunnable) Run(ctx context.Context) error {
+	t := time.NewTimer(m.startupDuration)
+	select {
+	case <-ctx.Done():
+		return ctx.Err()
+	case <-t.C:
+	}
+	if m.runError {
+		return assert.AnError
+	}
+	return nil
+}
+
+type mockController struct {
+	name             string
+	mock             mockRunnable
+	shutdownDuration time.Duration
+	shutdownDone     uint64
+}
+
+func newMockController(name string, mock mockRunnable) *mockController {
+	return &mockController{
+		name: name,
+		mock: mock,
+	}
+}
+
+var _ controllermanager.Controller = (*mockController)(nil)
+
+func (c *mockController) Run(ctx context.Context) error {
+	return c.mock.Run(ctx)
+}
+
+func (c *mockController) GetHealth() controllermanager.HealthStatus {
+	return controllermanager.HealthStatus{
+		Name:    c.name,
+		Healthy: true,
+	}
+}
+
+func (c *mockController) Shutdown(ctx context.Context) {
+	t := time.NewTimer(c.shutdownDuration)
+	select {
+	case <-ctx.Done():
+		return
+	case <-t.C:
+	}
+	atomic.StoreUint64(&c.shutdownDone, 1)
+}
+
+type mockStore struct {
+	name string
+	mock mockRunnable
+}
+
+var _ controllermanager.Store = (*mockStore)(nil)
+
+func newMockStore(name string, mock mockRunnable) *mockStore {
+	return &mockStore{
+		name: name,
+		mock: mock,
+	}
+}
+
+func (m *mockStore) Name() string {
+	return m.name
+}
+
+func (m *mockStore) Recover(ctx context.Context) error {
+	return m.mock.Run(ctx)
+}
+
+func TestControllerManager_Start(t *testing.T) {
+	tests := []struct {
+		name             string
+		ctrlCfg          configv1alpha1.ControllerManagerConfigSpec
+		defaultLeaseName string
+		controllers      []controllermanager.Controller
+		stores           []controllermanager.Store
+		wantInitErr      bool
+		timeout          time.Duration
+		cancelAfter      time.Duration
+		wantStartErr     assert.ErrorAssertionFunc
+	}{
+		{
+			name:         "no controllers",
+			wantStartErr: assert.NoError,
+		},
+		{
+			name: "start single controller",
+			controllers: []controllermanager.Controller{
+				newMockController("mock", mockRunnable{}),
+			},
+			wantStartErr: assert.NoError,
+		},
+		{
+			name: "start single controller with error",
+			controllers: []controllermanager.Controller{
+				newMockController("mock", mockRunnable{runError: true}),
+			},
+			wantStartErr: assertErrorIs(assert.AnError),
+		},
+		{
+			name: "start single store",
+			stores: []controllermanager.Store{
+				newMockStore("mock", mockRunnable{}),
+			},
+			wantStartErr: assert.NoError,
+		},
+		{
+			name: "start single store with error",
+			stores: []controllermanager.Store{
+				newMockStore("mock", mockRunnable{runError: true}),
+			},
+			wantStartErr: assertErrorIs(assert.AnError),
+		},
+		{
+			name: "start controller timeout",
+			controllers: []controllermanager.Controller{
+				newMockController("mock1", mockRunnable{startupDuration: time.Millisecond * 500}),
+				newMockController("mock2", mockRunnable{startupDuration: time.Millisecond * 20}),
+			},
+			timeout:      time.Millisecond * 50,
+			wantStartErr: assertErrorIs(context.DeadlineExceeded),
+		},
+		{
+			name: "start store timeout",
+			controllers: []controllermanager.Controller{
+				newMockController("mock1", mockRunnable{}),
+				newMockController("mock2", mockRunnable{}),
+			},
+			stores: []controllermanager.Store{
+				newMockStore("mock", mockRunnable{startupDuration: time.Millisecond * 500}),
+			},
+			timeout:      time.Millisecond * 50,
+			wantStartErr: assertErrorIs(context.DeadlineExceeded),
+		},
+		{
+			name: "start multiple controllers and stores with varying duration",
+			controllers: []controllermanager.Controller{
+				newMockController("mock1", mockRunnable{startupDuration: time.Millisecond * 50}),
+				newMockController("mock2", mockRunnable{startupDuration: time.Millisecond * 20}),
+			},
+			stores: []controllermanager.Store{
+				newMockStore("mock1", mockRunnable{startupDuration: time.Millisecond * 30}),
+				newMockStore("mock2", mockRunnable{startupDuration: time.Millisecond * 60}),
+			},
+			timeout:      time.Second,
+			wantStartErr: assert.NoError,
+		},
+		{
+			name: "cancel context while starting",
+			controllers: []controllermanager.Controller{
+				newMockController("mock1", mockRunnable{startupDuration: time.Millisecond * 50}),
+				newMockController("mock2", mockRunnable{startupDuration: time.Millisecond * 20}),
+			},
+			stores: []controllermanager.Store{
+				newMockStore("mock1", mockRunnable{startupDuration: time.Millisecond * 30}),
+				newMockStore("mock2", mockRunnable{startupDuration: time.Millisecond * 60}),
+			},
+			cancelAfter:  time.Millisecond * 40,
+			timeout:      time.Second,
+			wantStartErr: assertErrorIs(context.DeadlineExceeded),
+		},
+		{
+			name: "start with leader election",
+			ctrlCfg: configv1alpha1.ControllerManagerConfigSpec{
+				LeaderElection: &configv1alpha1.LeaderElectionSpec{
+					Enabled: pointer.Bool(true),
+				},
+			},
+			defaultLeaseName: "execution-controller",
+			controllers: []controllermanager.Controller{
+				newMockController("mock", mockRunnable{}),
+			},
+			wantStartErr: assert.NoError,
+		},
+		{
+			name: "need default lease name",
+			ctrlCfg: configv1alpha1.ControllerManagerConfigSpec{
+				LeaderElection: &configv1alpha1.LeaderElectionSpec{
+					Enabled: pointer.Bool(true),
+				},
+			},
+			controllers: []controllermanager.Controller{
+				newMockController("mock", mockRunnable{}),
+			},
+			wantInitErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := mock.NewContext()
+			mgr, err := controllermanager.NewControllerManager(c, tt.ctrlCfg, tt.defaultLeaseName)
+			if (err != nil) != tt.wantInitErr {
+				t.Errorf("NewControllerManager(c, %v, %v) got err %v, wantInitErr = %v",
+					tt.ctrlCfg, tt.defaultLeaseName, err, tt.wantInitErr)
+			}
+			if err != nil {
+				return
+			}
+			for _, controller := range tt.controllers {
+				mgr.Add(controller)
+			}
+			mgr.AddStore(tt.stores...)
+			ctx := context.Background()
+			if tt.cancelAfter > 0 {
+				newCtx, cancel := context.WithTimeout(context.Background(), tt.cancelAfter)
+				defer cancel()
+				ctx = newCtx
+			}
+			tt.wantStartErr(t, mgr.Start(ctx, tt.timeout))
+		})
+	}
+}
+
+func TestControllerManager_Shutdown(t *testing.T) {
+	tests := []struct {
+		name          string
+		controllers   []*mockController
+		cancelAfter   time.Duration
+		wantInterrupt bool
+	}{
+		{
+			name: "no controllers",
+		},
+		{
+			name: "shutdown without delay",
+			controllers: []*mockController{
+				newMockController("mock", mockRunnable{}),
+			},
+		},
+		{
+			name: "shutdown interrupted",
+			controllers: []*mockController{
+				newMockController("mock1", mockRunnable{}),
+				{
+					name:             "mock2",
+					shutdownDuration: time.Millisecond * 500,
+				},
+			},
+			cancelAfter:   time.Millisecond * 50,
+			wantInterrupt: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := mock.NewContext()
+			mgr, err := controllermanager.NewControllerManager(c, configv1alpha1.ControllerManagerConfigSpec{}, "")
+			assert.NoError(t, err)
+			for _, controller := range tt.controllers {
+				mgr.Add(controller)
+			}
+			err = mgr.Start(context.Background(), 0)
+			assert.NoError(t, err)
+
+			ctx := context.Background()
+			if tt.cancelAfter > 0 {
+				newCtx, cancel := context.WithTimeout(context.Background(), tt.cancelAfter)
+				defer cancel()
+				ctx = newCtx
+			}
+			mgr.ShutdownAndWait(ctx)
+			allDone := true
+			hasInterrupt := false
+			for _, controller := range tt.controllers {
+				if controller.shutdownDone == 0 {
+					hasInterrupt = true
+					allDone = false
+				}
+			}
+			if tt.wantInterrupt {
+				assert.True(t, hasInterrupt)
+			} else {
+				assert.True(t, allDone)
+			}
+		})
+	}
+}
+
+func TestControllerManager_GetHealth(t *testing.T) {
+	c := mock.NewContext()
+	mgr, err := controllermanager.NewControllerManager(c, configv1alpha1.ControllerManagerConfigSpec{}, "")
+	assert.NoError(t, err)
+
+	// Add some controllers
+	mgr.Add(newMockController("mock1", mockRunnable{}))
+	mgr.Add(newMockController("mock2", mockRunnable{}))
+
+	// No health status before starting.
+	assert.Len(t, mgr.GetHealth(), 0)
+
+	// Populate health status after starting.
+	err = mgr.Start(context.Background(), 0)
+	assert.NoError(t, err)
+	assert.Len(t, mgr.GetHealth(), 2)
+}
+
+func assertErrorIs(target error) assert.ErrorAssertionFunc {
+	return func(t assert.TestingT, err error, i ...interface{}) bool {
+		return assert.ErrorIs(t, err, target, i...)
+	}
+}