feat(api): adds target kubernetes reporting and app preflight reporting events (#2631)

* Update the installation page to show app preflight checks

* f

* f

* f

* initial timeline design

* f

* f

* fix lint

* improve unit test

* cursor feedback

* feat(api): update api reporting events

* f

* f

---------

Co-authored-by: Salah Aldeen Al Saleh <[email protected]>

Author: emosbaugh

api/controllers/app/install/controller.go

@@ -159,6 +159,7 @@ func NewInstallController(opts ...InstallControllerOption) (*InstallController,
 	if controller.appPreflightManager == nil {
 		controller.appPreflightManager = apppreflightmanager.NewAppPreflightManager(
 			apppreflightmanager.WithLogger(controller.logger),
+			apppreflightmanager.WithAppPreflightStore(controller.store.AppPreflightStore()),
 		)
 	}
 

api/controllers/kubernetes/install/controller.go

@@ -201,7 +201,7 @@ func NewInstallController(opts ...InstallControllerOption) (*InstallController,
 	if controller.infraManager == nil {
 		infraManager, err := infra.NewInfraManager(
 			infra.WithLogger(controller.logger),
-			infra.WithInfraStore(controller.store.LinuxInfraStore()),
+			infra.WithInfraStore(controller.store.KubernetesInfraStore()),
 			infra.WithRESTClientGetter(controller.restClientGetter),
 			infra.WithPassword(controller.password),
 			infra.WithTLSConfig(controller.tlsConfig),
@@ -216,6 +216,8 @@ func NewInstallController(opts ...InstallControllerOption) (*InstallController,
 		controller.infraManager = infraManager
 	}
 
+	controller.registerReportingHandlers()
+
 	return controller, nil
 }
 

api/controllers/kubernetes/install/controller_test.go

@@ -5,10 +5,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/mock"
-	"github.com/stretchr/testify/require"
-
 	appcontroller "github.com/replicatedhq/embedded-cluster/api/controllers/app/install"
 	appconfig "github.com/replicatedhq/embedded-cluster/api/internal/managers/app/config"
 	"github.com/replicatedhq/embedded-cluster/api/internal/managers/kubernetes/infra"
@@ -23,6 +19,9 @@ import (
 	"github.com/replicatedhq/embedded-cluster/pkg/release"
 	kotsv1beta1 "github.com/replicatedhq/kotskinds/apis/kots/v1beta1"
 	"github.com/replicatedhq/kotskinds/multitype"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
 )
 
 func TestGetInstallationConfig(t *testing.T) {
@@ -127,7 +126,7 @@ func TestConfigureInstallation(t *testing.T) {
 		config        types.KubernetesInstallationConfig
 		currentState  statemachine.State
 		expectedState statemachine.State
-		setupMock     func(*installation.MockInstallationManager, *kubernetesinstallation.MockInstallation, types.KubernetesInstallationConfig)
+		setupMock     func(*installation.MockInstallationManager, *kubernetesinstallation.MockInstallation, types.KubernetesInstallationConfig, *store.MockStore, *metrics.MockReporter)
 		expectedErr   bool
 	}{
 		{
@@ -140,7 +139,8 @@ func TestConfigureInstallation(t *testing.T) {
 			},
 			currentState:  states.StateApplicationConfigured,
 			expectedState: states.StateInstallationConfigured,
-			setupMock: func(m *installation.MockInstallationManager, ki *kubernetesinstallation.MockInstallation, config types.KubernetesInstallationConfig) {
+
+			setupMock: func(m *installation.MockInstallationManager, ki *kubernetesinstallation.MockInstallation, config types.KubernetesInstallationConfig, st *store.MockStore, mr *metrics.MockReporter) {
 				mock.InOrder(
 					m.On("ConfigureInstallation", mock.Anything, ki, config).Return(nil),
 				)
@@ -152,19 +152,49 @@ func TestConfigureInstallation(t *testing.T) {
 			config:        types.KubernetesInstallationConfig{},
 			currentState:  states.StateApplicationConfigured,
 			expectedState: states.StateInstallationConfigurationFailed,
-			setupMock: func(m *installation.MockInstallationManager, ki *kubernetesinstallation.MockInstallation, config types.KubernetesInstallationConfig) {
-				m.On("ConfigureInstallation", mock.Anything, ki, config).Return(errors.New("validation error"))
+			setupMock: func(m *installation.MockInstallationManager, ki *kubernetesinstallation.MockInstallation, config types.KubernetesInstallationConfig, st *store.MockStore, mr *metrics.MockReporter) {
+				mock.InOrder(
+					m.On("ConfigureInstallation", mock.Anything, ki, config).Return(errors.New("validation error")),
+					st.KubernetesInstallationMockStore.On("GetStatus").Return(types.Status{Description: "validation error"}, nil),
+					mr.On("ReportInstallationFailed", mock.Anything, errors.New("validation error")),
+				)
 			},
 			expectedErr: true,
 		},
 		{
-			name: "invalid state transition",
-			config: types.KubernetesInstallationConfig{
-				AdminConsolePort: 9000,
+			name:          "set config error on retry from already configured",
+			config:        types.KubernetesInstallationConfig{},
+			currentState:  states.StateInstallationConfigured,
+			expectedState: states.StateInstallationConfigurationFailed,
+			setupMock: func(m *installation.MockInstallationManager, ki *kubernetesinstallation.MockInstallation, config types.KubernetesInstallationConfig, st *store.MockStore, mr *metrics.MockReporter) {
+				mock.InOrder(
+					m.On("ConfigureInstallation", mock.Anything, ki, config).Return(errors.New("validation error")),
+					st.KubernetesInstallationMockStore.On("GetStatus").Return(types.Status{Description: "validation error"}, nil),
+					mr.On("ReportInstallationFailed", mock.Anything, errors.New("validation error")),
+				)
 			},
+			expectedErr: true,
+		},
+		{
+			name:          "set config error on retry from that failed to configure",
+			config:        types.KubernetesInstallationConfig{},
+			currentState:  states.StateInstallationConfigurationFailed,
+			expectedState: states.StateInstallationConfigurationFailed,
+			setupMock: func(m *installation.MockInstallationManager, ki *kubernetesinstallation.MockInstallation, config types.KubernetesInstallationConfig, st *store.MockStore, mr *metrics.MockReporter) {
+				mock.InOrder(
+					m.On("ConfigureInstallation", mock.Anything, ki, config).Return(errors.New("validation error")),
+					st.KubernetesInstallationMockStore.On("GetStatus").Return(types.Status{Description: "validation error"}, nil),
+					mr.On("ReportInstallationFailed", mock.Anything, errors.New("validation error")),
+				)
+			},
+			expectedErr: true,
+		},
+		{
+			name:          "invalid state transition",
+			config:        types.KubernetesInstallationConfig{},
 			currentState:  states.StateInfrastructureInstalling,
 			expectedState: states.StateInfrastructureInstalling,
-			setupMock: func(m *installation.MockInstallationManager, ki *kubernetesinstallation.MockInstallation, config types.KubernetesInstallationConfig) {
+			setupMock: func(m *installation.MockInstallationManager, ki *kubernetesinstallation.MockInstallation, config types.KubernetesInstallationConfig, st *store.MockStore, mr *metrics.MockReporter) {
 			},
 			expectedErr: true,
 		},
@@ -177,13 +207,17 @@ func TestConfigureInstallation(t *testing.T) {
 			sm := NewStateMachine(WithCurrentState(tt.currentState))
 
 			mockManager := &installation.MockInstallationManager{}
+			metricsReporter := &metrics.MockReporter{}
+			mockStore := &store.MockStore{}
 
-			tt.setupMock(mockManager, mockInstallation, tt.config)
+			tt.setupMock(mockManager, mockInstallation, tt.config, mockStore, metricsReporter)
 
 			controller, err := NewInstallController(
 				WithInstallation(mockInstallation),
 				WithStateMachine(sm),
 				WithInstallationManager(mockManager),
+				WithStore(mockStore),
+				WithMetricsReporter(metricsReporter),
 				WithReleaseData(getTestReleaseData(&kotsv1beta1.Config{})),
 			)
 			require.NoError(t, err)
@@ -193,8 +227,6 @@ func TestConfigureInstallation(t *testing.T) {
 				assert.Error(t, err)
 			} else {
 				assert.NoError(t, err)
-
-				assert.NotEqual(t, tt.currentState, sm.CurrentState(), "state should have changed and should not be %s", tt.currentState)
 			}
 
 			assert.Eventually(t, func() bool {
@@ -203,6 +235,10 @@ func TestConfigureInstallation(t *testing.T) {
 			assert.False(t, sm.IsLockAcquired(), "state machine should not be locked after configuration")
 
 			mockManager.AssertExpectations(t)
+			metricsReporter.AssertExpectations(t)
+			mockStore.KubernetesInfraMockStore.AssertExpectations(t)
+			mockStore.KubernetesInstallationMockStore.AssertExpectations(t)
+			mockStore.AppConfigMockStore.AssertExpectations(t)
 			mockInstallation.AssertExpectations(t)
 		})
 	}
@@ -301,7 +337,6 @@ func TestSetupInfra(t *testing.T) {
 				mock.InOrder(
 					fm.On("Install", mock.Anything, ki).Return(nil),
 					// TODO: we are not yet reporting
-					// mr.On("ReportInstallationSucceeded", mock.Anything),
 				)
 			},
 			expectedErr: nil,
@@ -313,9 +348,8 @@ func TestSetupInfra(t *testing.T) {
 			setupMocks: func(ki kubernetesinstallation.Installation, im *installation.MockInstallationManager, fm *infra.MockInfraManager, mr *metrics.MockReporter, st *store.MockStore, am *appconfig.MockAppConfigManager) {
 				mock.InOrder(
 					fm.On("Install", mock.Anything, ki).Return(errors.New("install error")),
-					st.LinuxInfraMockStore.On("GetStatus").Return(types.Status{Description: "install error"}, nil),
-					// TODO: we are not yet reporting
-					// mr.On("ReportInstallationFailed", mock.Anything, errors.New("install error")),
+					st.KubernetesInfraMockStore.On("GetStatus").Return(types.Status{Description: "install error"}, nil),
+					mr.On("ReportInstallationFailed", mock.Anything, errors.New("install error")),
 				)
 			},
 			expectedErr: nil,
@@ -327,7 +361,7 @@ func TestSetupInfra(t *testing.T) {
 			setupMocks: func(ki kubernetesinstallation.Installation, im *installation.MockInstallationManager, fm *infra.MockInfraManager, mr *metrics.MockReporter, st *store.MockStore, am *appconfig.MockAppConfigManager) {
 				mock.InOrder(
 					fm.On("Install", mock.Anything, ki).Return(errors.New("install error")),
-					st.LinuxInfraMockStore.On("GetStatus").Return(nil, assert.AnError),
+					st.KubernetesInfraMockStore.On("GetStatus").Return(nil, assert.AnError),
 				)
 			},
 			expectedErr: nil,
@@ -339,9 +373,8 @@ func TestSetupInfra(t *testing.T) {
 			setupMocks: func(ki kubernetesinstallation.Installation, im *installation.MockInstallationManager, fm *infra.MockInfraManager, mr *metrics.MockReporter, st *store.MockStore, am *appconfig.MockAppConfigManager) {
 				mock.InOrder(
 					fm.On("Install", mock.Anything, ki).Panic("this is a panic"),
-					st.LinuxInfraMockStore.On("GetStatus").Return(types.Status{Description: "this is a panic"}, nil),
-					// TODO: we are not yet reporting
-					// mr.On("ReportInstallationFailed", mock.Anything, errors.New("this is a panic")),
+					st.KubernetesInfraMockStore.On("GetStatus").Return(types.Status{Description: "this is a panic"}, nil),
+					mr.On("ReportInstallationFailed", mock.Anything, errors.New("this is a panic")),
 				)
 			},
 			expectedErr: nil,

api/controllers/kubernetes/install/reporting_handlers.go

@@ -0,0 +1,86 @@
+package install
+
+import (
+	"context"
+	"errors"
+	"fmt"
+
+	"github.com/replicatedhq/embedded-cluster/api/internal/statemachine"
+	states "github.com/replicatedhq/embedded-cluster/api/internal/states/install"
+	"github.com/replicatedhq/embedded-cluster/api/types"
+)
+
+func (c *InstallController) registerReportingHandlers() {
+	c.stateMachine.RegisterEventHandler(states.StateHostConfigurationFailed, c.reportInstallFailed)
+	c.stateMachine.RegisterEventHandler(states.StateInstallationConfigurationFailed, c.reportInstallFailed)
+	c.stateMachine.RegisterEventHandler(states.StateInfrastructureInstallFailed, c.reportInstallFailed)
+	c.stateMachine.RegisterEventHandler(states.StateAppInstallFailed, c.reportInstallFailed)
+	c.stateMachine.RegisterEventHandler(states.StateSucceeded, c.reportInstallSucceeded)
+
+	// report preflight failures and bypassed
+	c.stateMachine.RegisterEventHandler(states.StateAppPreflightsFailed, c.reportAppPreflightsFailed)
+	c.stateMachine.RegisterEventHandler(states.StateAppPreflightsFailedBypassed, c.reportAppPreflightsBypassed)
+	c.stateMachine.RegisterEventHandler(states.StateAppPreflightsSucceeded, c.reportAppPreflightsSucceeded)
+}
+
+func (c *InstallController) reportInstallSucceeded(ctx context.Context, _, _ statemachine.State) {
+	c.logger.Info("Reporting metrics event install succeeded")
+	c.metricsReporter.ReportInstallationSucceeded(ctx)
+}
+
+func (c *InstallController) reportInstallFailed(ctx context.Context, _, toState statemachine.State) {
+	var status types.Status
+	var err error
+
+	switch toState {
+	case states.StateInstallationConfigurationFailed:
+		status, err = c.store.KubernetesInstallationStore().GetStatus()
+		if err != nil {
+			err = fmt.Errorf("get status from installation store: %w", err)
+		}
+	case states.StateInfrastructureInstallFailed:
+		status, err = c.store.KubernetesInfraStore().GetStatus()
+		if err != nil {
+			err = fmt.Errorf("get status from infra store: %w", err)
+		}
+	case states.StateAppInstallFailed:
+		status, err = c.store.AppInstallStore().GetStatus()
+		if err != nil {
+			err = fmt.Errorf("get status from app install store: %w", err)
+		}
+	}
+	if err != nil {
+		c.logger.WithError(err).Error("failed to report failed install")
+		return
+	}
+
+	c.logger.Info("Reporting metrics event install failed")
+	c.metricsReporter.ReportInstallationFailed(ctx, errors.New(status.Description))
+}
+
+func (c *InstallController) reportAppPreflightsFailed(ctx context.Context, _, _ statemachine.State) {
+	output, err := c.store.AppPreflightStore().GetOutput()
+	if err != nil {
+		err = fmt.Errorf("get output from app preflight store: %w", err)
+		c.logger.WithError(err).Error("failed to report app preflights failed")
+		return
+	}
+	c.logger.Info("Reporting metrics event app preflights failed")
+	c.metricsReporter.ReportAppPreflightsFailed(ctx, output)
+}
+
+func (c *InstallController) reportAppPreflightsBypassed(ctx context.Context, _, _ statemachine.State) {
+	output, err := c.store.AppPreflightStore().GetOutput()
+	if err != nil {
+		err = fmt.Errorf("get output from app preflight store: %w", err)
+		c.logger.WithError(err).Error("failed to report app preflights bypassed")
+		return
+	}
+	c.logger.Info("Reporting metrics event app preflights bypassed")
+	c.metricsReporter.ReportAppPreflightsBypassed(ctx, output)
+}
+
+func (c *InstallController) reportAppPreflightsSucceeded(ctx context.Context, _, _ statemachine.State) {
+	c.logger.Info("Reporting metrics event app preflights succeeded")
+	c.metricsReporter.ReportAppPreflightsSucceeded(ctx)
+}

api/controllers/kubernetes/install/statemachine_test.go

@@ -143,6 +143,11 @@ func TestStateMachineTransitions(t *testing.T) {
 			startState:       states.StateAppInstallFailed,
 			validTransitions: []statemachine.State{},
 		},
+		{
+			name:             `State "Succeeded" can not transition to any other state`,
+			startState:       states.StateSucceeded,
+			validTransitions: []statemachine.State{},
+		},
 	}
 
 	for _, tt := range tests {

api/controllers/linux/install/controller_test.go

@@ -5,10 +5,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/mock"
-	"github.com/stretchr/testify/require"
-
 	appcontroller "github.com/replicatedhq/embedded-cluster/api/controllers/app/install"
 	appconfig "github.com/replicatedhq/embedded-cluster/api/internal/managers/app/config"
 	apppreflightmanager "github.com/replicatedhq/embedded-cluster/api/internal/managers/app/preflight"
@@ -27,6 +23,9 @@ import (
 	kotsv1beta1 "github.com/replicatedhq/kotskinds/apis/kots/v1beta1"
 	"github.com/replicatedhq/kotskinds/multitype"
 	troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
 )
 
 var failedPreflightOutput = &types.PreflightsOutput{
@@ -266,8 +265,8 @@ func TestConfigureInstallation(t *testing.T) {
 					st.LinuxInstallationMockStore.On("SetStatus", mock.MatchedBy(func(status types.Status) bool {
 						return status.State == types.StateFailed && status.Description == "write: set config error"
 					})).Return(nil),
-					st.LinuxInstallationMockStore.On("GetStatus").Return(types.Status{Description: "validate: validation error"}, nil),
-					mr.On("ReportInstallationFailed", mock.Anything, errors.New("validate: validation error")),
+					st.LinuxInstallationMockStore.On("GetStatus").Return(types.Status{Description: "write: set config error"}, nil),
+					mr.On("ReportInstallationFailed", mock.Anything, errors.New("write: set config error")),
 				)
 			},
 			expectedErr: true,
@@ -598,7 +597,7 @@ func TestRunHostPreflights(t *testing.T) {
 					})).Return(nil),
 					pm.On("GetHostPreflightOutput", mock.Anything).Return(failedPreflightOutput, nil),
 					st.LinuxPreflightMockStore.On("GetOutput").Return(failedPreflightOutput, nil),
-					mr.On("ReportPreflightsFailed", mock.Anything, failedPreflightOutput).Return(nil),
+					mr.On("ReportHostPreflightsFailed", mock.Anything, failedPreflightOutput).Return(nil),
 				)
 			},
 			expectedErr: false,
@@ -631,7 +630,7 @@ func TestRunHostPreflights(t *testing.T) {
 					})).Return(nil),
 					pm.On("GetHostPreflightOutput", mock.Anything).Return(failedPreflightOutput, nil),
 					st.LinuxPreflightMockStore.On("GetOutput").Return(failedPreflightOutput, nil),
-					mr.On("ReportPreflightsFailed", mock.Anything, failedPreflightOutput).Return(nil),
+					mr.On("ReportHostPreflightsFailed", mock.Anything, failedPreflightOutput).Return(nil),
 				)
 			},
 			expectedErr: false,
@@ -648,7 +647,7 @@ func TestRunHostPreflights(t *testing.T) {
 					})).Return(nil),
 					pm.On("GetHostPreflightOutput", mock.Anything).Return(failedPreflightOutput, nil),
 					st.LinuxPreflightMockStore.On("GetOutput").Return(failedPreflightOutput, nil),
-					mr.On("ReportPreflightsFailed", mock.Anything, failedPreflightOutput).Return(nil),
+					mr.On("ReportHostPreflightsFailed", mock.Anything, failedPreflightOutput).Return(nil),
 				)
 			},
 			expectedErr: false,
@@ -665,7 +664,7 @@ func TestRunHostPreflights(t *testing.T) {
 					})).Return(nil),
 					pm.On("GetHostPreflightOutput", mock.Anything).Return(failedPreflightOutput, nil),
 					st.LinuxPreflightMockStore.On("GetOutput").Return(failedPreflightOutput, nil),
-					mr.On("ReportPreflightsFailed", mock.Anything, failedPreflightOutput).Return(nil),
+					mr.On("ReportHostPreflightsFailed", mock.Anything, failedPreflightOutput).Return(nil),
 				)
 			},
 			expectedErr: false,
@@ -1085,7 +1084,7 @@ func TestSetupInfra(t *testing.T) {
 			setupMocks: func(rc runtimeconfig.RuntimeConfig, pm *preflight.MockHostPreflightManager, im *installation.MockInstallationManager, fm *infra.MockInfraManager, am *appconfig.MockAppConfigManager, mr *metrics.MockReporter, st *store.MockStore) {
 				mock.InOrder(
 					st.LinuxPreflightMockStore.On("GetOutput").Return(failedPreflightOutput, nil),
-					mr.On("ReportPreflightsBypassed", mock.Anything, failedPreflightOutput),
+					mr.On("ReportHostPreflightsBypassed", mock.Anything, failedPreflightOutput),
 					fm.On("Install", mock.Anything, rc).Return(nil),
 				)
 			},