Skip to content

Conversation

zhlsunshine
Copy link
Contributor

@zhlsunshine zhlsunshine commented Jun 23, 2021

make installation error details become more clear when building kubernetes client based on issue#33402

[ ] Configuration Infrastructure
[ ] Docs
[ ] Installation
[ ] Networking
[ ] Performance and Scalability
[ ] Policies and Telemetry
[ ] Security
[ ] Test and Release
[ ] User Experience
[ ] Developer Infrastructure

Pull Request Attributes

Please check any characteristics that apply to this pull request.

[ ] Does not have any user-facing changes. This may include CLI changes, API changes, behavior changes, performance improvements, etc.

@zhlsunshine zhlsunshine requested a review from a team as a code owner June 23, 2021 06:03
@google-cla google-cla bot added the cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. label Jun 23, 2021
@istio-testing istio-testing added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jun 23, 2021
@zhlsunshine zhlsunshine added the release-notes-none Indicates a PR that does not require release notes. label Jun 23, 2021
@zhlsunshine
Copy link
Contributor Author

I have done 2 test cases in my environment as below:

With wrong value of KUBECONFIG:

$ istioctl install --set profile=demo -y
Error: encounter error when creating extended kubernetes client according to given parameters, error detail:(error loading config file "/root": read /root: is a directory)

With wrong information inside config file of KUBECONFIG:

$ istioctl install --set profile=demo -y
Error: encounter error when fetching kubernetes config file and clientset according to given config path, error detail:(Get "https://10.238.154.68:6444/api?timeout=32s": dial tcp 10.238.154.68:6444: connect: connection refused)

@zhlsunshine
Copy link
Contributor Author

/test unit-tests_istio

Comment on lines 137 to 138
operation := "encounter error when creating Kubernetes client, error detail"
return wrapErrorf(operation, err)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
operation := "encounter error when creating Kubernetes client, error detail"
return wrapErrorf(operation, err)
return fmt.Errorf("create Kubernetes client: %v", err)

This follows standard patterns in the rest of the code base. When creating errors like this, we get useful messages as errors propogate up, like installation failed: create Kubernetes client: permission denied or similar

Copy link
Member

Choose a reason for hiding this comment

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

similar comment applies to the rest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@howardjohn the code style is the same now. ^_^

@istio-testing istio-testing merged commit ce557cd into istio:master Jun 29, 2021
@zhlsunshine zhlsunshine deleted the install branch June 29, 2021 03:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Set by the Google CLA bot to indicate the author of a PR has signed the Google CLA. release-notes-none Indicates a PR that does not require release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants