Skip to content

Expose rootless state dir under ~/.rancher/k3s/rootless#9308

Merged
brandond merged 1 commit intok3s-io:masterfrom
hinshun:feature/expose-rootless-statedir
Feb 9, 2024
Merged

Expose rootless state dir under ~/.rancher/k3s/rootless#9308
brandond merged 1 commit intok3s-io:masterfrom
hinshun:feature/expose-rootless-statedir

Conversation

@hinshun
Copy link
Copy Markdown
Contributor

@hinshun hinshun commented Jan 27, 2024

Proposed Changes

Currently, the ~/.rancher/k3s/rootless directory is created but is unused. The directory is passed to the createParentOpts but the variable is then overrided by a call to os.MkdirTemp. This makes it harder to nsenter via rootlesskit's child_pid or hit the rootlesskit's API socket.

Types of Changes

  • When running k3s in rootless mode, expose rootlesskit's state directory as ~/.rancher/k3s/rootless

Verification

  • Run k3s in rootless mode
  • List files in ~/.rancher/k3s/rootless

Linked Issues

User-Facing Change

When running k3s in rootless mode, expose rootlesskit's state directory as `~/.rancher/k3s/rootless`

@hinshun hinshun requested a review from a team as a code owner January 27, 2024 03:57
Signed-off-by: Edgar Lee <edgarhinshunlee@gmail.com>
@hinshun hinshun force-pushed the feature/expose-rootless-statedir branch from d6085fe to a7c8186 Compare January 27, 2024 03:59
@brandond
Copy link
Copy Markdown
Member

It's been a while since I poked at this, but I believe that the state dir is intentionally an unlinked tempdir, because no state should be retained when rootless k3s exits. When it is done, the ephemeral namespace that k3s is running in is cleaned up, and all the child processes are killed, so leaving that around for the next invocation would be incorrect.

Is there some other way to accomplish what you're trying to do, that doesn't involve leaving the state around afterwards? If you're just trying to use nsenter to interact with rootless k3s, there are some examples at #8512 (comment)

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 30, 2024

Codecov Report

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

Comparison is base (ce6158f) 49.07% compared to head (a7c8186) 40.54%.
Report is 184 commits behind head on master.

Files Patch % Lines
pkg/etcd/snapshot.go 35.15% 439 Missing and 72 partials ⚠️
pkg/etcd/s3.go 0.00% 233 Missing ⚠️
pkg/etcd/snapshot_controller.go 0.00% 216 Missing ⚠️
pkg/etcd/etcd.go 19.65% 168 Missing and 16 partials ⚠️
pkg/spegel/spegel.go 2.98% 130 Missing ⚠️
pkg/agent/containerd/containerd.go 0.00% 116 Missing ⚠️
pkg/spegel/bootstrap.go 1.81% 108 Missing ⚠️
pkg/apis/k3s.cattle.io/v1/zz_generated_deepcopy.go 17.35% 93 Missing and 7 partials ⚠️
pkg/server/secrets-encrypt.go 0.00% 84 Missing ⚠️
...d/controllers/k3s.cattle.io/v1/etcdsnapshotfile.go 0.00% 75 Missing ⚠️
... and 51 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9308      +/-   ##
==========================================
- Coverage   49.07%   40.54%   -8.54%     
==========================================
  Files         154      154              
  Lines       16416    16551     +135     
==========================================
- Hits         8056     6710    -1346     
- Misses       7090     8693    +1603     
+ Partials     1270     1148     -122     
Flag Coverage Δ
e2etests ?
inttests 37.70% <25.65%> (-4.59%) ⬇️
unittests 14.53% <6.85%> (-0.05%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hinshun
Copy link
Copy Markdown
Contributor Author

hinshun commented Jan 31, 2024

Can we simply just _ = os.RemoveAll the rootless directory in the setup phase of rootless mode to remove any previous state? I want to have a deterministic location (be it $XDG_RUNTIME_DIR, ~/.rancher/k3s/rootless or anywhere else) where the child_pid and api.sock are available. For any deterministic location, we'll have to clean up previous state.

@brandond
Copy link
Copy Markdown
Member

Maybe just change those paths to be outside a mount namespace'd path when running rootless? then you wouldn't have to nsenter at all to get at them.

@brandond
Copy link
Copy Markdown
Member

There seems to be a bit of overlap in goals between #9309 and this PR, do you think it'd be worthwhile to just make all the changes in a single PR?

We'll also need an issue describing what the current problems are that you're solving with this PR.

@hinshun
Copy link
Copy Markdown
Contributor Author

hinshun commented Feb 2, 2024

Maybe just change those paths to be outside a mount namespace'd path when running rootless? then you wouldn't have to nsenter at all to get at them.

I believe ~/.rancher/k3s/rootless is already outside a mount namespace'd path, hence there's no changes to the list of rootless bind mounts. With this change, you don't need nsenter to access them.

There seems to be a bit of overlap in goals between #9309 and this PR, do you think it'd be worthwhile to just make all the changes in a single PR?

Happy to move them in the same PR, they do have different goals though. #9309 is for end-users to seamlessly use the rootless containerd embedded in k3s along with rootless k3s (for parity with rootful k3s + containerd), whereas this is for debugging rootless k3s (my main use case) or building tools around rootless k3s.

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