-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Upgrade Bottlerocket node with explicit Version
field
#7666
Conversation
ff6f479
to
2395f48
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great catch spotting that Bottlerocket release version doesn't necessarily change with K8s version!
This inconsistency makes reasoning about nodegroup upgrades annoyingly complicated. The fix seems to work, but the code section as a whole feels hacky. I take the blame for not properly re-factoring it myself when previously working on a related issue.
I don't have a problem approving it as is, so that we un-block eksctl users, but we may want to consider adding a new ticket for refactoring this area. In general, make it easier to understand whether release version, K8s version or both are being upgraded, and why that is.
pkg/actions/nodegroup/upgrade.go
Outdated
ngResource.Version = gfnt.NewString(kubernetesVersion) | ||
} else { | ||
logger.Info("will only upgrade release version") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this log doesn't always reflect the reality, as you might upgrade to a release version corresponding to next K8s version (e.g. for AL2 images).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I agree, any suggestions? how about something like will only use release version for upgrade
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we can have two separate logs depending on which ngResource
field is set.
if ngResource.ReleaseVersion == nil {
logger.Info(fmt.Sprintf("will upgrade nodes to Kubernetes version: %s", ngResource.KubernetesVersion))
} else {
logger.Info(fmt.Sprintf("will upgrade nodes to release version: %s", ngResource.ReleaseVersion))
}
Something along these lines, wdyt?
2395f48
to
febff8e
Compare
Description
Fixes #7627
Checklist
README.md
, or theuserdocs
directory)area/nodegroup
) and kind (e.g.kind/improvement
)BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯