feat(v2): fix extensions apply #1799
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This PR has been released (on staging) and is available for download with a embedded-cluster-smoke-test-staging-app license ID. Online Installer: Airgap Installer (may take a few minutes before the airgap bundle is built): Happy debugging! |
sgalsaleh
reviewed
Feb 3, 2025
pkg/extensions/upgrade.go
Outdated
| @@ -47,6 +47,8 @@ func Upgrade(ctx context.Context, kcli client.Client, prev *ecv1beta1.Installati | |||
| // diff the extensions | |||
| diffResult := diffExtensions(prev.Spec.Config.Extensions, in.Spec.Config.Extensions) | |||
|
|
|||
| // NOTE: this does not respect order and may add extensions prior to upgrading ones with a lower order. | |||
There was a problem hiding this comment.
order is respected in the diffExtensions function per operation though. How should it be respected otherwise?
There was a problem hiding this comment.
im not sure how it should be respected. but what if an extension needed to be upgraded first because it contained a new crd for a cr that was applied from another extension that was added?
emosbaugh
force-pushed
the
emosbaugh/sc-119635/extensions-helm-values-are-not-being-properly
branch
from
83f5fd9 to
d598b1b
Compare
sgalsaleh
reviewed
Feb 4, 2025
|
|
||
| actionIng, _ := formatAction(action) | ||
|
|
||
| slog.Info(fmt.Sprintf("%s extension %s", actionIng, ext.Name)) |
There was a problem hiding this comment.
why not keep logging the version like before?
There was a problem hiding this comment.
I am just trying to make it consistent. Should we go with all structured logging?
Other instances where it is not structured:
slog.Info(fmt.Sprintf("%s already installed", ext.Name))
slog.Info(fmt.Sprintf("%s is ready!", ext.Name))
There was a problem hiding this comment.
we should go with structured logging, but slog doesn't provide wrapping out of the box. this is how i had it before:
slog.Info(fmt.Sprintf("%s extension", actionIng), "name", ext.Name, "version", ext.Version)
sgalsaleh
reviewed
Feb 4, 2025
sgalsaleh
approved these changes
Feb 4, 2025
emosbaugh
deleted the
emosbaugh/sc-119635/extensions-helm-values-are-not-being-properly
branch
emosbaugh
restored the
emosbaugh/sc-119635/extensions-helm-values-are-not-being-properly
branch
emosbaugh
deleted the
emosbaugh/sc-119635/extensions-helm-values-are-not-being-properly
branch
laverya
added a commit
that referenced
this pull request
Feb 4, 2025
* V2 upgrades without manager * overrides for ha migration * fix build * clean error message when upgrading addons and extensions * include logs in support bundle * record upgrade failures * run all v1 install tests in v2 (#1775) * remove TestInstallWithoutEmbed * use new install * check postupgrade override labels * fix extension upgrade messages * further improve wording * fix nil map issue when upgrading operator * f * Update pkg/extensions/upgrade.go * Update pkg/extensions/upgrade.go --------- Co-authored-by: Salah Al Saleh <[email protected]> * yaml v3 * handle panics * use slog * replace join with join2 (#1780) * fix build * fix ha join * fix lint (#1781) * add comments to ignore deprecated BuiltinConfigs field * fix linting * mount charts dir * feat(v2): enable v2 restore (#1776) * fix upgrades * fix TestLocalArtifactMirror and TestCommandsRequireSudo (#1782) * chore: no spinner if no terminal (#1778) * fix panic in TestHostPreflightCustomSpec * fix compile * require license for install2 * f * f * use single-node-install.sh in LAM e2e test * improve sudo test logging * ??? * see if it's just one thing failing in sudo test * use join2 command in 'node join'. Hide 'node' commands * improve how we are requiring the license flag for installation --------- Co-authored-by: Ethan Mosbaugh <[email protected]> * chore(ci): fix TestGetInstallation migrate data dirs (#1786) * feat(v2): fix e2e TestVersion (#1784) * feat(v2): fix e2e TestVersion * f * f * fix operator upgrade in airgap * fix velero airgap upgrades * fix airgap upgrades * chore(v2): e2e test multiple upgrades (#1783) * chore(v2): e2e test multiple upgrades * f * f * f * f * f * f * f * remove use of ecUpgrade2TargetVersion * f * f * f * f * f * f * f * f * f * f * f * f * f * f * f * f * f --------- Co-authored-by: Andrew Lavery <[email protected]> * fix TestHostPreflightCustomSpec and TestUnsupportedOverrides (#1787) * create an app release to test unsupported overrides with * f * simplify unsupported overrides test * update TestHostPreflightCustomSpec to use real releases * f * disable old installations * better logging * remove operator/pkg/charts as it is now unused (#1795) * remove operator/pkg/charts as it is now unused * run the operator even when the version does not match * add unit tests * feat(v2): support for restore (#1794) * feat(v2): support for restore * feat(v2): support for restore * feat(v2): support for restore * f * f * f * check chart contents directly instead of using k0s chart object (#1798) * check chart contents directly instead of using k0s chart object * simplify by no longer checking chart values, just results * feat(v2): restore ha (#1797) * feat(v2): restore ha * f * f * f * Fix dryrun tests (#1800) * disable TestOldVersionUpgrade test * feat(v2): fix extensions apply (#1799) * fix validate-success --------- Co-authored-by: Andrew Lavery <[email protected]> Co-authored-by: Ethan Mosbaugh <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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?