feat(ui): new manager-experience sub-command, use app-slug for data dir, respect flags as defaults #2284

emosbaugh · 2025-06-09T23:35:34Z

What this PR does / why we need it:

use the app slug as the default dir only for the manager-experience install flow
changes --manager-experience flag to sub-command to embedded-cluster install manager-experience
respects flags passed in by end user as defaults in the manager-experience config form

Which issue(s) this PR fixes:

Does this PR require a test?

Does this PR require a release note?

Does this PR require documentation?

github-actions · 2025-06-10T05:28:26Z

This PR has been released (on staging) and is available for download with a embedded-cluster-smoke-test-staging-app license ID.

Online Installer:

curl "https://staging.replicated.app/embedded/embedded-cluster-smoke-test-staging-app/ci/appver-dev-30e59d1" -H "Authorization: $EC_SMOKE_TEST_LICENSE_ID" -o embedded-cluster-smoke-test-staging-app-ci.tgz

Airgap Installer (may take a few minutes before the airgap bundle is built):

curl "https://staging.replicated.app/embedded/embedded-cluster-smoke-test-staging-app/ci-airgap/appver-dev-30e59d1?airgap=true" -H "Authorization: $EC_SMOKE_TEST_LICENSE_ID" -o embedded-cluster-smoke-test-staging-app-ci.tgz

Happy debugging!

emosbaugh · 2025-06-11T02:59:24Z

cmd/installer/cli/install.go

+	proxy, err := parseProxyFlags(cmd)
+	if err != nil {
+		return err
+	}
+	flags.proxy = proxy
+
+	if err := verifyProxyConfig(flags.proxy, prompts.New(), flags.assumeYes); err != nil {
+		return err
+	}
+	logrus.Debug("User confirmed prompt to proceed installing with `http_proxy` set and `https_proxy` unset")
+
+	if err := validateCIDRFlags(cmd); err != nil {
+		return err
+	}
+
+	// parse the various cidr flags to make sure we have exactly what we want
+	cidrCfg, err := getCIDRConfig(cmd)
+	if err != nil {
+		return fmt.Errorf("unable to determine pod and service CIDRs: %w", err)
+	}
+	flags.cidrCfg = cidrCfg
+
+	// if a network interface flag was not provided, attempt to discover it
+	if flags.networkInterface == "" {
+		autoInterface, err := newconfig.DetermineBestNetworkInterface()
+		if err == nil {
+			flags.networkInterface = autoInterface
+		}
+	}


This is probably all redundant in the api right now where defaults are set

emosbaugh · 2025-06-11T03:00:40Z

cmd/installer/cli/install.go

-
-		if err := validateCIDRFlags(cmd); err != nil {
-			return err
+		if err := updateRuntimeConfigFromInstallCmdFlags(flags, rc); err != nil {


rather than a configChan, we should just have a runtimeConfigChan and set it directly since its all computed in the api now.

I got rid of the config chan in my branch that installs everything in the api, so there won't be a need for it

emosbaugh · 2025-06-11T03:04:51Z

api/controllers/install/installation.go

@@ -34,35 +33,12 @@ func (c *InstallController) ConfigureInstallation(ctx context.Context, config *t
 		return fmt.Errorf("validate: %w", err)
 	}

-	if err := c.computeCIDRs(config); err != nil {


this was moved to the manager

emosbaugh · 2025-06-11T03:05:03Z

api/controllers/install/installation.go

-	// TODO (@team): discuss the distinction between the runtime config and the installation config
-	// update the runtime config
-	c.rc.SetDataDir(config.DataDirectory)
-	c.rc.SetLocalArtifactMirrorPort(config.LocalArtifactMirrorPort)
-	c.rc.SetAdminConsolePort(config.AdminConsolePort)


this was moved to the manager

emosbaugh · 2025-06-11T03:05:41Z

api/internal/managers/installation/manager.go

@@ -22,12 +22,11 @@ type InstallationManager interface {
 	SetStatus(status types.Status) error
 	ValidateConfig(config *types.InstallationConfig) error
 	SetConfigDefaults(config *types.InstallationConfig) error
-	ConfigureForInstall(ctx context.Context, config *types.InstallationConfig) error
+	ConfigureHost(ctx context.Context) error


this feels out of place in this manager

emosbaugh · 2025-06-11T03:06:23Z

api/controllers/install/controller_test.go

@@ -190,65 +171,6 @@ func TestConfigureInstallation(t *testing.T) {
 	}
 }

-// TestIntegrationComputeCIDRs tests the CIDR computation with real networking utility
-func TestIntegrationComputeCIDRs(t *testing.T) {


this comment is not correct. it is redundant and tested already in the utils package

emosbaugh · 2025-06-11T03:12:34Z

pkg/config/config.go

@@ -30,7 +30,7 @@ var k0sConfigPathOverride string
 func RenderK0sConfig(proxyRegistryDomain string) *k0sconfig.ClusterConfig {
 	cfg := k0sconfig.DefaultClusterConfig()
 	// Customize the default k0s configuration to our taste.
-	cfg.Name = runtimeconfig.BinaryName()
+	cfg.Name = "k0s"


i checked in the cluster and it seems to be named this regardless

sgalsaleh · 2025-06-11T05:04:49Z

api/internal/managers/installation/manager.go

 	if manager.installationStore == nil {
-		manager.installationStore = NewMemoryStore(manager.installation)
+		manager.installationStore = NewMemoryStore(manager.rc, types.NewStatus())


The status needs to be tied to the one in the "install" type i think, can't be a new one, otherwise it wouldn't update correctly there. That's why the installation object was passed down as a reference

Can you elaborate?

before this change, the manager.installation would be a pointer to the one on the install object here:

embedded-cluster/api/controllers/install/controller.go

Line 130 in 7f68707

installation.WithInstallation(controller.install.Steps.Installation),

so the changes reflect in the install object. with this change, it's creating a new status and changes wouldn't reflect in the install object.

Perhaps we need to refactor out these pointers? It is hard to understand and has side effects.

Yeah agreed

emosbaugh · 2025-06-11T13:02:12Z

operator/controllers/installation_controller.go

+					{
+						Name: "k0s",
+						VolumeSource: corev1.VolumeSource{
+							HostPath: &corev1.HostPathVolumeSource{
+								Path: runtimeconfig.K0sBinaryPath,
+								Type: ptr.To(corev1.HostPathFile),
 							},
 						},
 					},


i have no idea why im all of a sudden seeing this error

$ kubectl -n embedded-cluster logs copy-host-preflight-results-node3-4d8ks /embedded-cluster/bin/kubectl: /embedded-cluster/bin/kubectl: exec: exec: line 2: line 2: /usr/local/bin/k0s: not found

emosbaugh added the type::feature label Jun 9, 2025

emosbaugh force-pushed the emosbaugh/sc-124300/use-the-app-slug-for-the-default-data-directory branch from 883e121 to 33d8ba6 Compare June 9, 2025 23:35

emosbaugh force-pushed the emosbaugh/sc-124300/use-the-app-slug-for-the-default-data-directory branch from 99a930e to c0a2758 Compare June 10, 2025 21:13

emosbaugh requested review from sgalsaleh and JGAntunes June 11, 2025 02:52

emosbaugh added 7 commits June 10, 2025 19:52

feat(ui): use the app slug for the default data directory

38a4ba8

f

8e0ed6a

f

a655888

f

19a2cb7

f

62ffae4

f

d3634b6

f

79c94b9

emosbaugh force-pushed the emosbaugh/sc-124300/use-the-app-slug-for-the-default-data-directory branch from 314eeb0 to 79c94b9 Compare June 11, 2025 02:57

f

24a7bed

emosbaugh commented Jun 11, 2025

View reviewed changes

emosbaugh changed the title ~~feat(ui): use the app slug for the default data directory~~ feat(ui): new manager-experience sub-command, use app-slug for data dir, respect flags as defaults Jun 11, 2025

emosbaugh commented Jun 11, 2025

View reviewed changes

emosbaugh added 3 commits June 10, 2025 20:13

f

a737383

f

e644412

f

471f916

sgalsaleh reviewed Jun 11, 2025

View reviewed changes

emosbaugh added 2 commits June 11, 2025 05:53

f

a6fbaea

f

c55901e

emosbaugh commented Jun 11, 2025

View reviewed changes

emosbaugh added 5 commits June 11, 2025 06:11

f

9f86672

f

ed29a8d

f

3922c9d

f

165aad0

f

30e59d1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(ui): new manager-experience sub-command, use app-slug for data dir, respect flags as defaults #2284

feat(ui): new manager-experience sub-command, use app-slug for data dir, respect flags as defaults #2284

Uh oh!

emosbaugh commented Jun 9, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Jun 10, 2025 •

edited

Loading

Uh oh!

emosbaugh Jun 11, 2025

Uh oh!

emosbaugh Jun 11, 2025

Uh oh!

sgalsaleh Jun 11, 2025

Uh oh!

emosbaugh Jun 11, 2025

Uh oh!

emosbaugh Jun 11, 2025

Uh oh!

emosbaugh Jun 11, 2025

Uh oh!

emosbaugh Jun 11, 2025

Uh oh!

emosbaugh Jun 11, 2025

Uh oh!

sgalsaleh Jun 11, 2025

Uh oh!

emosbaugh Jun 11, 2025

Uh oh!

sgalsaleh Jun 11, 2025

Uh oh!

emosbaugh Jun 11, 2025

Uh oh!

sgalsaleh Jun 11, 2025

Uh oh!

emosbaugh Jun 11, 2025

Uh oh!

Uh oh!

feat(ui): new manager-experience sub-command, use app-slug for data dir, respect flags as defaults #2284

Are you sure you want to change the base?

feat(ui): new manager-experience sub-command, use app-slug for data dir, respect flags as defaults #2284

Uh oh!

Conversation

emosbaugh commented Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does / why we need it:

Which issue(s) this PR fixes:

Does this PR require a test?

Does this PR require a release note?

Does this PR require documentation?

Uh oh!

github-actions bot commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

emosbaugh commented Jun 9, 2025 •

edited

Loading

github-actions bot commented Jun 10, 2025 •

edited

Loading