Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 choose latest version on upgrade #77

Merged
merged 4 commits into from
Nov 7, 2022

Conversation

jimmykarily
Copy link
Contributor

@jimmykarily jimmykarily commented Nov 4, 2022

Part of the fix for: kairos-io/kairos#249

We also need to select the last item instead of the first here: https://github.com/kairos-io/kairos/blob/c141ea17923888c1b484fdbec09925c8b2febb26/internal/agent/upgrade.go#L54

and optionally show a prompt to the user to confirm the version before doing the ugprade (unless some cli flag is passed to confirm automatically)

@mudler maybe we should exclude "rc" versions and such (everything non-stable)?

@jimmykarily jimmykarily force-pushed the 249-choose-latest-version-on-upgrade branch from cebf202 to 687f590 Compare November 4, 2022 08:45
@jimmykarily
Copy link
Contributor Author

jimmykarily commented Nov 4, 2022

Tests fail. This looks relevant: golang/go#47979 (with a link to this)

@jimmykarily jimmykarily force-pushed the 249-choose-latest-version-on-upgrade branch from fec003d to 6ee7834 Compare November 4, 2022 10:12
Copy link
Member

@mudler mudler left a comment

Choose a reason for hiding this comment

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

Looking good!

Dimitris Karakasilis added 2 commits November 4, 2022 14:22
@jimmykarily jimmykarily force-pushed the 249-choose-latest-version-on-upgrade branch from 6ee7834 to fda779b Compare November 4, 2022 12:27
Signed-off-by: Dimitris Karakasilis <[email protected]>
@jimmykarily jimmykarily force-pushed the 249-choose-latest-version-on-upgrade branch from 3c3f6e6 to e956402 Compare November 4, 2022 14:35
@jimmykarily jimmykarily marked this pull request as ready for review November 4, 2022 15:40
Copy link
Member

@mudler mudler left a comment

Choose a reason for hiding this comment

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

Looking good! Also nice add with the filtering!

@mudler mudler changed the title 249 choose latest version on upgrade 🐛 choose latest version on upgrade Nov 4, 2022
@jimmykarily
Copy link
Contributor Author

@mudler
Copy link
Member

mudler commented Nov 7, 2022

Looks good here! The failures are known ( #37 as a possible fix) and needs to be tackled separately

@mudler mudler merged commit 7cc7883 into main Nov 7, 2022
@jimmykarily
Copy link
Contributor Author

I just realized the "stable" filtering is not working. I'm trying to figure out why the tests where green. Will open another PR with a fix.

@jimmykarily
Copy link
Contributor Author

in semver, this is a pre-release v1.0.0-k3sv1.23.9-k3s1 even though it doesn't have an rc in it. I don't want to introduce any non-standard semver logic in our code and given we consider versions like the above to be stable, I think I'll just remove the filtering feature.
The alternative is to look for the rc string but that non-standard afaict.

@jimmykarily
Copy link
Contributor Author

removed here: #82

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants