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

Support tls/ssl connection to etcd (#7703) #17012

Merged
merged 1 commit into from May 20, 2022

Conversation

ownbylichaobao
Copy link
Contributor

Support tls/ssl connection to etcd
Signed-off-by: lichaobao 1527563274@qq.com
Resolves: #7703

@sre-ci-robot sre-ci-robot added the size/M Denotes a PR that changes 30-99 lines. label May 15, 2022
@mergify mergify bot added the dco-passed DCO check passed. label May 15, 2022
@ownbylichaobao
Copy link
Contributor Author

/assign @yanliang567

@xiaofan-luan
Copy link
Contributor

thanks for the contribution @ownbylichaobao ~ welcome!

@codecov
Copy link

codecov bot commented May 15, 2022

Codecov Report

Merging #17012 (5611a9f) into master (328ab7e) will increase coverage by 0.01%.
The diff coverage is 84.93%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #17012      +/-   ##
==========================================
+ Coverage   81.15%   81.17%   +0.01%     
==========================================
  Files         456      457       +1     
  Lines       72712    72898     +186     
==========================================
+ Hits        59011    59173     +162     
- Misses      10966    10985      +19     
- Partials     2735     2740       +5     
Impacted Files Coverage Δ
internal/util/etcd/etcd_util.go 81.63% <76.92%> (-18.37%) ⬇️
internal/querynode/segment_loader.go 68.98% <89.47%> (+0.39%) ⬆️
internal/util/paramtable/service_param.go 78.20% <100.00%> (+2.31%) ⬆️
internal/kv/rocksdb/RocksIterator.go 72.09% <0.00%> (-6.98%) ⬇️
internal/storage/minio_chunk_manager.go 68.87% <0.00%> (-6.89%) ⬇️
internal/storage/local_chunk_manager.go 75.59% <0.00%> (-5.62%) ⬇️
internal/indexnode/indexnode_mock.go 83.71% <0.00%> (-4.17%) ⬇️
internal/indexcoord/index_coord.go 76.32% <0.00%> (-2.01%) ⬇️
internal/proxy/segment.go 87.92% <0.00%> (-1.51%) ⬇️
internal/querycoord/querynode.go 69.93% <0.00%> (-1.05%) ⬇️
... and 30 more

@sre-ci-robot sre-ci-robot added size/L Denotes a PR that changes 100-499 lines. and removed size/M Denotes a PR that changes 30-99 lines. labels May 16, 2022
@mergify
Copy link
Contributor

mergify bot commented May 16, 2022

@ownbylichaobao E2e jenkins job failed, comment /run-checks can trigger the job again.

@ownbylichaobao
Copy link
Contributor Author

/run-checks

@xiaofan-luan
Copy link
Contributor

first time contributor can not run ci automatically, simply let me know when you are ready to run ci @ownbylichaobao

@mergify
Copy link
Contributor

mergify bot commented May 16, 2022

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

@ownbylichaobao
Copy link
Contributor Author

rerun ut

@github-actions
Copy link
Contributor

Hello ownbylichaobao, you are not in the organization, so you do not have the permission to rerun the workflow, please contact @milvus-io/milvus-maintainers for help.

@ownbylichaobao
Copy link
Contributor Author

ok

@ownbylichaobao
Copy link
Contributor Author

ent rerun ut can trigger the job again.

@mergify
Copy link
Contributor

mergify bot commented May 16, 2022

@ownbylichaobao E2e jenkins job failed, comment /run-checks can trigger the job again.

@ownbylichaobao
Copy link
Contributor Author

/run-checks

@ownbylichaobao
Copy link
Contributor Author

ownbylichaobao commented May 16, 2022

first time contributor can not run ci automatically, simply let me know when you are ready to run ci @ownbylichaobao

Hi, I'm ready to run ci @xiaofan-luan

@yanliang567
Copy link
Contributor

/assign @xiaofan-luan
/unassign

@xiaofan-luan
Copy link
Contributor

good job! overall is good to me, thanks @ownbylichaobao
We just wait for all test passed

@mergify
Copy link
Contributor

mergify bot commented May 17, 2022

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

@ownbylichaobao
Copy link
Contributor Author

good job! overall is good to me, thanks @ownbylichaobao We just wait for all test passed

Hi ,looks like some tests failed. I don't think that's my problem. Do you need to run again? @xiaofan-luan

@xiaofan-luan
Copy link
Contributor

good job! overall is good to me, thanks @ownbylichaobao We just wait for all test passed

Hi ,looks like some tests failed. I don't think that's my problem. Do you need to run again? @xiaofan-luan

Will do~ looks like there are some race condition on it.
I have one question, do we support --auto-tls for etcd?

@ownbylichaobao
Copy link
Contributor Author

ownbylichaobao commented May 17, 2022

good job! overall is good to me, thanks @ownbylichaobao We just wait for all test passed

Hi ,looks like some tests failed. I don't think that's my problem. Do you need to run again? @xiaofan-luan

Will do~ looks like there are some race condition on it. I have one question, do we support --auto-tls for etcd?

not support yet @xiaofan-luan Now that it looks like unit test coverage has dropped, I added some test code and resubmitted it

Signed-off-by: lichaobao <1527563274@qq.com>
@mergify
Copy link
Contributor

mergify bot commented May 17, 2022

@ownbylichaobao E2e jenkins job failed, comment /run-checks can trigger the job again.

@ownbylichaobao
Copy link
Contributor Author

/run-checks

@xiaofan-luan
Copy link
Contributor

Cool, if we can open another pr for auto-tls that would be great~ Just take a look on etcd tls is enabled and find a part about it

@ownbylichaobao
Copy link
Contributor Author

Cool, if we can open another pr for auto-tls that would be great~ Just take a look on etcd tls is enabled and find a part about it

👌

@mergify
Copy link
Contributor

mergify bot commented May 17, 2022

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

@ownbylichaobao
Copy link
Contributor Author

Hi ,it fails again. @xiaofan-luan

@ownbylichaobao
Copy link
Contributor Author

Run failed again, it looks like the download dependency crashed

@xiaofan-luan
Copy link
Contributor

Run failed again, it looks like the download dependency crashed

Will just ignore macOS pipeline for now since it's not the pr issue~
Thanks @ownbylichaobao , I will merge it

@xiaofan-luan
Copy link
Contributor

/lgtm
/approve

@sre-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ownbylichaobao, xiaofan-luan

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

The pull request process is described 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 bot removed the ci-passed label May 18, 2022
@ownbylichaobao
Copy link
Contributor Author

Run failed again, it looks like the download dependency crashed

Will just ignore macOS pipeline for now since it's not the pr issue~ Thanks @ownbylichaobao , I will merge it

👌

@mergify mergify bot removed the ci-passed label May 19, 2022
@mergify mergify bot removed the ci-passed label May 20, 2022
@xiaofan-luan xiaofan-luan merged commit 2f8a7e7 into milvus-io:master May 20, 2022
@xiaofan-luan
Copy link
Contributor

squash merge it due to the unstable of macos pipeline~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved dco-passed DCO check passed. lgtm size/L Denotes a PR that changes 100-499 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve security of Milvus
4 participants