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

Add a k3s data directory location specified by the cli #7791

Merged
merged 1 commit into from
Jul 3, 2023

Conversation

zhaileilei123
Copy link
Contributor

@zhaileilei123 zhaileilei123 commented Jun 16, 2023

Proposed Changes

Add a cli to specify the self-holding of the k3s certificate rotate-ca command for the k3s custom data directory

Types of Changes

Just add a cli to cert.go, specifying the k3s data directory location with the --data-dir parameter

Verification

Because the installation and compilation process of the compilation environment took too long, I only chose to compile and test on the server of amd64. The test results are as follows:

[root@leilei:/opt/leilei-test/k3s 15:23 $]k3s certificate rotate-ca --help
NAME:
   k3s certificate rotate-ca - Write updated k3s CA certificates to the datastore

USAGE:
   k3s certificate rotate-ca [command options] [arguments...]

OPTIONS:
   --server value, -s value    (cluster) Server to connect to (default: "https://127.0.0.1:6443") [$K3S_URL]
   --data-dir value, -d value  (data) Folder to hold state default /var/lib/rancher/k3s or ${HOME}/.rancher/k3s if not root
   --path value                Path to directory containing new CA certificates
   --force                     Force certificate replacement, even if consistency checks fail
[root@leilei:/opt/leilei-test/k3s/dist/artifacts 11:42 $]k3s certificate rotate-ca --path=/data/k3s/server/rotate-ca
INFO[0000] Acquiring lock file /var/lib/rancher/k3s/data/.lock
INFO[0000] Preparing data dir /var/lib/rancher/k3s/data/2d10fa8a16a4bc82af6f81ae09d24f83a2cb4449c9848485d59ac2826518119f
FATA[0000] open /var/lib/rancher/k3s/server/token: no such file or directory
[root@leilei:/opt/leilei-test/k3s/dist/artifacts 11:43 $]k3s certificate rotate-ca --path=/data/k3s/server/rotate-ca --data-dir=/data/k3s
certificates saved to datastore

Testing

Linked Issues

#7774

User-Facing Change

The `k3s certificate rotate-ca` command now supports the data-dir flag.

Further Comments

I don't know much about golang, so I just looked for the logic of rotating certificates, not sure if there are other problems

@zhaileilei123 zhaileilei123 requested a review from a team as a code owner June 16, 2023 07:30
Need to add a cli flag for this. Also, should probably have config file loading support for the certificate commands.

Signed-off-by: leilei.zhai <[email protected]>
@zhaileilei123
Copy link
Contributor Author

Because the demand is urgent, so I plan to try to modify it myself.

@codecov
Copy link

codecov bot commented Jun 16, 2023

Codecov Report

Patch coverage has no change and project coverage change: +4.37 🎉

Comparison is base (fe9604c) 47.14% compared to head (cd31117) 51.51%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7791      +/-   ##
==========================================
+ Coverage   47.14%   51.51%   +4.37%     
==========================================
  Files         143      143              
  Lines       14509    14509              
==========================================
+ Hits         6840     7475     +635     
+ Misses       6580     5849     -731     
- Partials     1089     1185      +96     
Flag Coverage Δ
e2etests 49.31% <ø> (?)
inttests 44.50% <ø> (ø)
unittests 19.85% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/cli/cmds/certs.go 100.00% <ø> (ø)

... and 40 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@brandond brandond left a comment

Choose a reason for hiding this comment

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

Thanks, this is perfect!

@cwayne18
Copy link
Member

This is wonderful, thank you so much for the PR! Nothing better than seeing community members fix issues that bother them :) We'll get this merged in as soon as we're out of code freeze and it should be good to go in the July patches, thanks again!

@dereknola dereknola merged commit 72d50b1 into k3s-io:master Jul 3, 2023
brandond pushed a commit to brandond/k3s that referenced this pull request Jul 7, 2023
Need to add a cli flag for this. Also, should probably have config file loading support for the certificate commands.

Signed-off-by: leilei.zhai <[email protected]>
(cherry picked from commit 72d50b1)
Signed-off-by: Brad Davidson <[email protected]>
brandond pushed a commit to brandond/k3s that referenced this pull request Jul 7, 2023
Need to add a cli flag for this. Also, should probably have config file loading support for the certificate commands.

Signed-off-by: leilei.zhai <[email protected]>
(cherry picked from commit 72d50b1)
Signed-off-by: Brad Davidson <[email protected]>
brandond pushed a commit to brandond/k3s that referenced this pull request Jul 7, 2023
Need to add a cli flag for this. Also, should probably have config file loading support for the certificate commands.

Signed-off-by: leilei.zhai <[email protected]>
(cherry picked from commit 72d50b1)
Signed-off-by: Brad Davidson <[email protected]>
brandond pushed a commit that referenced this pull request Jul 7, 2023
Need to add a cli flag for this. Also, should probably have config file loading support for the certificate commands.

Signed-off-by: leilei.zhai <[email protected]>
(cherry picked from commit 72d50b1)
Signed-off-by: Brad Davidson <[email protected]>
brandond pushed a commit that referenced this pull request Jul 7, 2023
Need to add a cli flag for this. Also, should probably have config file loading support for the certificate commands.

Signed-off-by: leilei.zhai <[email protected]>
(cherry picked from commit 72d50b1)
Signed-off-by: Brad Davidson <[email protected]>
brandond pushed a commit that referenced this pull request Jul 7, 2023
Need to add a cli flag for this. Also, should probably have config file loading support for the certificate commands.

Signed-off-by: leilei.zhai <[email protected]>
(cherry picked from commit 72d50b1)
Signed-off-by: Brad Davidson <[email protected]>
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.

4 participants