Skip to content

[etcd] use clientv3 forcibly if user requested#5128

Closed
ymmt2005 wants to merge 1 commit into
hashicorp:masterfrom
ymmt2005:force_clientv3
Closed

[etcd] use clientv3 forcibly if user requested#5128
ymmt2005 wants to merge 1 commit into
hashicorp:masterfrom
ymmt2005:force_clientv3

Conversation

@ymmt2005
Copy link
Copy Markdown
Contributor

@ymmt2005 ymmt2005 commented Aug 17, 2018

etcd storage auto detects cluster version. Unfortunately though,
the auto detection sometimes detect v3 cluster as v2 (I will open
another issue for this problem; edit #5129).

Worse, even if a user specifies the cluster version in HCL as:

storage "etcd" {
    etcd_api = "v3"
    ...
}

vault denies to start with this error message:

etcd3 is required: etcd2 is running

This commit eliminates this version check to work around the
auto detection problem because the check is in fact unnecessary
as the user explicitly requested to connect with v3 protocol.

etcd storage auto detects cluster version.  Unfortunately though,
the auto detection sometimes detect v3 cluster as v2.

Worse, even if a user specifies the cluster version in HCL as

    storage "etcd" {
        etcd_api = "v3"
        ...
    }

vault denies to start with this error message:

    etcd3 is required: etcd2 is running

This commit eliminates this version check to work around the
auto detection problem because the check is in fact unnecessary
as the user explicitly requested to connect with v3 protocol.
@jefferai
Copy link
Copy Markdown
Member

Have you reported this to the etcd project? Feels like having this check in there is good, it's the cluster reporting the wrong version that should be solved.

@ymmt2005
Copy link
Copy Markdown
Contributor Author

Not yet because we never use v2 protocol just as used in Vault when
checking the cluster version. I will take some time if it is etcd that
reported wrong cluster version.

That said, I still think the version check code is not necessary here.
Because:

  • The user explicitly requests the use of v3 protocol, and
  • clientv3.Client will fail when the cluster only supports v2 protocol anyway soon.

@ymmt2005
Copy link
Copy Markdown
Contributor Author

Additional note:

We tested Vault with this patch in house and confirmed the problem just disappeared.
Vault always can connect the v3 etcd cluster despite the reported cluster version.

@ymmt2005
Copy link
Copy Markdown
Contributor Author

ymmt2005 commented Aug 19, 2018

I have investigated the problem further and find these things:

  1. etcd v3 cluster starts with the minimal version which is "3.0.0".

    https://github.com/coreos/etcd/blob/0a25f49d6165e4c61eefb0a834fc2b5098c6f852/etcdserver/server.go#L2253-L2257

  2. The cluster version will be updated to higher version later at some point.

  3. Vault requires the cluster version >= "3.1.0" so the reported problem occurs.

Verified that the problem occurs when etcd reports its cluster version as "3.0.0".

I cannot find the reason why Vault requires >= 3.1.0 as it is just implemented so in #2299.

@ymmt2005
Copy link
Copy Markdown
Contributor Author

@jefferai What are your thoughts on this?

@jefferai
Copy link
Copy Markdown
Member

I think you should open up a bug with coreos and/or etcd. I agree that it would be nice for this to be fixed, but they wrote the Vault code in the first place, so if there is an incompatibility with their own software then it's clearly a workflow that needs to be sorted out.

@jefferai
Copy link
Copy Markdown
Member

Also where do you see the 3.1 requirement? The check in Vault looks for a remote API version of 3.

@ymmt2005
Copy link
Copy Markdown
Contributor Author

ymmt2005 commented Aug 23, 2018

Thank you for the advise. I will open an issue with etcd.

Also where do you see the 3.1 requirement? The check in Vault looks for a remote API version of 3.

Here:

if sv.LessThan(*semver.Must(semver.NewVersion("3.1.0"))) {

The function returns remote API as 2 for "3.0.0".

@hashicorp-cla
Copy link
Copy Markdown

CLA assistant check

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

Have you signed the CLA already but the status is still pending? Recheck it.

@ymmt2005
Copy link
Copy Markdown
Contributor Author

I'm closing this because:

  • We workaround the problem w/o this patch.
  • The patch conflicts with the current master.

@ymmt2005 ymmt2005 closed this Jan 15, 2019
@ymmt2005 ymmt2005 deleted the force_clientv3 branch January 15, 2019 18:15
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.

3 participants