Warn about incompatible kubectl versions. #3377
Conversation
Signed-off-by: Bruno Miguel Custodio <[email protected]>
Signed-off-by: Bruno Miguel Custodio <[email protected]>
k8s-ci-robot
added
cncf-cla: yes
|
Can one of the admins verify this patch? |
|
|
||
| // Warn the user if the version of kubectl doesn't fall within the allowed version range. | ||
| if !svKubernetesRange(svKubectlVersion) { | ||
| warn("The version of kubectl installed (%s) is not compatible with the desired Kubernetes version (%s).", svKubectlVersion, svKubernetesVersion) |
There was a problem hiding this comment.
I'd rather see this error out by default, and if necessary, we can add a --force flag to allow users to shoot themselves in the foot.
I find that users generally ignore warnings unless they have to correct for them. Incompatible kubectl versions tend to lead to very strange bug reports.
There was a problem hiding this comment.
Now that I know a bit better - I'm OK with this warning. For consistency, could you make it use console.OutStyle("warning") though?
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: bmcstdio If they are not already assigned, you can assign the PR to them by writing The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
NOTE: Merge conflicts here should be relatively trivial to merge in. |
tstromberg
added
the
needs-rebase
|
Marking closed for now, as there are merge conflicts to resolve and there hasn't been an update on this PR in some time. Please re-open if you get the chance to take a look at this again! |
This PR makes
minikubeoutput a warning message in case it detects a too-large version skew between the desired Kubernetes version and the version ofkubectlthat is found inPATH.As part of this I ran
dep ensure- even though I haven't introduced any dependencies -, and I got a bunch of updates tovendor/as a result. As I am not sure these should go in, I am adding them as a separate commit.Closes #3329.