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

feat: [CHERRY-PICK] support etcd authentication #29044

Closed
wants to merge 1 commit into from
Closed

feat: [CHERRY-PICK] support etcd authentication #29044

wants to merge 1 commit into from

Conversation

PowderLi
Copy link
Contributor

@PowderLi PowderLi commented Dec 7, 2023 •
edited
Loading

issue: #28895
master pr: #28912 #30226

add 3 configuration for ETCD config

@sre-ci-robot sre-ci-robot added the size/L Denotes a PR that changes 100-499 lines. label Dec 7, 2023
@sre-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: PowderLi
To complete the pull request process, please assign jiaoew1991 after the PR has been reviewed.
You can assign the PR to them by writing /assign @jiaoew1991 in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@mergify Mergify
Copy link
Contributor

mergify bot commented Dec 7, 2023

@PowderLi

Invalid PR Title Format Detected

Your PR submission does not adhere to our required standards. To ensure clarity and consistency, please meet the following criteria:

  1. Title Format: The PR title must begin with one of these prefixes:
  • feat: for introducing a new feature.
  • fix: for bug fixes.
  • enhance: for improvements to existing functionality.
  • test: for add tests to existing functionality.
  • doc: for modifying documentation.
  • auto: for the pull request from bot.
  1. Description Requirement: The PR must include a non-empty description, detailing the changes and their impact.

Required Title Structure:

[Type]: [Description of the PR]

Where Type is one of feat, fix, enhance, test or doc.

Example:

enhance: improve search performance significantly

Please review and update your PR to comply with these guidelines.

@mergify Mergify
Copy link
Contributor

mergify bot commented Dec 7, 2023

@PowderLi E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@PowderLi
Copy link
Contributor Author

PowderLi commented Dec 7, 2023

/run-cpu-e2e

1 similar comment
@PowderLi
Copy link
Contributor Author

PowderLi commented Dec 7, 2023

/run-cpu-e2e

@mergify Mergify
Copy link
Contributor

mergify bot commented Dec 7, 2023

@PowderLi E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@PowderLi
Copy link
Contributor Author

PowderLi commented Dec 7, 2023

/run-cpu-e2e

@mergify Mergify
Copy link
Contributor

mergify bot commented Dec 7, 2023

@PowderLi ut workflow job failed, comment rerun ut can trigger the job again.

@PowderLi PowderLi changed the title feature: [CHERRY-PICK] support etcd authentication feat: [CHERRY-PICK] support etcd authentication Dec 7, 2023
@mergify mergify bot added kind/feature Issues related to feature request from users and removed do-not-merge/invalid-pr-format labels Dec 7, 2023
@mergify Mergify
Copy link
Contributor

mergify bot commented Dec 7, 2023

@PowderLi ut workflow job failed, comment rerun ut can trigger the job again.

@mergify Mergify
Copy link
Contributor

mergify bot commented Dec 7, 2023

@PowderLi E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@PowderLi
Copy link
Contributor Author

PowderLi commented Dec 7, 2023

/run-cpu-e2e

@codecov Codecov
Copy link

codecov bot commented Dec 7, 2023 •
edited
Loading

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (4088b00) 82.13% compared to head (d121924) 82.14%.
Report is 79 commits behind head on 2.3.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##              2.3   #29044      +/-   ##
==========================================
+ Coverage   82.13%   82.14%   +0.01%     
==========================================
  Files         832      832              
  Lines      119765   119841      +76     
==========================================
+ Hits        98367    98442      +75     
- Misses      18176    18185       +9     
+ Partials     3222     3214       -8
Files Coverage Δ
internal/distributed/datacoord/service.go 87.75% <100.00%> (+0.15%) ⬆️
internal/distributed/datanode/service.go 82.11% <100.00%> (+0.24%) ⬆️
internal/distributed/indexnode/service.go 71.34% <100.00%> (+0.53%) ⬆️
internal/distributed/proxy/service.go 79.39% <100.00%> (+0.08%) ⬆️
internal/distributed/querycoord/service.go 73.99% <100.00%> (+0.35%) ⬆️
internal/distributed/querynode/service.go 80.48% <100.00%> (+0.28%) ⬆️
internal/distributed/rootcoord/service.go 79.06% <100.00%> (+0.22%) ⬆️
internal/kv/etcd/metakv_factory.go 70.73% <100.00%> (+2.31%) ⬆️
internal/util/dependency/kv/kv_client_handler.go 89.18% <100.00%> (+0.95%) ⬆️
pkg/config/etcd_source.go 73.19% <100.00%> (+0.85%) ⬆️
... and 5 more

... and 11 files with indirect coverage changes

@PowderLi
Copy link
Contributor Author

PowderLi commented Dec 7, 2023

/run-cpu-e2e

@mergify Mergify
Copy link
Contributor

mergify bot commented Dec 7, 2023

@PowderLi E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@PowderLi
Copy link
Contributor Author

PowderLi commented Dec 7, 2023

/run-cpu-e2e

@mergify mergify bot added the ci-passed label Dec 8, 2023
@PowderLi
Copy link
Contributor Author

/run-cpu-e2e

@mergify mergify bot removed the ci-passed label Dec 10, 2023
@mergify Mergify
Copy link
Contributor

mergify bot commented Dec 10, 2023

@PowderLi E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@PowderLi
Copy link
Contributor Author

/run-cpu-e2e

@mergify Mergify
Copy link
Contributor

mergify bot commented Dec 11, 2023

@PowderLi E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@mergify Mergify
Copy link
Contributor

mergify bot commented Dec 12, 2023

@PowderLi E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@PowderLi
Copy link
Contributor Author

/run-cpu-e2e

@mergify mergify bot added the ci-passed label Dec 12, 2023
@mergify mergify bot removed the ci-passed label Jan 6, 2024
@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 6, 2024

@PowderLi E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

@PowderLi
Copy link
Contributor Author

PowderLi commented Jan 6, 2024

/run-cpu-e2e

@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 6, 2024

@PowderLi ut workflow job failed, comment rerun ut can trigger the job again.

@PowderLi
feature: support etcd authentication
d121924 
add 3 configuration for ETCD config

Signed-off-by: PowderLi <min.li@zilliz.com>
@mergify Mergify
Copy link
Contributor

mergify bot commented Jan 6, 2024

@PowderLi E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

xiaofan-luan
xiaofan-luan reviewed Jan 6, 2024
View reviewed changes
internal/distributed/querynode/service.go
@@ -101,6 +101,9 @@ func (s *Server) init() error {

etcdCli, err := etcd.GetEtcdClient(
etcdConfig.UseEmbedEtcd.GetAsBool(),
etcdConfig.EtcdEnableAuth.GetAsBool(),
etcdConfig.EtcdAuthUserName.GetValue(),
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need to change all test?

by default we should not enable auth.
So GetEtcdClient do not need to add new params?

Copy link
Contributor

Choose a reason for hiding this comment

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

this pr should change limited file as possible but current change involves too much unecessary changes.

@PowderLi
Copy link
Contributor Author

PowderLi commented Jan 6, 2024

/run-cpu-e2e

@mergify mergify bot added the ci-passed label Jan 6, 2024
@mergify mergify bot added ci-passed and removed ci-passed labels Jan 23, 2024
@PowderLi PowderLi closed this Jan 25, 2024
@PowderLi PowderLi deleted the 2.3-etcd-authentication branch January 25, 2024 04:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xiaofan-luan xiaofan-luan xiaofan-luan left review comments

@aoiasd aoiasd Awaiting requested review from aoiasd

@bigsheeper bigsheeper Awaiting requested review from bigsheeper

Assignees
No one assigned
Labels
area/test ci-passed dco-passed DCO check passed. kind/feature Issues related to feature request from users sig/testing size/L Denotes a PR that changes 100-499 lines. test/integration integration test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@PowderLi @sre-ci-robot @xiaofan-luan