Skip to content
This repository has been archived by the owner on Jul 30, 2021. It is now read-only.

pkg/util/etcdutil: improve etcd migration logs #481

Merged
merged 1 commit into from
Apr 28, 2017

Conversation

squat
Copy link
Contributor

@squat squat commented Apr 28, 2017

After running into issues deploying self-hosted etcd on Tectonic, I saw
lots of error logs. I thought the logs could be improved and be more
consistent.

cc @hongchaodeng

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 28, 2017
Copy link
Contributor

@hongchaodeng hongchaodeng left a comment

Choose a reason for hiding this comment

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

LGTM after nit


if err := waitEtcdClusterRunning(restClient); err != nil {
return fmt.Errorf("wait etcd cluster running failed: %v", err)
return fmt.Errorf("failed to wait for etcd cluster to be running: %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

"etcd cluster's status"

@@ -158,7 +158,7 @@ func waitEtcdClusterRunning(restclient restclient.Interface) error {
err := wait.Poll(10*time.Second, waitEtcdClusterRunningTime, func() (bool, error) {
b, err := restclient.Get().RequestURI(makeEtcdClusterURI(etcdClusterName)).DoRaw()
if err != nil {
return false, fmt.Errorf("fail to get etcdcluster: %v", err)
return false, fmt.Errorf("failed to get etcd cluster: %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

etcd cluster TPR

@@ -195,12 +195,12 @@ func waitBootEtcdRemoved(etcdServiceIP string) error {
cancel()
etcdcli.Close()
if err != nil {
glog.Errorf("fail to list member, will retry: %v", err)
glog.Errorf("failed to list members, will retry: %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

etcd members

@xiang90
Copy link
Contributor

xiang90 commented Apr 28, 2017

lgtm after addressing the minor issues hongchao pointed out. thanks!

@squat squat force-pushed the fix_migrate_logs branch from ae8c9d4 to a4e8e74 Compare April 28, 2017 22:36
After running into issues deploying self-hosted etcd on Tectonic, I saw
lots of error logs. I thought the logs could be improved and be more
consistent.
@squat
Copy link
Contributor Author

squat commented Apr 28, 2017

Thanks, @hongchaodeng and @xiang90. I addressed the nits. @yifan-gu or @pbx0 can someone with rights plz merge?

@aaronlevy aaronlevy merged commit 7247143 into kubernetes-retired:master Apr 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants