fix: ensure containerd-related directories removed on failed bootstrap/join-cluster.

[aznashwan]

fix: ensure containerd-related directories removed on failed bootstrap/join-cluster

k8sd automatically sets up some directories with the appropriate
ownership/permissions to be used by containerd in the early stages
of the bootstrap and join-cluster commands.

In the classic (non-strict) version of the k8s-snap, these
containerd directories are system-wide (e.g. /etc/containerd,
/run/containerd, etc).

Should any of the other setup steps fail after the containerd
directories were set up, the directories would still remain on
disk and thus lead to a 'partial installation' of on the host system.

This patch ensures that k8s will automatically remove any
containerd-related directories which were created in the event of
the bootstrap / join-cluster commands failing.

fix: ensure containerd Base Dir lockfile is never accidentally deleted.

The containerd Base Dir is the special path all other containerd-related
paths on the snap are derived from.

Under classic confinement and default settings, this path defaults
to the host's root (/), and thus extreme care must be taken to
not accidentally include it in k8sd's cleanup routine or the
k8s-snap's remove hook.

[berkayoz]

LGTM overall, we should probably wait and adjust for #817

[aznashwan]

@berkayoz note that it just hit me that because of the new pytest.mark.tags(tags.NIGHTLY) test tagging system the integration test doesn't get executed as part of the PR, sorry 🤦

While I believe it's better to leave that test for NIGHTLY, I've manually run the test on a 22.04 VM and I have.

I needed to add this small fix (I had forgotten to actually call socket.listen() in the contextmanager).

Can vouch that the added integration test seems to work when run manually now BUT it does sometimes exhibit the EOF error on my machine (which I am still positive is unrelated and hope Claudiu will push a fix for soon).

[bschimke95]

thanks @aznashwan - a couple of comments.

[claudiubelu]

The mentioned PR has been merged, so this should be fine now to go in as well, after the conflicts have been resolved.

It seems I have been doing something similar as well. Though, just cleaning up the containerd-related directories is not going to be enough, we should probably also shutdown the Kubernetes services, otherwise, on the next bootstrap / join-cluster, we're going to see that the ports are in use.

[aznashwan]

@bschimke95 @berkayoz related to this latest defensive commit I had to add to the cleanup

@claudiubelu literally accidentally deleted his root dir using this cleanup code after I re-based my patch over his recently-merged configurable containerd paths one (thank you Claudiu, this was the definition of taking one for the team!)

I think it's really bad that we have our cleanup logic split between the snap's remove hook (on snap remove) and in k8sd (on failed bootstrap/join-cluster), and we'll always be exposing ourselves to us/users messing up the lockfiles.

I would strongly suggest we move ALL cleanup logic to a dedicated routine which we re-use internally and expose via a dedicated k8s cleanup command which should be best-effort; and directly called in the snap's remove hook.

[berkayoz]

LGTM, agreed on turning this into a cleanup command we can call on both the snap hook and in bootstrap/join cleanup. We can pick that up as a separate task and backport it to 1.32.

[bschimke95]

LGTM, minor nit and Berkays comment

[bschimke95]

please also add some description to this PR. IMHO the title is not sufficient anymore.

[aznashwan]

please also add some description to this PR. IMHO the title is not sufficient anymore.

Made the commit messages very explicit for posterity; and pasted their descriptions in the PR.