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 check for /boot/cmdline.txt #206

Merged
merged 2 commits into from May 16, 2022
Merged

Conversation

ikifar2012
Copy link
Member

@ikifar2012 ikifar2012 commented Apr 3, 2022

Hopefully this will fix the error some are facing in regards to setting cgroupv1 on Raspberry Pi
I would greatly apricate it if someone could help me test this as I currently do not have access to a Raspberry Pi running Debian

Debian package can be found here:
https://github.com/home-assistant/supervised-installer/actions/runs/2086587575

@fcastilloec
Copy link

I'm installed the .deb from the link on my Raspberry Pi 4, and everything went smoothly. No more errors and cgroupv1 is enabled in /boot/cmdline.txt.

@ikifar2012
Copy link
Member Author

Thank you @fcastilloec, I am going to mark this as "Ready for review"

@ikifar2012 ikifar2012 requested a review from pvizeli April 4, 2022 00:57
@ikifar2012 ikifar2012 marked this pull request as ready for review April 4, 2022 00:57
@ikifar2012 ikifar2012 linked an issue Apr 4, 2022 that may be closed by this pull request
3 tasks
@pvizeli
Copy link
Member

pvizeli commented Apr 4, 2022

I'm against that because this installation type is documented by https://github.com/home-assistant/architecture/blob/master/adr/0014-home-assistant-supervised.md - Raspbian is still not supported and we also do not take care of its work or not ether we can test it. It's enough hard to support 1 OS

@ikifar2012
Copy link
Member Author

I'm against that because this installation type is documented by https://github.com/home-assistant/architecture/blob/master/adr/0014-home-assistant-supervised.md - Raspbian is still not supported and we also do not take care of its work or not ether we can test it. It's enough hard to support 1 OS

I cannot test this for sure but according to what I have gathered from talking to others in the discord, it appears even vanilla Debian on Raspberry Pi does not support grub

@fcastilloec
Copy link

fcastilloec commented Apr 4, 2022

If you download the latest official Debian Bullseye image for Raspberry Pi (NOT Raspbian/Raspberry Pi OS) and mount it in your system, you'll see that the file /etc/default/grub doesn't exist, instead, there's another partition containing the file cmdline.txt which will be placed under /boot/firmware/

You can also read the official Debian wiki about overwriting /boot/firmware/cmdline.txt when update-initramfs is called, and they suggest using /etc/default/raspi-extra-cmdline instead; but I wasn't aware of this until now, so I've never used it.

@ikifar2012
Copy link
Member Author

@fcastilloec would you be willing to help test changes? You can find me in the Home Assistant Discord.

update-grub
touch /var/run/reboot-required
fi
elif [ -f /boot/cmdline.txt ]

Choose a reason for hiding this comment

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

I guess this works for Raspbian/Raspberry Pi OS but not for vanilla Debian

Suggested change
elif [ -f /boot/cmdline.txt ]
elif [ -f /boot/firmware/cmdline.txt ]

Choose a reason for hiding this comment

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

Just an observation but I don't understand all the extra complexity. Going back one step, the code introduced by PR201 is straightforward:

# Switch to cgroup v1
if ! grep -q "systemd.unified_cgroup_hierarchy=false" /etc/default/grub; then
    info "Switching to cgroup v1"
    cp /etc/default/grub /etc/default/grub.bak
    sed -i 's/^GRUB_CMDLINE_LINUX_DEFAULT="/&systemd.unified_cgroup_hierarchy=false /' /etc/default/grub
    update-grub
    touch /var/run/reboot-required
fi

The problem is the implied assumption that /etc/default/grub always exists, with the consequence that grep fails when it doesn't.

Why not just augment the if test to protect the grep? Something like:

if [ -f /etc/default/grub ] && ! grep -q "systemd.unified_cgroup_hierarchy=false" /etc/default/grub ; then

I realise that has the same effect as the first part of your proposal. I suppose the bit I don't get is the stuff with cmdline.txt. It seems to me that that's not needed if either of the following is true:

  • /etc/default/grub doesn't exist; or
  • /etc/default/grub exists and contains systemd.unified_cgroup_hierarchy=false

Am I missing the point entirely?

I keep looking at that "not" in ! grep and wondering if it is somehow misleading my basic analysis?

Here is what I think is going on. First, a test:

alias TEST='if [ -f /etc/default/grub ] && ! grep -q "systemd.unified_cgroup_hierarchy=false" /etc/default/grub ; then echo "true" ; else echo "false" ; fi'

Starting point = /etc/default/grub does not exist

$ ls /etc/default/grub
ls: cannot access '/etc/default/grub': No such file or directory
$ TEST
false

Create /etc/default/grub as an empty file (ie the file will exist but doesn't contain the magic incantation):

$ sudo touch /etc/default/grub
$ TEST
true

Now add the magic incantation:

$ echo "systemd.unified_cgroup_hierarchy=false" | sudo tee /etc/default/grub
$ TEST
false

Don't those three tests reflect the desired behaviour? The conditional code should only execute on a system that actually knows about grub, and where the expected file does not contain the magic incantation?

Then, if the system does not know about grub, why be adding systemd.unified_cgroup_hierarchy=false to cmdline? I'm really confused. Apologies if I wind up confusing everyone else too.

Copy link

Choose a reason for hiding this comment

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

What I understood is that we're trying to solve issue home-assistant/supervisor#3444 where cgroup2 is the default on Debian and Home Assistant requires cgroup1. This is not related to grub, but grub is used to set the correct version of cgroup. Raspberry Pi doesn't support grub even on Debian Bullseye, so even if the test you mention is correct, the issue wouldn't be fixed.

So, we still need to insert the appropriate line of code into a file that controls the version of cgroup on non-grub systems, which in the case of Raspberry Pi is /boot/firmware/cmdline.txt for vanilla Debian Bullseye and /boot/cmdline.txt for Raspberry Pi OS (which is not supported and hence the edit).

I don't expect all non-grub devices to be supported, but the Raspberry Pi is extremely popular and regularly used to run Home Assistant, so I would expect this device to be supported (still running vanilla Debian Bullseye as required).

Choose a reason for hiding this comment

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

Thank you for the clarification. I came into this via a different path - a person reporting cp: cannot stat '/etc/default/grub': No such file or directory so I was just looking at the symptoms.

I don't know if it will help or hinder your investigations but I just did a fresh build of current Raspberry Pi OS 64-bit (which describes itself as Debian). My cmdline.txt has:

console=serial0,115200 console=tty1 root=PARTUUID=7ef150bd-02 rootfstype=ext4 fsck.repair=yes rootwait quiet splash plymouth.ignore-serial-consoles cgroup_memory=1 cgroup_enable=memory apparmor=1 security=apparmor

As I understand it, cgroup_memory=1 cgroup_enable=memory enables cgroups, save that I think of it as being needed for docker stats rather than HA.

When I run:

$ mount | grep cgroup
cgroup2 on /sys/fs/cgroup type cgroup2 (rw,nosuid,nodev,noexec,relatime,nsdelegate,memory_recursiveprot)

So, all other things being equal, this should be incompatible with HA and trigger the #3444 problem - yes?

All I did before installing HA was:

$ echo "systemd.unified_cgroup_hierarchy=false" | sudo tee /etc/default/grub

After installing HA:

$ docker ps
CONTAINER ID   IMAGE                                                           COMMAND                  CREATED       STATUS       PORTS                  NAMES
9741bfdbae13   ghcr.io/home-assistant/aarch64-hassio-multicast:2022.02.0       "/init"                  2 hours ago   Up 2 hours                          hassio_multicast
3e2125245ea1   ghcr.io/home-assistant/aarch64-hassio-audio:2021.07.0           "/init"                  2 hours ago   Up 2 hours                          hassio_audio
286b68296dd7   ghcr.io/home-assistant/aarch64-hassio-dns:2021.06.0             "/init"                  2 hours ago   Up 2 hours                          hassio_dns
9d2926cc7afe   ghcr.io/home-assistant/aarch64-hassio-cli:2021.12.0             "/init /bin/bash -c …"   2 hours ago   Up 2 hours                          hassio_cli
73a9d69180bb   ghcr.io/home-assistant/raspberrypi4-64-homeassistant:2022.3.8   "/init"                  2 hours ago   Up 2 hours                          homeassistant
f2f4f880120d   ghcr.io/home-assistant/aarch64-hassio-observer:2021.10.0        "/init"                  2 hours ago   Up 2 hours   0.0.0.0:4357->80/tcp   hassio_observer
e2cd80c66fa2   ghcr.io/home-assistant/aarch64-hassio-supervisor                "/init"                  2 hours ago   Up 2 hours                          hassio_supervisor

Collecting all the container logs (attached) and looking for mentions of "cgroup" yields silence.

Searching /var/log/syslog gets a few mentions but nothing saying "this ain't working":

Jan 28 03:21:55 raspberrypi kernel: [    0.002087] cgroup: Disabling memory control group subsystem
Apr  5 11:38:38 sec-dev kernel: [    0.001986] cgroup: Disabling memory control group subsystem
Apr  5 11:39:17 sec-dev kernel: [    0.002007] cgroup: Disabling memory control group subsystem
Apr  5 11:57:20 sec-dev kernel: [    0.001985] cgroup: Disabling memory control group subsystem
Apr  5 12:05:43 sec-dev kernel: [    0.001986] cgroup: Disabling memory control group subsystem
Apr  5 12:09:02 sec-dev kernel: [    0.000000] Kernel command line: coherent_pool=1M 8250.nr_uarts=0 snd_bcm2835.enable_compat_alsa=0 snd_bcm2835.enable_hdmi=1  smsc95xx.macaddr=DC:A6:32:4A:89:F9 vc_mem.mem_base=0x3eb00000 vc_mem.mem_size=0x3ff00000  console=ttyS0,115200 console=tty1 root=PARTUUID=7ef150bd-02 rootfstype=ext4 fsck.repair=yes rootwait quiet splash plymouth.ignore-serial-consoles cgroup_memory=1 cgroup_enable=memory apparmor=1 security=apparmor
Apr  5 12:09:02 sec-dev kernel: [    0.000000] cgroup: Enabling memory control group subsystem
Apr  5 12:09:02 sec-dev kernel: [    0.000000] Unknown kernel command line parameters "splash cgroup_memory=1", will be passed to user space.
Apr  5 12:09:02 sec-dev kernel: [    1.992849]     cgroup_memory=1

Other stuff:

$ uname -a
Linux sec-dev 5.15.32-v8+ #1538 SMP PREEMPT Thu Mar 31 19:40:39 BST 2022 aarch64 GNU/Linux

$ ha info
arch: aarch64
channel: stable
docker: 20.10.14
features:
- reboot
- shutdown
- services
- network
- hostname
- timedate
- os_agent
hassos: null
homeassistant: 2022.3.8
hostname: sec-dev
logging: info
machine: raspberrypi4-64
operating_system: Debian GNU/Linux 11 (bullseye)
state: running
supervisor: 2022.03.5
supported: true
supported_arch:
- aarch64
- armv7
- armhf
timezone: Australia/Sydney

Bottom line: it looks to me like HA is working without needing to add anything like systemd.unified_cgroup_hierarchy=false to /boot/cmdline.txt.

Anywhere else I should be looking for signs of this problem?

collected-home-assistant-logs.txt

Copy link

Choose a reason for hiding this comment

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

The problem is originally reported as happening when using RaspberryMatic add-on in Home Assistant. I do not use this add-on, but if you want to check for errors, I suggest you install it. Here's a guide, which actually has a warning regarding the original problem with cgroupversions, and even mentions what to do to fix it.

I don't know if it will help or hinder your investigations but I just did a fresh build of current Raspberry Pi OS 64-bit (which describes itself as Debian).

Thanks for the info, but unless Home Assistant decides to support other OSs (even if they are based on Debian), only Debian Bullseye is supported; they are very explicit about this in their documentation. If you want to re-do your tests (including RaspberryMatic), I suggest you download Debian v11 (Bullseye) for your hardware from here.

Choose a reason for hiding this comment

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

Can you switch all the references from /boot/cmdline.txt to /boot/firmware/cmdline.txt as suggested? I have vanilla debian on an rpi4 right now and am hitting this. Can't test your branch because this file also doesn't exist.

@pvizeli after this change I would suggest we merge this. I don't believe this is an unsupported OS issue. I am hitting this right now on a freshly flashed vanilla image from here on an RPI4 which should be supported (it's in our list of supported machine types and running vanilla debian 11)

@fcastilloec
Copy link

@fcastilloec would you be willing to help test changes? You can find me in the Home Assistant Discord.

Hopefully, in the next few days, I'll have some time to test this PR but using /etc/default/raspi-extra-cmdline, to avoid losing any added commands when raspi-firmware is run.

@ikifar2012
Copy link
Member Author

The thing I am worried about, is what is the best way to ensure compatibility with all devices /etc/default/raspi-extra-cmdline only works on Raspberry Pi

@jhampson-dbre
Copy link

Is this something the installer needs to handle, or does it make sense to document the requirement for cgroups v1 and leave it as an exercise for the user to implement?

@fcastilloec
Copy link

Given that bootloaders are specific to each distro and/or device, it's better to leave it to each user to deal with this. Especially when the problem only happens when using a specific add-on. RaspberryMatic already points out this issue in their installing guide and points to instructions on how to fix it.
The only way to properly fix this would be for HomeAssistant to use cgroups v2.
I would suggest reverting PR #201 code.

@ikifar2012
Copy link
Member Author

I am thinking the best option going forward might just be to check if grub is there, if it is, modify it, else warn the user they will have to switch to cgroupv1 manually

@fcastilloec
Copy link

I am thinking the best option going forward might just be to check if grub is there, if it is, modify it, else warn the user they will have to switch to cgroupv1 manually

And you'll have to modify the Readme.md and remove every supported architecture except for x86_64 and maybe qemu, but not sure about qemu; since you're only applying a fix to a single architecture. At the very least, mention that Raspberry Pis and other boards aren't 100% supported and they need special treatment from the user that this installer doesn't support.

@jutonium
Copy link

@fcastilloec is right.
It would be great If you could add a list to readme.md for known devices and parameter combinations. During my research for switching to cgroup v1 support I stumbled upon some kubernetes-how-tos which promised the required parameters for switching to cgroup v1. Unfortunately they have not been helpful for resolving the Home Assistant on RPi issues.
The issues occur with the Raspberrymatic add-on because it requires access to a hardware driver which is part of Home Assistant OS or has to be installed manually in a supervised installation.

@LaurenceGough
Copy link

Hopefully this will fix the error some are facing in regards to setting cgroupv1 on Raspberry Pi I would greatly apricate it if someone could help me test this as I currently do not have access to a Raspberry Pi running Debian

Debian package can be found here: https://github.com/home-assistant/supervised-installer/actions/runs/2086587575

Thanks for this, this fixed my issue when running Raspberry OS. That was a bit scary after running updates on Raspberry OS home Assistant really didn't like that.

@Steve973
Copy link

It would be very nice, if this work won't be done, to include an "unofficial" troubleshooting section for people that want to do a supervised install on their own. This is what needs to be done:

  1. Add systemd.unified_cgroup_hierarchy=false to /etc/default/grub.
  2. Add systemd.unified_cgroup_hierarchy=false to the end of /boot/cmdline.txt.
  3. Reboot.
  4. Install ha supervised.

I understand the difficulty of supporting multiple environments, and especially environments that you have no control over. But why not include the information that helps people, along with a disclaimer that this is not recommended, and that the user is responsible for their own install if they need to perform customizations like these? It is a lot friendlier than requiring a special/single-purpose OS.

@ikifar2012
Copy link
Member Author

I am going to merge the change as is, since my changes have been approved, if any further issues are discovered please file them under issues and hopefully we can get everything sorted out

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug Report: /etc/default/grub: No such file or directory
10 participants