Skip to content

refactor: drop support for etcd v2 API #162

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

Merged
merged 6 commits into from
Jun 28, 2022
Merged

Conversation

tzssangglass
Copy link
Contributor

etcd will soon drop the V2 API: etcd-io/etcd#12913

@tzssangglass tzssangglass marked this pull request as ready for review June 23, 2022 10:16
Copy link
Contributor

@membphis membphis left a comment

Choose a reason for hiding this comment

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

LGTM

local sub_ver = ver.etcdserver:sub(1, 4)
opts.api_prefix = prefix_v3[sub_ver] or "/v3beta"
if not opts.api_prefix or not utils.has_value(prefix_v3, opts.api_prefix) then
local ver, err = etcd_version(opts)
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need to detect the version. The only supported prefix now is /v3, see

["3.4."] = "/v3",

@tzssangglass
Copy link
Contributor Author

change t/v3/tls.t test case 2 response body form

--- response_body
err: 18: self signed certificate

to

--- response_body eval
qr/18: self signed certificate/

Because the step of getting the version is omitted, the self-signed certificate should be returned when getting the version, but it is returned here:

if err then
health_check.report_failure(endpoint.http_host)
return nil, endpoint.http_host .. ": " .. err
end

with endpoint.http_host

@tzssangglass tzssangglass requested a review from spacewander June 24, 2022 08:33
@@ -56,7 +56,7 @@ __DATA__
local etcd, err = require "resty.etcd" .new({
protocol = "v3",
Copy link
Contributor

Choose a reason for hiding this comment

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

if we only support v3 protocol, can we drop protocol field? it is useless.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe etcd will have a v4 protocol in the future?

Copy link
Contributor

Choose a reason for hiding this comment

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

for protocol field, the default value should be v3, all right?

@spacewander spacewander requested a review from membphis June 26, 2022 13:58
@tzssangglass
Copy link
Contributor Author

@membphis pls review again, thanks

@spacewander spacewander merged commit d4a8f92 into api7:master Jun 28, 2022
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