Fix LXC container implementations

[Nomsplease]

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
• 🚀

[timothystewart6]

FYI @acdoussan Look ok?

[acdoussan]

Wow, someone else already using this, pretty cool!

Not assuming the user is root makes total sense, this LGTM.

My assumption was that the hosts would be dedicated for kubernetes since containers have a very low overhead, so I wasn't too worried about stomping over an already existing rc.local. With that being said I don't see any reason to not be nice and not overwrite stuff that is already there.

Have a couple of comments on the impl, otherwise think this looks good to me. Also nice to add this to the clean-up script.

Unrelated note: maybe I should update the clean-up to remove the container conf update lines as well?

[timothystewart6]

Unrelated note: maybe I should update the clean-up to remove the container conf update lines as well?

That would be awesome and thank you for the help!

[Nomsplease]

@acdoussan I removed the need for set fact in the deploy task and validated it works exactly as it should.

I also added in resetting the LXC container configs as well, made testing the changes faster so figured may as well knock it out in the same reboot on the cluster teardown.

[acdoussan]

sorry, couple more thoughts, but appreciate you adding in cleaning up the proxmox conf files!

Had one other thought, the rc.local script has a shebang at the beginning, but this will get wrapped in the ansible block comments. I'm not sure if that matters, but we should probably make sure the block gets added to the top of the file, rather than the end? Will look into if this really matters or not.

[acdoussan]

shebang does have to be the first line, not clear if it matters for rc.local or not. Maybe we remove the line from the templated rc.local, and instead have it as a line in file, with firstmatch as true, insertbefore as a match anything regex, and create as true? Would then not need the create on the block in file, that could just stay as is and remove the file perms.

[Nomsplease]

sorry, couple more thoughts, but appreciate you adding in cleaning up the proxmox conf files!

Had one other thought, the rc.local script has a shebang at the beginning, but this will get wrapped in the ansible block comments. I'm not sure if that matters, but we should probably make sure the block gets added to the top of the file, rather than the end? Will look into if this really matters or not.

This is a good catch, yes that technically matters as the bang should be the first line. My first thought would be to break the block up. If rc.local exists this should only add the managed block, but if we are creating a file we can add the shebang.

This should be solvable with lineinfile to look for a shebang statement.

[acdoussan]

cool cool, let me know if you need any help here making those changes, can always separate things out into separate PRs

[Nomsplease]

Just noting here that I still have this on my to do list, just haven't been able to check this back out and validate. This is in my list for this week.

[timothystewart6]

Just noting here that I still have this on my to do list, just haven't been able to check this back out and validate. This is in my list for this week.

Sounds good, thank you!

[Nomsplease]

I think we should be good here. I rewrote a lot of how we handle the rc.local file to not blow away an existing file. I also included and validated the lxc configuration and removal extra settings. All of these changes were tested locally and I can confirm they work. I did however find an issue with the implementation, and thats zfs and the overlayfs. I will open an issue on this an look into that.

Also added a clean up task that if the rc.local file only has 1 line or less after we removed our modifications, Im assuming its just a shebang line and deleting the file. There could be an edge case here where someone did not setup a rc.local file correctly and has a one liner or didn't have a shebang when they made a custom file. I don't have an answer on a good way to handle this and I would rather clean up our changes.

Examples of the rc.local modifications.
Existing rc.local file ends up like this:

root@K3S-01:~# cat /etc/rc.local
#!/bin/bash

echo "Starting up"
echo "We are doing the stuff here"
# BEGIN ANSIBLE MANAGED BLOCK
# Kubeadm 1.15 needs /dev/kmsg to be there, but it's not in lxc, but we can just use /dev/console instead
# see: https://github.com/kubernetes-sigs/kind/issues/662
if [ ! -e /dev/kmsg ]; then
    ln -s /dev/console /dev/kmsg
fi

# https://medium.com/@kvaps/run-kubernetes-in-lxc-container-f04aa94b6c9c
mount --make-rshared /
# END ANSIBLE MANAGED BLOCK

An new rc.local file will end up like this:

root@K3S-02:~# cat /etc/rc.local
#!/bin/sh -e
# BEGIN ANSIBLE MANAGED BLOCK
# Kubeadm 1.15 needs /dev/kmsg to be there, but it's not in lxc, but we can just use /dev/console instead
# see: https://github.com/kubernetes-sigs/kind/issues/662
if [ ! -e /dev/kmsg ]; then
    ln -s /dev/console /dev/kmsg
fi

# https://medium.com/@kvaps/run-kubernetes-in-lxc-container-f04aa94b6c9c
mount --make-rshared /
# END ANSIBLE MANAGED BLOCK

Edit: I goofed on my code blocks