Test context cancel in manager and fix cleanup bug

Signed-off-by: Kimmo Lehto <[email protected]>

phase/manager.go

@@ -208,7 +208,8 @@ func (m *Manager) Run(ctx context.Context) error {
 		title := p.Title()
 
 		if err := ctx.Err(); err != nil {
-			return fmt.Errorf("context cancelled before entering phase %q: %w", title, err)
+			result = fmt.Errorf("context canceled before entering phase %q: %w", title, err)
+			return result
 		}
 
 		if p, ok := p.(withmanager); ok {

phase/manager_test.go

@@ -64,9 +64,12 @@ func TestConfigPhase(t *testing.T) {
 }
 
 type hookedPhase struct {
-	beforeCalled bool
-	afterCalled  bool
-	err          error
+	fn            func() error
+	beforeCalled  bool
+	afterCalled   bool
+	cleanupCalled bool
+	runCalled     bool
+	err           error
 }
 
 func (p *hookedPhase) Title() string {
@@ -84,7 +87,15 @@ func (p *hookedPhase) After(err error) error {
 	return nil
 }
 
+func (p *hookedPhase) CleanUp() {
+	p.cleanupCalled = true
+}
+
 func (p *hookedPhase) Run(_ context.Context) error {
+	p.runCalled = true
+	if p.fn != nil {
+		return p.fn()
+	}
 	return fmt.Errorf("run failed")
 }
 
@@ -97,3 +108,27 @@ func TestHookedPhase(t *testing.T) {
 	require.True(t, p.afterCalled, "after hook was not called")
 	require.EqualError(t, p.err, "run failed")
 }
+
+func TestContextCancel(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	m := Manager{Config: &v1beta1.Cluster{Spec: &cluster.Spec{}}}
+	p1 := &hookedPhase{fn: func() error {
+		cancel()
+		return nil
+	}}
+	p2 := &hookedPhase{}
+	m.AddPhase(p1, p2)
+	require.Error(t, m.Run(ctx))
+	require.Contains(t, ctx.Err().Error(), "cancel")
+
+	require.True(t, p1.beforeCalled, "1st before hook was not called")
+	require.True(t, p1.afterCalled, "1st after hook was not called")
+	require.True(t, p1.runCalled, "1st run was not called")
+	// this should happen because the phase was completed before the context was cancelled
+	require.True(t, p1.cleanupCalled, "1st cleanup was not called")
+
+	require.False(t, p2.beforeCalled, "2nd before hook was called")
+	require.False(t, p2.afterCalled, "2nd after hook was called")
+	require.False(t, p2.runCalled, "2nd run was called")
+	require.False(t, p2.cleanupCalled, "2nd cleanup was called")
+}