chore: runtime config as argument (#2329)

* chore: use runtime config

* f

* duplicated argument

* f

Author: emosbaugh

api/client/install.go

@@ -35,8 +35,8 @@ func (c *client) GetInstallationConfig() (*types.InstallationConfig, error) {
 	return &config, nil
 }
 
-func (c *client) ConfigureInstallation(cfg *types.InstallationConfig) (*types.Status, error) {
-	b, err := json.Marshal(cfg)
+func (c *client) ConfigureInstallation(config *types.InstallationConfig) (*types.Status, error) {
+	b, err := json.Marshal(config)
 	if err != nil {
 		return nil, err
 	}

api/controllers/install/controller.go

@@ -173,7 +173,6 @@ func NewInstallController(opts ...InstallControllerOption) (*InstallController,
 
 	if controller.installationManager == nil {
 		controller.installationManager = installation.NewInstallationManager(
-			installation.WithRuntimeConfig(controller.rc),
 			installation.WithLogger(controller.logger),
 			installation.WithInstallation(controller.install.Steps.Installation),
 			installation.WithLicenseFile(controller.licenseFile),
@@ -185,7 +184,6 @@ func NewInstallController(opts ...InstallControllerOption) (*InstallController,
 
 	if controller.hostPreflightManager == nil {
 		controller.hostPreflightManager = preflight.NewHostPreflightManager(
-			preflight.WithRuntimeConfig(controller.rc),
 			preflight.WithLogger(controller.logger),
 			preflight.WithMetricsReporter(controller.metricsReporter),
 			preflight.WithHostPreflightStore(preflight.NewMemoryStore(controller.install.Steps.HostPreflight)),
@@ -195,7 +193,6 @@ func NewInstallController(opts ...InstallControllerOption) (*InstallController,
 
 	if controller.infraManager == nil {
 		controller.infraManager = infra.NewInfraManager(
-			infra.WithRuntimeConfig(controller.rc),
 			infra.WithLogger(controller.logger),
 			infra.WithInfra(controller.install.Steps.Infra),
 			infra.WithPassword(controller.password),

api/controllers/install/controller_test.go

@@ -37,7 +37,7 @@ func TestGetInstallationConfig(t *testing.T) {
 				mock.InOrder(
 					m.On("GetConfig").Return(config, nil),
 					m.On("SetConfigDefaults", config).Return(nil),
-					m.On("ValidateConfig", config).Return(nil),
+					m.On("ValidateConfig", config, 9001).Return(nil),
 				)
 			},
 			expectedErr: false,
@@ -73,7 +73,7 @@ func TestGetInstallationConfig(t *testing.T) {
 				mock.InOrder(
 					m.On("GetConfig").Return(config, nil),
 					m.On("SetConfigDefaults", config).Return(nil),
-					m.On("ValidateConfig", config).Return(errors.New("validation error")),
+					m.On("ValidateConfig", config, 9001).Return(errors.New("validation error")),
 				)
 			},
 			expectedErr:   true,
@@ -85,6 +85,7 @@ func TestGetInstallationConfig(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			rc := runtimeconfig.New(nil, runtimeconfig.WithEnvSetter(&testEnvSetter{}))
 			rc.SetDataDir(t.TempDir())
+			rc.SetManagerPort(9001)
 
 			mockManager := &installation.MockInstallationManager{}
 			tt.setupMock(mockManager)
@@ -114,7 +115,7 @@ func TestConfigureInstallation(t *testing.T) {
 	tests := []struct {
 		name        string
 		config      *types.InstallationConfig
-		setupMock   func(*installation.MockInstallationManager, *types.InstallationConfig)
+		setupMock   func(*installation.MockInstallationManager, runtimeconfig.RuntimeConfig, *types.InstallationConfig)
 		expectedErr bool
 	}{
 		{
@@ -123,29 +124,29 @@ func TestConfigureInstallation(t *testing.T) {
 				LocalArtifactMirrorPort: 9000,
 				DataDirectory:           t.TempDir(),
 			},
-			setupMock: func(m *installation.MockInstallationManager, config *types.InstallationConfig) {
+			setupMock: func(m *installation.MockInstallationManager, rc runtimeconfig.RuntimeConfig, config *types.InstallationConfig) {
 				mock.InOrder(
-					m.On("ValidateConfig", config).Return(nil),
+					m.On("ValidateConfig", config, 9001).Return(nil),
 					m.On("SetConfig", *config).Return(nil),
-					m.On("ConfigureHost", t.Context()).Return(nil),
+					m.On("ConfigureHost", t.Context(), rc).Return(nil),
 				)
 			},
 			expectedErr: false,
 		},
 		{
 			name:   "validate error",
 			config: &types.InstallationConfig{},
-			setupMock: func(m *installation.MockInstallationManager, config *types.InstallationConfig) {
-				m.On("ValidateConfig", config).Return(errors.New("validation error"))
+			setupMock: func(m *installation.MockInstallationManager, rc runtimeconfig.RuntimeConfig, config *types.InstallationConfig) {
+				m.On("ValidateConfig", config, 9001).Return(errors.New("validation error"))
 			},
 			expectedErr: true,
 		},
 		{
 			name:   "set config error",
 			config: &types.InstallationConfig{},
-			setupMock: func(m *installation.MockInstallationManager, config *types.InstallationConfig) {
+			setupMock: func(m *installation.MockInstallationManager, rc runtimeconfig.RuntimeConfig, config *types.InstallationConfig) {
 				mock.InOrder(
-					m.On("ValidateConfig", config).Return(nil),
+					m.On("ValidateConfig", config, 9001).Return(nil),
 					m.On("SetConfig", *config).Return(errors.New("set config error")),
 				)
 			},
@@ -157,16 +158,16 @@ func TestConfigureInstallation(t *testing.T) {
 				GlobalCIDR:    "10.0.0.0/16",
 				DataDirectory: t.TempDir(),
 			},
-			setupMock: func(m *installation.MockInstallationManager, config *types.InstallationConfig) {
+			setupMock: func(m *installation.MockInstallationManager, rc runtimeconfig.RuntimeConfig, config *types.InstallationConfig) {
 				// Create a copy with expected CIDR values after computation
 				configWithCIDRs := *config
 				configWithCIDRs.PodCIDR = "10.0.0.0/17"
 				configWithCIDRs.ServiceCIDR = "10.0.128.0/17"
 
 				mock.InOrder(
-					m.On("ValidateConfig", config).Return(nil),
+					m.On("ValidateConfig", config, 9001).Return(nil),
 					m.On("SetConfig", configWithCIDRs).Return(nil),
-					m.On("ConfigureHost", t.Context()).Return(nil),
+					m.On("ConfigureHost", t.Context(), rc).Return(nil),
 				)
 			},
 			expectedErr: false,
@@ -177,13 +178,14 @@ func TestConfigureInstallation(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			rc := runtimeconfig.New(nil, runtimeconfig.WithEnvSetter(&testEnvSetter{}))
 			rc.SetDataDir(t.TempDir())
+			rc.SetManagerPort(9001)
 
 			mockManager := &installation.MockInstallationManager{}
 
 			// Create a copy of the config to avoid modifying the original
 			configCopy := *tt.config
 
-			tt.setupMock(mockManager, &configCopy)
+			tt.setupMock(mockManager, rc, &configCopy)
 
 			controller, err := NewInstallController(
 				WithRuntimeConfig(rc),
@@ -273,15 +275,15 @@ func TestRunHostPreflights(t *testing.T) {
 
 	tests := []struct {
 		name        string
-		setupMocks  func(*preflight.MockHostPreflightManager)
+		setupMocks  func(*preflight.MockHostPreflightManager, runtimeconfig.RuntimeConfig)
 		expectedErr bool
 	}{
 		{
 			name: "successful run preflights",
-			setupMocks: func(pm *preflight.MockHostPreflightManager) {
+			setupMocks: func(pm *preflight.MockHostPreflightManager, rc runtimeconfig.RuntimeConfig) {
 				mock.InOrder(
-					pm.On("PrepareHostPreflights", t.Context(), mock.Anything).Return(expectedHPF, nil),
-					pm.On("RunHostPreflights", t.Context(), mock.MatchedBy(func(opts preflight.RunHostPreflightOptions) bool {
+					pm.On("PrepareHostPreflights", t.Context(), rc, mock.Anything).Return(expectedHPF, nil),
+					pm.On("RunHostPreflights", t.Context(), rc, mock.MatchedBy(func(opts preflight.RunHostPreflightOptions) bool {
 						return expectedHPF == opts.HostPreflightSpec
 					})).Return(nil),
 				)
@@ -290,19 +292,19 @@ func TestRunHostPreflights(t *testing.T) {
 		},
 		{
 			name: "prepare preflights error",
-			setupMocks: func(pm *preflight.MockHostPreflightManager) {
+			setupMocks: func(pm *preflight.MockHostPreflightManager, rc runtimeconfig.RuntimeConfig) {
 				mock.InOrder(
-					pm.On("PrepareHostPreflights", t.Context(), mock.Anything).Return(nil, errors.New("prepare error")),
+					pm.On("PrepareHostPreflights", t.Context(), rc, mock.Anything).Return(nil, errors.New("prepare error")),
 				)
 			},
 			expectedErr: true,
 		},
 		{
 			name: "run preflights error",
-			setupMocks: func(pm *preflight.MockHostPreflightManager) {
+			setupMocks: func(pm *preflight.MockHostPreflightManager, rc runtimeconfig.RuntimeConfig) {
 				mock.InOrder(
-					pm.On("PrepareHostPreflights", t.Context(), mock.Anything).Return(expectedHPF, nil),
-					pm.On("RunHostPreflights", t.Context(), mock.MatchedBy(func(opts preflight.RunHostPreflightOptions) bool {
+					pm.On("PrepareHostPreflights", t.Context(), rc, mock.Anything).Return(expectedHPF, nil),
+					pm.On("RunHostPreflights", t.Context(), rc, mock.MatchedBy(func(opts preflight.RunHostPreflightOptions) bool {
 						return expectedHPF == opts.HostPreflightSpec
 					})).Return(errors.New("run preflights error")),
 				)
@@ -313,9 +315,6 @@ func TestRunHostPreflights(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			mockPreflightManager := &preflight.MockHostPreflightManager{}
-			tt.setupMocks(mockPreflightManager)
-
 			rc := runtimeconfig.New(nil)
 			rc.SetDataDir(t.TempDir())
 			rc.SetProxySpec(&ecv1beta1.ProxySpec{
@@ -325,6 +324,9 @@ func TestRunHostPreflights(t *testing.T) {
 				NoProxy:         "no-proxy.com",
 			})
 
+			mockPreflightManager := &preflight.MockHostPreflightManager{}
+			tt.setupMocks(mockPreflightManager, rc)
+
 			controller, err := NewInstallController(
 				WithRuntimeConfig(rc),
 				WithHostPreflightManager(mockPreflightManager),
@@ -566,29 +568,25 @@ func TestGetInstallationStatus(t *testing.T) {
 func TestSetupInfra(t *testing.T) {
 	tests := []struct {
 		name        string
-		setupMocks  func(*preflight.MockHostPreflightManager, *installation.MockInstallationManager, *infra.MockInfraManager, *metrics.MockReporter)
+		setupMocks  func(runtimeconfig.RuntimeConfig, *preflight.MockHostPreflightManager, *installation.MockInstallationManager, *infra.MockInfraManager, *metrics.MockReporter)
 		expectedErr bool
 	}{
 		{
 			name: "successful setup with passed preflights",
-			setupMocks: func(pm *preflight.MockHostPreflightManager, im *installation.MockInstallationManager, fm *infra.MockInfraManager, r *metrics.MockReporter) {
+			setupMocks: func(rc runtimeconfig.RuntimeConfig, pm *preflight.MockHostPreflightManager, im *installation.MockInstallationManager, fm *infra.MockInfraManager, r *metrics.MockReporter) {
 				preflightStatus := &types.Status{
 					State: types.StateSucceeded,
 				}
-				config := &types.InstallationConfig{
-					AdminConsolePort: 8000,
-				}
 				mock.InOrder(
 					pm.On("GetHostPreflightStatus", t.Context()).Return(preflightStatus, nil),
-					im.On("GetConfig").Return(config, nil),
-					fm.On("Install", t.Context(), config).Return(nil),
+					fm.On("Install", t.Context(), rc).Return(nil),
 				)
 			},
 			expectedErr: false,
 		},
 		{
 			name: "successful setup with failed preflights",
-			setupMocks: func(pm *preflight.MockHostPreflightManager, im *installation.MockInstallationManager, fm *infra.MockInfraManager, r *metrics.MockReporter) {
+			setupMocks: func(rc runtimeconfig.RuntimeConfig, pm *preflight.MockHostPreflightManager, im *installation.MockInstallationManager, fm *infra.MockInfraManager, r *metrics.MockReporter) {
 				preflightStatus := &types.Status{
 					State: types.StateFailed,
 				}
@@ -600,29 +598,25 @@ func TestSetupInfra(t *testing.T) {
 						},
 					},
 				}
-				config := &types.InstallationConfig{
-					AdminConsolePort: 8000,
-				}
 				mock.InOrder(
 					pm.On("GetHostPreflightStatus", t.Context()).Return(preflightStatus, nil),
 					pm.On("GetHostPreflightOutput", t.Context()).Return(preflightOutput, nil),
 					r.On("ReportPreflightsFailed", t.Context(), preflightOutput).Return(nil),
-					im.On("GetConfig").Return(config, nil),
-					fm.On("Install", t.Context(), config).Return(nil),
+					fm.On("Install", t.Context(), rc).Return(nil),
 				)
 			},
 			expectedErr: false,
 		},
 		{
 			name: "preflight status error",
-			setupMocks: func(pm *preflight.MockHostPreflightManager, im *installation.MockInstallationManager, fm *infra.MockInfraManager, r *metrics.MockReporter) {
+			setupMocks: func(rc runtimeconfig.RuntimeConfig, pm *preflight.MockHostPreflightManager, im *installation.MockInstallationManager, fm *infra.MockInfraManager, r *metrics.MockReporter) {
 				pm.On("GetHostPreflightStatus", t.Context()).Return(nil, errors.New("get preflight status error"))
 			},
 			expectedErr: true,
 		},
 		{
 			name: "preflight not completed",
-			setupMocks: func(pm *preflight.MockHostPreflightManager, im *installation.MockInstallationManager, fm *infra.MockInfraManager, r *metrics.MockReporter) {
+			setupMocks: func(rc runtimeconfig.RuntimeConfig, pm *preflight.MockHostPreflightManager, im *installation.MockInstallationManager, fm *infra.MockInfraManager, r *metrics.MockReporter) {
 				preflightStatus := &types.Status{
 					State: types.StateRunning,
 				}
@@ -632,7 +626,7 @@ func TestSetupInfra(t *testing.T) {
 		},
 		{
 			name: "preflight output error",
-			setupMocks: func(pm *preflight.MockHostPreflightManager, im *installation.MockInstallationManager, fm *infra.MockInfraManager, r *metrics.MockReporter) {
+			setupMocks: func(rc runtimeconfig.RuntimeConfig, pm *preflight.MockHostPreflightManager, im *installation.MockInstallationManager, fm *infra.MockInfraManager, r *metrics.MockReporter) {
 				preflightStatus := &types.Status{
 					State: types.StateFailed,
 				}
@@ -643,32 +637,15 @@ func TestSetupInfra(t *testing.T) {
 			},
 			expectedErr: true,
 		},
-		{
-			name: "get config error",
-			setupMocks: func(pm *preflight.MockHostPreflightManager, im *installation.MockInstallationManager, fm *infra.MockInfraManager, r *metrics.MockReporter) {
-				preflightStatus := &types.Status{
-					State: types.StateSucceeded,
-				}
-				mock.InOrder(
-					pm.On("GetHostPreflightStatus", t.Context()).Return(preflightStatus, nil),
-					im.On("GetConfig").Return(nil, errors.New("get config error")),
-				)
-			},
-			expectedErr: true,
-		},
 		{
 			name: "install infra error",
-			setupMocks: func(pm *preflight.MockHostPreflightManager, im *installation.MockInstallationManager, fm *infra.MockInfraManager, r *metrics.MockReporter) {
+			setupMocks: func(rc runtimeconfig.RuntimeConfig, pm *preflight.MockHostPreflightManager, im *installation.MockInstallationManager, fm *infra.MockInfraManager, r *metrics.MockReporter) {
 				preflightStatus := &types.Status{
 					State: types.StateSucceeded,
 				}
-				config := &types.InstallationConfig{
-					AdminConsolePort: 8000,
-				}
 				mock.InOrder(
 					pm.On("GetHostPreflightStatus", t.Context()).Return(preflightStatus, nil),
-					im.On("GetConfig").Return(config, nil),
-					fm.On("Install", t.Context(), config).Return(errors.New("install error")),
+					fm.On("Install", t.Context(), rc).Return(errors.New("install error")),
 				)
 			},
 			expectedErr: true,
@@ -677,13 +654,18 @@ func TestSetupInfra(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			rc := runtimeconfig.New(nil)
+			rc.SetDataDir(t.TempDir())
+			rc.SetManagerPort(9001)
+
 			mockPreflightManager := &preflight.MockHostPreflightManager{}
 			mockInstallationManager := &installation.MockInstallationManager{}
 			mockInfraManager := &infra.MockInfraManager{}
 			mockMetricsReporter := &metrics.MockReporter{}
-			tt.setupMocks(mockPreflightManager, mockInstallationManager, mockInfraManager, mockMetricsReporter)
+			tt.setupMocks(rc, mockPreflightManager, mockInstallationManager, mockInfraManager, mockMetricsReporter)
 
 			controller, err := NewInstallController(
+				WithRuntimeConfig(rc),
 				WithHostPreflightManager(mockPreflightManager),
 				WithInstallationManager(mockInstallationManager),
 				WithInfraManager(mockInfraManager),

api/controllers/install/hostpreflight.go

@@ -15,7 +15,7 @@ func (c *InstallController) RunHostPreflights(ctx context.Context, opts RunHostP
 	ecDomains := utils.GetDomains(c.releaseData)
 
 	// Prepare host preflights
-	hpf, err := c.hostPreflightManager.PrepareHostPreflights(ctx, preflight.PrepareHostPreflightOptions{
+	hpf, err := c.hostPreflightManager.PrepareHostPreflights(ctx, c.rc, preflight.PrepareHostPreflightOptions{
 		ReplicatedAppURL:      netutils.MaybeAddHTTPS(ecDomains.ReplicatedAppDomain),
 		ProxyRegistryURL:      netutils.MaybeAddHTTPS(ecDomains.ProxyRegistryDomain),
 		HostPreflightSpec:     c.releaseData.HostPreflights,
@@ -28,7 +28,7 @@ func (c *InstallController) RunHostPreflights(ctx context.Context, opts RunHostP
 	}
 
 	// Run host preflights
-	return c.hostPreflightManager.RunHostPreflights(ctx, preflight.RunHostPreflightOptions{
+	return c.hostPreflightManager.RunHostPreflights(ctx, c.rc, preflight.RunHostPreflightOptions{
 		HostPreflightSpec: hpf,
 	})
 }

api/controllers/install/infra.go

@@ -27,13 +27,7 @@ func (c *InstallController) SetupInfra(ctx context.Context) error {
 		}
 	}
 
-	// Get current installation config
-	config, err := c.installationManager.GetConfig()
-	if err != nil {
-		return fmt.Errorf("failed to read installation config: %w", err)
-	}
-
-	if err := c.infraManager.Install(ctx, config); err != nil {
+	if err := c.infraManager.Install(ctx, c.rc); err != nil {
 		return fmt.Errorf("install infra: %w", err)
 	}
 

api/controllers/install/installation.go

@@ -24,15 +24,15 @@ func (c *InstallController) GetInstallationConfig(ctx context.Context) (*types.I
 		return nil, fmt.Errorf("set defaults: %w", err)
 	}
 
-	if err := c.installationManager.ValidateConfig(config); err != nil {
+	if err := c.installationManager.ValidateConfig(config, c.rc.ManagerPort()); err != nil {
 		return nil, fmt.Errorf("validate: %w", err)
 	}
 
 	return config, nil
 }
 
 func (c *InstallController) ConfigureInstallation(ctx context.Context, config *types.InstallationConfig) error {
-	if err := c.installationManager.ValidateConfig(config); err != nil {
+	if err := c.installationManager.ValidateConfig(config, c.rc.ManagerPort()); err != nil {
 		return fmt.Errorf("validate: %w", err)
 	}
 
@@ -70,7 +70,7 @@ func (c *InstallController) ConfigureInstallation(ctx context.Context, config *t
 		return fmt.Errorf("set env vars: %w", err)
 	}
 
-	if err := c.installationManager.ConfigureHost(ctx); err != nil {
+	if err := c.installationManager.ConfigureHost(ctx, c.rc); err != nil {
 		return fmt.Errorf("configure: %w", err)
 	}