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

Handle invalid k8s versions at startup #5199

Merged
merged 7 commits into from
Jul 24, 2023

Conversation

ericpromislow
Copy link
Contributor

@ericpromislow ericpromislow commented Jul 19, 2023

Fixes #4952

First, the invalid version can come from a deployment profile, the command-line, or be manually written into an existing settings file. All of them are handled the same way.

Currently we quietly change an invalid version to the default k8s version, usually the most recent stable version.

The first change here is to log this change to kube.log. Also this code was made shared between lima and wsl, so this should all work on Windows.

The second change is to prevent mysterious TypeError: Invalid version messages from propagating to a UI dialog box. This was caused by availableVersions.find(v => v.compare(version) === 0). If version is null, this will throw an uncaught TypeError exception.

The third change is to add a bats test to verify this works.

@ericpromislow ericpromislow marked this pull request as draft July 20, 2023 16:50
@ericpromislow ericpromislow marked this pull request as ready for review July 20, 2023 17:34
@gaktive gaktive requested a review from rak-phillip July 21, 2023 18:09
Comment on lines 156 to 164
// If we're here either there's no existing cfg.k8s.version, or it isn't valid
if (!availableVersions.length) {
if (currentConfigVersionString) {
console.log(invalidK8sVersionMainMessage);
} else {
console.log('Internal error: no available kubernetes versions found.');
}
throw new Error('No kubernetes version available.');
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to move the check for availableVersions to the beginning of this function? It looks to me like we can't do anything without a collection of avaialableVersions so we'd be able to handle the error case without having to go through the above block first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point. Let's me break two if blocks into one.

This also traps `InvalidVersion` errors when the k8s.version
field is invalid or empty. Hard to find because it came out of
an `Array.find()` callback method.

Invalid versions are logged to kube.log but aren't treated
as fatal errors.

Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
Keep logging if no-modal-dialogs is specified.

Signed-off-by: Eric Promislow <epromislow@suse.com>
Signed-off-by: Eric Promislow <epromislow@suse.com>
Copy link
Contributor

@rak-phillip rak-phillip left a comment

Choose a reason for hiding this comment

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

Looks good!

@ericpromislow ericpromislow merged commit 6d6d49e into main Jul 24, 2023
14 checks passed
@ericpromislow ericpromislow deleted the 4952-handle-invalid-k8s-version branch July 24, 2023 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deployment profiles are ignoring Kubernetes version wrong data type
2 participants