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.

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

Upgrade etcd client to v3 #1562

Merged
merged 2 commits into from
May 26, 2022

Conversation

thomasferrandiz
Copy link
Contributor

@thomasferrandiz thomasferrandiz commented May 5, 2022

Description

This PR upgrades the etcd client to v3 to solve a CVE issue in the v2 client.

The registry tests now use go.etcd.io/etcd/tests/v3/integration instead of mock_etcd.
This is why there are many new dependencies.

Todos

  • Tests: connection to a cluster using TLS
  • Documentation: should I update Documentation/upgrade.md?
  • Release note

Release Note

The v2 and v3 API of etcd are not compatible so if a live upgrade is performed, 
the flannel data stored in etcd must be migrated to be accessible by the v2 client after the upgrade of flannel.  

See the etcd documentation (https://etcd.io/docs/v3.5/op-guide/v2-migration/).

@thomasferrandiz thomasferrandiz marked this pull request as ready for review May 11, 2022 12:49
@luthermonson
Copy link
Contributor

@thomasferrandiz can you merge all the code commits into one commit e.g. squash the lint changes into the rest of the code. I also would like you to split out the go.mod, go.sum and ./vendor changes into a separate commit with commit message of go mod or something so the code changes are completely split off from the go modules changes.

@thomasferrandiz
Copy link
Contributor Author

@thomasferrandiz can you merge all the code commits into one commit e.g. squash the lint changes into the rest of the code. I also would like you to split out the go.mod, go.sum and ./vendor changes into a separate commit with commit message of go mod or something so the code changes are completely split off from the go modules changes.

Done in 6b6ba94 and 3a4cdf4.

@luthermonson
Copy link
Contributor

@thomasferrandiz merged your netlink upgrade, can you rebase this and we'll get it merged

@thomasferrandiz
Copy link
Contributor Author

@luthermonson the PR is rebased.

@luthermonson
Copy link
Contributor

@manuelbuil give this a quick review but im feeling like we're done here now that ci works

@manuelbuil
Copy link
Collaborator

If we replace etcdv2 with etcdv3, we should also change the name of the subnet/etcdv2 directory, right?

@luthermonson
Copy link
Contributor

@manuelbuil thought about this a bit too after we talked... maybe just rename to etcd? do we really need the version in the dir name?

@manuelbuil
Copy link
Collaborator

@manuelbuil thought about this a bit too after we talked... maybe just rename to etcd? do we really need the version in the dir name?

good point. I don't think we do. So we could change it to etcd

@thomasferrandiz
Copy link
Contributor Author

I thought about changing the name but it would have made the diff hard to read so I decided not to rename the folder.
I'll do it now

@manuelbuil
Copy link
Collaborator

@sjoerdsimons @zhangzhangzf I think you are users of etcd subnet. Anything against this PR? If not we will merge it

@luthermonson
Copy link
Contributor

@thomasferrandiz squash that last commit into the first and let's get this merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants