Support upgrade command for V3 #2949

screspod · 2025-09-18T20:24:10Z

What this PR does / why we need it:

Adds support for upgrade command in the V3 installation path.

Which issue(s) this PR fixes:

https://app.shortcut.com/replicated/story/128992/iteration-1-new-upgrade-command-with-enable-v3-flag

Does this PR require a test?

Unit tests were added

Does this PR require a release note?

NONE

Does this PR require documentation?

NONE

sgalsaleh · 2025-09-18T20:30:09Z

cmd/installer/cli/upgrade.go

+)
+
+type UpgradeCmdFlags struct {
+	adminConsolePort   int


can't change the admin console port on upgrade, i also don't think this is used

sgalsaleh · 2025-09-18T20:32:22Z

cmd/installer/cli/upgrade.go

+	configValues       string
+
+	// linux flags
+	dataDir              string


can't be changed on upgrade. i'd say remove

sgalsaleh · 2025-09-18T20:32:28Z

cmd/installer/cli/upgrade.go

+	dataDir              string
+	ignoreHostPreflights bool
+	ignoreAppPreflights  bool
+	networkInterface     string


sgalsaleh · 2025-09-18T20:34:32Z

cmd/installer/cli/upgrade.go

+	flagSet := pflag.NewFlagSet("linux", pflag.ContinueOnError)
+
+	// For upgrade, we'll detect the existing data directory
+	flagSet.StringVar(&flags.dataDir, "data-dir", "", "Path to the existing data directory (will be auto-detected if not provided)")


sgalsaleh · 2025-09-18T20:34:38Z

cmd/installer/cli/upgrade.go

+
+	// For upgrade, we'll detect the existing data directory
+	flagSet.StringVar(&flags.dataDir, "data-dir", "", "Path to the existing data directory (will be auto-detected if not provided)")
+	flagSet.StringVar(&flags.networkInterface, "network-interface", "", "The network interface to use for the cluster")


sgalsaleh · 2025-09-18T20:35:31Z

cmd/installer/cli/upgrade.go

+	return flagSet
+}
+
+func newKubernetesUpgradeFlags(flags *UpgradeCmdFlags) *pflag.FlagSet {


aren't these exactly the same as the install ones? let's just rename and reuse the existing one

sgalsaleh · 2025-09-18T20:36:01Z

cmd/installer/cli/upgrade.go

+	return nil
+}
+
+func addUpgradeTLSFlags(cmd *cobra.Command, flags *UpgradeCmdFlags) error {


this should come from the cluster, the user shouldn't have to provide them again on upgrade

sgalsaleh · 2025-09-18T20:36:13Z

cmd/installer/cli/upgrade.go

+	flags.kubernetesEnvSettings = s
+}
+
+func processUpgradeTLSConfig(flags *UpgradeCmdFlags) error {


github-actions · 2025-09-18T20:37:55Z

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-9328d0b" -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-9328d0b?airgap=true" -H "Authorization: $EC_SMOKE_TEST_LICENSE_ID" -o embedded-cluster-smoke-test-staging-app-ci.tgz

Happy debugging!

sgalsaleh · 2025-09-18T20:38:55Z

cmd/installer/cli/upgrade.go

+}
+
+func addUpgradeManagementConsoleFlags(cmd *cobra.Command, flags *UpgradeCmdFlags) error {
+	cmd.Flags().IntVar(&flags.managerPort, "manager-port", ecv1beta1.DefaultManagerPort, "Port on which the Manager will be served")


hmm, the default of this should ideally come from the runtimeconfig in the cluster because we store that information there.

sgalsaleh · 2025-09-18T20:40:09Z

cmd/installer/cli/upgrade.go

+	// this does not return an error - it returns the previous umask
+	_ = syscall.Umask(0o022)
+
+	// If data directory wasn't provided, try to detect it from existing installation


user shouldn't be able to provide a data dir in the upgrade command. in fact, we should disable some fields that can't be updated in the setup page. not in scope for this pr though

sgalsaleh · 2025-09-18T20:41:18Z

cmd/installer/cli/upgrade.go

+		if err == nil && existingRC.EmbeddedClusterHomeDirectory() != "" {
+			flags.dataDir = existingRC.EmbeddedClusterHomeDirectory()
+		} else {
+			// Fall back to default locations


this must come from the current runtimeconfig in the cluster, else fail

sgalsaleh · 2025-09-18T20:41:41Z

cmd/installer/cli/upgrade.go

+	}
+
+	// if a network interface flag was not provided, attempt to discover it
+	if flags.networkInterface == "" {


this must come from the runtimeconfig in the cluster, else fail

sgalsaleh · 2025-09-18T20:42:22Z

cmd/installer/cli/upgrade.go

+	return nil
+}
+
+func preRunUpgradeKubernetes(_ *cobra.Command, flags *UpgradeCmdFlags, _ kubernetesinstallation.Installation) error {


this is exactly the same logic as the install one, i think we should rename / reuse

sgalsaleh · 2025-09-18T20:44:14Z

cmd/installer/cli/upgrade.go

+	if err == nil {
+		// Parse admin console port from config if available
+		// This would depend on the actual structure of the config
+		flags.adminConsolePort = ecv1beta1.DefaultAdminConsolePort // fallback


this should come from the runtimeconfig in the cluster, not this configmap

cmd/installer/cli/upgrade.go

cmd/installer/cli/upgrade_test.go

sgalsaleh · 2025-09-18T20:48:33Z

cmd/installer/cli/upgrade_test.go

+-----END PRIVATE KEY-----`
+)
+
+func Test_readKotsadmPasswordSecret(t *testing.T) {


what is this test testing? i don't see it calling any real function? it's just reading data from a fake client.

sgalsaleh · 2025-09-18T20:49:05Z

cmd/installer/cli/upgrade_test.go

+	}
+}
+
+func Test_verifyExistingInstallation(t *testing.T) {


ditto, what is this exactly testing?

sgalsaleh · 2025-09-18T20:49:17Z

cmd/installer/cli/upgrade_test.go

+	}
+}
+
+func Test_readKotsadmTLSSecret(t *testing.T) {


cursor · 2025-09-20T22:56:39Z

cmd/installer/cli/upgrade.go

+	// default value is the manager port from the runtime config
+	cmd.Flags().IntVar(&flags.managerPort, "manager-port", upgradeConfig.managerPort, "Port on which the Manager will be served")
+	return nil
+}


Bug: Manager Port Flag Ignored

The manager-port flag defaults to 0 because upgradeConfig.managerPort is uninitialized when the flag is defined. The correct manager port is read from the runtime config later in preRunUpgrade. This also means any user-provided value for the flag is ignored, as the code uses upgradeConfig.managerPort directly.

Add upgrade command for V3

2b178e0

screspod added the type::feature label Sep 18, 2025

sgalsaleh reviewed Sep 18, 2025

View reviewed changes

cmd/installer/cli/upgrade.go Outdated Show resolved Hide resolved

sgalsaleh reviewed Sep 18, 2025

View reviewed changes

cmd/installer/cli/upgrade_test.go Outdated Show resolved Hide resolved

sgalsaleh reviewed Sep 18, 2025

View reviewed changes

cmd/installer/cli/upgrade_test.go Outdated

}

}

func Test_readKotsadmTLSSecret(t *testing.T) {

Copy link

Member

sgalsaleh Sep 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

f

9328d0b

screspod marked this pull request as ready for review September 20, 2025 22:53

cursor bot reviewed Sep 20, 2025

View reviewed changes

Support upgrade command for V3 #2949

Are you sure you want to change the base?

Support upgrade command for V3 #2949

Conversation

screspod commented Sep 18, 2025

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!

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!

github-actions bot commented Sep 18, 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!

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!

cursor bot Sep 20, 2025

Choose a reason for hiding this comment

Bug: Manager Port Flag Ignored

Uh oh!

Uh oh!

github-actions bot commented Sep 18, 2025 •

edited

Loading