googlearchive / k8s-service-catalog

[DEPRECATED] Commandline tool to manage Service Catalog lifecycle and GCP Service Broker atop Kubernetes Cluster
Apache License 2.0
69 stars 31 forks source link

Add actionable error messages when users hit the confusing error #134

Closed maqiuyujoyce closed 6 years ago

maqiuyujoyce commented 6 years ago

Add actionable error messages when users hit the confusing error in sc install command. Now the output is:

$ ./output/bin/sc install generated service catalog deployment config in dir: /tmp/service-catalog895050393 WARNING: Please run kubectl create clusterrolebinding cluster-admin-binding --clusterrole=cluster-admin --user=$(gcloud config get-value account) before sc install. Service Catalog could not be installed Error: error deploying YAML files: deploy failed with output: exit status 1 :Error from server (Forbidden): error when creating "/tmp/service-catalog895050393/etcd-operator-rbac.yaml": clusterroles.rbac.authorization.k8s.io "etcd-operator" is forbidden: attempt to grant extra privileges: [PolicyRule{Resources:["etcdclusters"], APIGroups:["etcd.database.coreos.com"], Verbs:[""]} PolicyRule{Resources:["customresourcedefinitions"], APIGroups:["apiextensions.k8s.io"], Verbs:[""]} PolicyRule{Resources:["storageclasses"], APIGroups:["storage.k8s.io"], Verbs:[""]} PolicyRule{Resources:["pods"], APIGroups:[""], Verbs:[""]} PolicyRule{Resources:["services"], APIGroups:[""], Verbs:[""]} PolicyRule{Resources:["endpoints"], APIGroups:[""], Verbs:[""]} PolicyRule{Resources:["persistentvolumeclaims"], APIGroups:[""], Verbs:[""]} PolicyRule{Resources:["events"], APIGroups:[""], Verbs:[""]} PolicyRule{Resources:["deployments"], APIGroups:["apps"], Verbs:[""]}] user=&{maqiuyu@google.com [system:authenticated] map[]} ownerrules=[PolicyRule{Resources:["selfsubjectaccessreviews"], APIGroups:["authorization.k8s.io"], Verbs:["create"]} PolicyRule{NonResourceURLs:["/api" "/api/" "/apis" "/apis/" "/healthz" "/swaggerapi" "/swaggerapi/" "/version"], Verbs:["get"]}] ruleResolutionErrors=[]

Fixes #115, fixes #67, and fixes #35.

mihnjong commented 6 years ago

Can we make the error message less verbose? We can put "Info:", "Warning:", or "Error:" prefix if we need to print more than one messages in one command.

Something like:

Info: generated service catalog deployment config in dir: /tmp/service-catalog895050393
Warning: please run kubectl create clusterrolebinding cluster-admin-binding --clusterrole=cluster-admin --user=$(gcloud config get-value account) before sc install.
Error: error deploying YAML files: deploy failed with output: exit status 1 :Error from server (Forbidden): error when creating "/tmp/service-catalog895050393/etcd-operator-rbac.yaml": clusterroles.rbac.authorization.k8s.io "etcd-operator" is forbidden: attempt to grant extra privileges: [PolicyRule{Resources:["etcdclusters"], APIGroups:["etcd.database.coreos.com"], Verbs:[""]} PolicyRule{Resources:["customresourcedefinitions"], APIGroups:["apiextensions.k8s.io"], Verbs:[""]} PolicyRule{Resources:["storageclasses"], APIGroups:["storage.k8s.io"], Verbs:[""]} PolicyRule{Resources:["pods"], APIGroups:[""], Verbs:[""]} PolicyRule{Resources:["services"], APIGroups:[""], Verbs:[""]} PolicyRule{Resources:["endpoints"], APIGroups:[""], Verbs:[""]} PolicyRule{Resources:["persistentvolumeclaims"], APIGroups:[""], Verbs:[""]} PolicyRule{Resources:["events"], APIGroups:[""], Verbs:[""]} PolicyRule{Resources:["deployments"], APIGroups:["apps"], Verbs:[""]}] user=&{maqiuyu@google.com [system:authenticated] map[]} ownerrules=[PolicyRule{Resources:["selfsubjectaccessreviews"], APIGroups:["authorization.k8s.io"], Verbs:["create"]} PolicyRule{NonResourceURLs:["/api" "/api/" "/apis" "/apis/" "/healthz" "/swaggerapi" "/swaggerapi/" "/version"], Verbs:["get"]}] ruleResolutionErrors=[]

service catalog could not be installed

The last message can be the consequence of the command.

maqiuyujoyce commented 6 years ago

The last message can be the consequence of the command.

Currently, we rely on Cobra to print the error message for all the failed commands. So the "Error:" section will always printed at last. I can make a change to print the error details before printing out the consequence. But I think there will always be an "Error:" section after our last message if we return an error.

One alternative is to never return any error after failures, but instead, print the error before "return nil". To create consistent behavior, we may need to refactor all the commands.