add validation to k8s version, improve error messages #26
Conversation
|
Ref issue #23 |
cmd/down.go
Outdated
| } | ||
| if version.Major <= 1 && version.Minor < 7 { | ||
| fmt.Println("Warning: Kubernetes with RBAC enabled (v1.7+) is required to run Kubeapps") | ||
| os.Exit(0) |
There was a problem hiding this comment.
the exit code should be different than 0 to return an error. Also, why exiting instead of returning the error?
There was a problem hiding this comment.
Actually it isn't an error. We just want to send a warning message and quit the installer gently.
There was a problem hiding this comment.
but it should be an error, I mean if I want to deploy kubeapps and the command doesn't succeed the exit code shouldn't be 0
There was a problem hiding this comment.
I agree with @andresmgot , this should be considered an error
There was a problem hiding this comment.
Okie I turned to an error message in the follow-up commit. Please @arapulido @andresmgot take a look
cmd/root.go
Outdated
| Short: "Kubeapps Installer manages to install Kubeapps components to your cluster", | ||
| Use: "kubeapps", | ||
| Short: "Kubeapps Installer manages to install Kubeapps components to your cluster", | ||
| SilenceErrors: true, |
There was a problem hiding this comment.
It's just a cobra's tactic for silencing errors down stream. Otherwise the error message will appear twice like this:
Error: the server could not find the requested resource
ERRO[0000] the server could not find the requested resource
cmd/up.go
Outdated
| } | ||
|
|
||
| return c.Run(objs, wd) | ||
| fmt.Println("Successfully installed kubeapps.") |
There was a problem hiding this comment.
this gives the impression that kubeapps is now ready to be used (which is not true since it will be deploying for a while). I would set here a message that explains what to do next: monitor the deployment or wait for something.
There was a problem hiding this comment.
Ok so that probably should be directing user to run $ kubectl get po --all-namespaces -w for monitoring the deployment progress. WDYT ?
There was a problem hiding this comment.
maybe it is better to show the deployments?
There was a problem hiding this comment.
Working on a separate PR #52
ngtuna
force-pushed
the
1.7-
branch
2 times, most recently
from
92b812f to
a5f1da8
Compare
|
This PR is pending a bit for this one vmware-archive/manifest#7 to be tackled |
|
I would like to merge this PR as the last part of it is displaying output for kubeapps up is tackled on the separate PR #52. Ping @andresmgot |
add created-by=kubeapps label
This PR adds validation to k8s version before installing/uninstalling kubeapps and returns a warning message to let user aware of using k8s 1.7+ with RBAC enabled. This also contains several error messages which turns the tool to become more friendly.