feat(upgrade): add verification release is deployable #3136

JGAntunes · 2025-10-29T19:18:30Z

What this PR does / why we need it:

Adds verifications that the upgrade is possible.

Which issue(s) this PR fixes:

https://app.shortcut.com/replicated/story/130790/add-verification-that-release-is-deployable

Does this PR require a test?

Yes, added unit tests, we don't have a dryrun test for upgrades yet, I would imagine this would be one test we could add. Unsure if we should make it part of this PR or not.

There's still some manual tests I want to run though.

Does this PR require a release note?

NONE

Does this PR require documentation?

NONE

github-actions · 2025-10-30T13:14:32Z

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

Happy debugging!

emosbaugh · 2025-10-30T13:20:57Z

pkg-new/validation/upgradable.go

+		if len(requiredReleases) > 0 {
+			// Extract version labels from required releases
+			for _, release := range requiredReleases {
+				requiredVersions = append(requiredVersions, release.VersionLabel)


why dont you check TargetAppSequence here like you do for online?

This is wrong yes, I'm trying to figure out how the required versions are sorted and what gets added to the metadata.

Addressed it in the latest changes.

pkg-new/validation/upgradable.go

emosbaugh · 2025-10-30T13:28:32Z

pkg-new/validation/upgradable.go

+	}
+
+	// Check if minor version is being skipped
+	if targetK8s.Minor() > currentK8s.Minor()+1 {


do you also need to check for downgrade here?

I'm not doing it because we're also checking for EC version downgrades already (and block it altogether) so that code path would never be used. We could do it if we want to ensure correctness though.

emosbaugh · 2025-10-30T13:32:26Z

pkg-new/replicatedapi/client.go

+}
+
+// GetPendingReleases fetches pending releases from the Replicated API
+func (c *client) GetPendingReleases(ctx context.Context, channelID string, currentSequence int64, opts *PendingReleasesOptions) (*PendingReleasesResponse, error) {


is it worth adding reporting info to this call? i know we can add the app status, sequence, number of nodes, etc...

it could be done in a follow up

Good question... Idk 🤷 who would be the best person to talk about this reporting info, its use and what's expected?

We have other instances in the cli where we hit this endpoint without sending any data whatsoever -

embedded-cluster/cmd/installer/cli/release.go

Lines 29 to 36 in 030261b

func getCurrentAppChannelRelease(ctx context.Context, license *kotsv1beta1.License, channelID string) (*apiChannelRelease, error) {

query := url.Values{}

query.Set("selectedChannelId", channelID)

query.Set("channelSequence", "") // sending an empty string will return the latest channel release

query.Set("isSemverSupported", "true")

apiURL := replicatedAppURL()

url := fmt.Sprintf("%s/release/%s/pending?%s", apiURL, license.Spec.AppSlug, query.Encode())

cmd/installer/cli/upgrade.go

emosbaugh · 2025-10-30T13:44:19Z

cmd/installer/cli/upgrade.go

+		}
+		upgradeConfig.license = updatedLicense
+		upgradeConfig.licenseBytes = licenseBytes
+		upgradeConfig.replicatedAPIClient = replicatedAPI


this should be taken out of the conditional as it is only set in airgap mode

It's the other way around, it's only set in online mode right? And isn't that what we want? In airgap there's no point in initialising the replicated API client right?

Co-authored-by: Ethan Mosbaugh <[email protected]>

sgalsaleh · 2025-11-04T18:24:47Z

cmd/installer/cli/upgrade.go

+	}
+
+	// Perform validation
+	if err := validation.ValidateIsReleaseUpgradable(ctx, opts); err != nil {


the logic that builds these opts should also be put into its own "hop" (sub-function) and unit tested IMO because there's an external dependency at the end of this path. note: the installation object should be passed to it though so it doesn't interact with the cluster or need a fake k8s client.

@sgalsaleh, we cannot unit test this method, it has a replicated API dependency. Internally we've added a function that we unit test:

embedded-cluster/cmd/installer/cli/upgrade.go

Lines 630 to 633 in 041d8ec

} else {

if err := opts.WithOnlineRequiredReleases(ctx, upgradeConfig.replicatedAPIClient); err != nil {

return fmt.Errorf("failed to extract required releases from replidated API's pending release call: %w", err)

}

This method is unit tested:

embedded-cluster/cmd/installer/cli/upgrade.go

Lines 627 to 629 in 041d8ec

if err := opts.WithAirgapRequiredReleases(upgradeConfig.airgapMetadata); err != nil {

return fmt.Errorf("failed to extract required releases from airgap metadata: %w", err)

}

So I'm trying to understand what we need to unit test, is it this logic?

embedded-cluster/cmd/installer/cli/upgrade.go

Lines 600 to 623 in 041d8ec

// Get current and target EC/K8s versions

var currentECVersion string

if currentInstallation.Spec.Config != nil {

currentECVersion = currentInstallation.Spec.Config.Version

}

targetECVersion := versions.Version

// Build validation options

opts := validation.UpgradableOptions{

CurrentECVersion: currentECVersion,

TargetECVersion: targetECVersion,

License: upgradeConfig.license,

}

// Add current app version info if available

if upgradeConfig.currentAppVersion != nil {

opts.CurrentAppVersion = upgradeConfig.currentAppVersion.VersionLabel

opts.CurrentAppSequence = upgradeConfig.currentAppVersion.ChannelSequence

}

// Add target app version info

opts.TargetAppVersion = channelRelease.VersionLabel

opts.TargetAppSequence = channelRelease.ChannelSequence

Or we only add a test for airgap?

JGAntunes changed the title ~~Jgantunes/sc 130790/add verification that release is deployable~~ feat(upgrade): add verification release is deployable Oct 29, 2025

JGAntunes self-assigned this Oct 30, 2025

github-actions bot added the type::feature label Oct 30, 2025

JGAntunes force-pushed the jgantunes/sc-130790/add-verification-that-release-is-deployable branch from 5e75ed0 to e21fcad Compare October 30, 2025 13:26

emosbaugh requested changes Oct 30, 2025

View reviewed changes

JGAntunes and others added 3 commits November 4, 2025 11:39

feat(upgrade): add verification release is deployable

012b892

chore: tests and cleaning up code

e6d0c86

Apply suggestions from code review

2fabba8

Co-authored-by: Ethan Mosbaugh <[email protected]>

JGAntunes force-pushed the jgantunes/sc-130790/add-verification-that-release-is-deployable branch from 0f083d4 to 2fabba8 Compare November 4, 2025 11:40

JGAntunes and others added 3 commits November 4, 2025 17:24

chore: address feedback and fix airgap required version setup

df62f74

Update pkg-new/validation/upgradable.go

0249e25

Co-authored-by: Ethan Mosbaugh <[email protected]>

chore: more comments and tests

041d8ec

JGAntunes marked this pull request as ready for review November 4, 2025 18:17

JGAntunes requested a review from emosbaugh November 4, 2025 18:18

This comment was marked as outdated.

Sign in to view

chore: feedback

473d223

sgalsaleh reviewed Nov 4, 2025

View reviewed changes

	func getCurrentAppChannelRelease(ctx context.Context, license kotsv1beta1.License, channelID string) (apiChannelRelease, error) {
	query := url.Values{}
	query.Set("selectedChannelId", channelID)
	query.Set("channelSequence", "") // sending an empty string will return the latest channel release
	query.Set("isSemverSupported", "true")

	apiURL := replicatedAppURL()
	url := fmt.Sprintf("%s/release/%s/pending?%s", apiURL, license.Spec.AppSlug, query.Encode())

	} else {
	if err := opts.WithOnlineRequiredReleases(ctx, upgradeConfig.replicatedAPIClient); err != nil {
	return fmt.Errorf("failed to extract required releases from replidated API's pending release call: %w", err)
	}

	if err := opts.WithAirgapRequiredReleases(upgradeConfig.airgapMetadata); err != nil {
	return fmt.Errorf("failed to extract required releases from airgap metadata: %w", err)
	}

	// Get current and target EC/K8s versions
	var currentECVersion string
	if currentInstallation.Spec.Config != nil {
	currentECVersion = currentInstallation.Spec.Config.Version
	}

	targetECVersion := versions.Version

	// Build validation options
	opts := validation.UpgradableOptions{
	CurrentECVersion: currentECVersion,
	TargetECVersion: targetECVersion,
	License: upgradeConfig.license,
	}

	// Add current app version info if available
	if upgradeConfig.currentAppVersion != nil {
	opts.CurrentAppVersion = upgradeConfig.currentAppVersion.VersionLabel
	opts.CurrentAppSequence = upgradeConfig.currentAppVersion.ChannelSequence
	}

	// Add target app version info
	opts.TargetAppVersion = channelRelease.VersionLabel
	opts.TargetAppSequence = channelRelease.ChannelSequence

feat(upgrade): add verification release is deployable #3136

Are you sure you want to change the base?

feat(upgrade): add verification release is deployable #3136

Uh oh!

Conversation

JGAntunes commented Oct 29, 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 Oct 30, 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!

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JGAntunes commented Oct 29, 2025 •

edited

Loading

github-actions bot commented Oct 30, 2025 •

edited

Loading