-
Notifications
You must be signed in to change notification settings - Fork 9.7k
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
pkg/fileutil: optimize file stats error #11997
Conversation
4a42a54
to
63f51aa
Compare
Codecov Report
@@ Coverage Diff @@
## master #11997 +/- ##
==========================================
+ Coverage 65.50% 65.92% +0.41%
==========================================
Files 403 403
Lines 37320 37320
==========================================
+ Hits 24447 24603 +156
+ Misses 11337 11180 -157
- Partials 1536 1537 +1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tangcong thanks for this PR. I am working on small related PR and will modify this changelog as needed but lgtm except couple of suggestions
- the changes are for both etcd data directory and the directory path when provided to automatically generate self-signed certificates for TLS connections with clients. So can you please try to cover both in the changelog?
- the desired permission is 700 on Linux but for windows it's 777 so if you can modify the text accordingly that would be great.
Thanks!
63f51aa
to
31ef82e
Compare
@spzala updated. thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tangcong thanks for quickly addressing my previous comments. It's almost lgtm, I have added couple comments inline. Thanks!
/cc @xiang90 |
31ef82e
to
9b444c6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Thanks for addressing comments!
CI failure is not related. |
@tangcong can you please cherry pick this to 3.4 and 3.3? Thanks! |
…97-origin-release-3.3 Automated cherry pick of #11997
…97-origin-release-3.4 Automated cherry pick of #11997
when I try to upgrade etcd cluster from v3.4 to v3.5, I failed to start etcd etcd process. I have to find solution from code, pr #11798 introduces this issue. it is better if we can print detail desired file permission in error log and add breaking change doc for it. thanks. @spzala