Fix LXC container implementations

[Nomsplease]

I moved branches around and it caused quite the Chaos on this PR. This a re-open of #219 with the same changes, just moved to a different branch

Proposed Changes

• correct some implementation ideas for lxc supprt
• We should not assume the rc.local is empty and just overwrite it, we can do this better and insert a managed code block
• We also should not assume we are root on reboot handler, and become has been added to this handler
• Also added resetting the lxc modification to a untouched state inside the container, resetting the lxc config in the host may also be a worthwhile adventure to make sure we get as close to original as possible.

Checklist

• Tested locally
• Ran site.yml playbook
• Ran reset.yml playbook
• Did not add any unnecessary changes
• Ran pre-commit install at least once before committing
• 🚀

[acdoussan]

Should this always run? I suppose there is a chance that the user configured these values outside of this playbook, and this would remove them

[Nomsplease]

I feel like it should, only because I consider this playbook as a it should configure and manage everything. If we don't we could leave security issues where they don't need to exist without user knowledge.

[acdoussan]

Think I agree, think the logic here should be that we assume these hosts are dedicated to k3s, so we should make sure this stuff is torn down through the reset process. Don't feel too strongly here though. Would just mean removing this when condition.

[Nomsplease]

That or the user just does not specify that the LXC is configured and hits those functions manually. I personally do this as the LXCs are dedicated solely to K3s, as I imagine most people do.

[acdoussan]

One super small comment, but I'm good with these changes as is 👍

[timothystewart6]

Looks like we have some lint. We aren't using the literal true for become, we've been using yes

[Nomsplease]

Looks like we have some lint. We aren't using the literal true for become, we've been using yes

Im seeing in a lot of other files they are used interchangeably, but the standard for Ansible is True/False. Would you like me to swap these from True/False to Yes/No?

[timothystewart6]

I'd just leave the ones in place so that this PR only contains your changes. We have an issue to change them all at once, which will probably require new lint rules. too. #204

If you install all of the dependencies it will fix and fail on commits, rather than later in CI.

[Nomsplease]

Ok that should be the last lint error, hopefully.

[timothystewart6]

Looks like the cleanup is failing

 Remove rc.local modificaitons for proxmox lxc containers

[timothystewart6]

@Nomsplease one small fix and then it's ready to go!

[Nomsplease]

@timothystewart6 ill get this corrected and update when done. Did you run locally and get an error? Curious how that step failed for you.

[timothystewart6]

@Nomsplease you can see it failing in CI at this line
https://github.com/techno-tim/k3s-ansible/actions/runs/4178688825/jobs/7313361925#step:9:966

[timothystewart6]

(expand and look at line 956)

[Nomsplease]

Updated branch and changed how we handle the rc.local file. This is a good catch because we should not be touching this file on a host that was not setup with lxc in the first place.

[timothystewart6]

We're so close to the finish line. What's left?

[Nomsplease]

We're so close to the finish line. What's left?

All we had were the other review changes, but I think we are good to go on the functionality that was intended in this change.

[Nomsplease]

I pulled the current upstream into this branch. One last round of checks should not hurt here and we can make the merge as smooth as possible.

[timothystewart6]

Thank you all for working through this. If we need to change a pattern, we can add a new issue for it! Thanks again everyone!