Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bump etcd client api dep #8037

Merged
merged 1 commit into from Jan 29, 2020
Merged

Bump etcd client api dep #8037

merged 1 commit into from Jan 29, 2020

Conversation

michelvocks
Copy link
Contributor

@michelvocks michelvocks commented Dec 17, 2019 •
edited

Fixes #4961
Fixes #4349
Fixes #7931
Fixes #7582
Related PR: #7403

This PR bumps the version of the etcd client SDK to V3.4.3 (2019-10-24) which requires additional dependency updates to google.golang.org/api and other.

This PR has been successfully tested against a 3-node etcd cluster (V3.3.18).

@michelvocks michelvocks added this to the 1.4 milestone Dec 17, 2019
@michelvocks michelvocks mentioned this pull request Dec 17, 2019
Closed
jefferai
jefferai requested changes Dec 17, 2019
View reviewed changes
Copy link
Member

@jefferai jefferai left a comment

Choose a reason for hiding this comment

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

I have a much, much smaller diff when I do this locally, and I don't need any replace directive.

@jefferai
Copy link
Member

The only two things I think you need to run here are

go get github.com/hashicorp/vault-plugin-secrets-gcp@master
go get go.etcd.io/etcd@3cf2f69b5738fb702ba1a935590f36b52b18979b

Which for me results in a pretty minimal and reasonable diff.

@michelvocks
Copy link
Contributor Author

The only two things I think you need to run here are

go get github.com/hashicorp/vault-plugin-secrets-gcp@master
go get go.etcd.io/etcd@3cf2f69b5738fb702ba1a935590f36b52b18979b

Which for me results in a pretty minimal and reasonable diff.

I started with go get -u go.etcd.io/etcd@v3.4.3 which apparently produces a complete different output. 😖

Thanks tho!

@jefferai
Copy link
Member

Don't use -u with go modules, it doesn't work the way you think it does. :-)

catsby
catsby previously approved these changes Dec 20, 2019
View reviewed changes
Copy link
Member

@catsby catsby left a comment •
edited

Choose a reason for hiding this comment

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

Looks good 👍 - looks like there are some conflicts in go.sum that need to be resolved though

@michelvocks
Copy link
Contributor Author

@catsby Thanks. Fixed.

@michelvocks
Copy link
Contributor Author

@catsby @jefferai Would be great to get another look on this to avoid additional future rebases.

catsby
catsby previously approved these changes Jan 14, 2020
View reviewed changes
Copy link
Member

@catsby catsby left a comment

Choose a reason for hiding this comment

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

LGTM 👍

jefferai
jefferai requested changes Jan 14, 2020
View reviewed changes
Copy link
Member

@jefferai jefferai left a comment

Choose a reason for hiding this comment

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

I get a very different set of updates when I do this. There are clearly changes here like the GCP plugin version that should not be getting updated just with an etcd client dep update. And others that I get in my diff that are not shown as being updated here.

@jefferai
Copy link
Member

What version of Go are you using?

@jefferai
Copy link
Member

I forgot about the GCP bit up above. That fixes/explains that issue. I still have differences though -- I am guessing that is due to Go 1.13.x vs 1.12?

@michelvocks
Copy link
Contributor Author

I forgot about the GCP bit up above. That fixes/explains that issue. I still have differences though -- I am guessing that is due to Go 1.13.x vs 1.12?

Yes, I was still using go 1.12.7. I upgraded to 1.13.6 and reran the go mod commands.

@jefferai
Copy link
Member

That looks better but I think you forgot to run go mod tidy :-)

@michelvocks
Copy link
Contributor Author

Now with go mod tidy.

@michelvocks
Copy link
Contributor Author

Rebased. @jefferai @catsby

@michelvocks michelvocks merged commit 96ff398 into master Jan 29, 2020
@michelvocks michelvocks deleted the upgrade_etcd_deps branch January 29, 2020 14:16
catsby added a commit that referenced this pull request Feb 4, 2020
@catsby
Merge branch 'b-fix-mysql-lock-panic' of github.com:hashicorp/vault i…
575c967 
…nto b-fix-mysql-lock-panic

* 'b-fix-mysql-lock-panic' of github.com:hashicorp/vault: (52 commits)
  Fix minor typo in doc string (#8277)
  Update gen_openapi.sh (#8273)
  update dependencies (#8271)
  docs: update vault k8s to 0.2.0 (#8269)
  Fix flaky test of api renewer by moving away from legacy api. (#8265)
  Clean AlibabaCloud physical backend code (#8186)
  Update GH issue template to point to forum (#8226)
  Fix default max_open_connections for db plugins (#8262)
  Fix broken link (#8259)
  Removing timing-dependent aspects of test. (#8261)
  Changelog++
  Added flag to disable X-Vault-Token header proxy if client passes the token (#8101)
  changelog++
  changelog++
  test: fix TestAgent_Template_Basic (#8257)
  docs: fix api path for merge entity identity doc (#8258)
  Bump etcd client API dep (#8037)
  Add Consul TLS options to access API endpoint (#8253)
  Docs: Add nomad TLS options (#8254)
  Update CHANGELOG.md
  ...
@michelvocks michelvocks mentioned this pull request Feb 7, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jefferai jefferai jefferai approved these changes

@catsby catsby Awaiting requested review from catsby

Assignees
No one assigned
Labels
storage/etcd
Projects
None yet
Milestone
1.4
3 participants
@michelvocks @jefferai @catsby