balena-os / balena-supervisor

Balena Supervisor: balena's agent on devices.
https://balena.io
Other
147 stars 63 forks source link

User would like to add conditional configurations #1573

Open jellyfish-bot opened 3 years ago

jellyfish-bot commented 3 years ago

[rahul-thakoor] The supervisor currently does not support conditional configs which means it gets deleted when the config.txt file is synchronised.

jellyfish-bot commented 3 years ago

[rahul-thakoor] This issue has attached support thread https://jel.ly.fish/e1f32a51-9ca4-4bfa-8346-fdf8a8a7961e

klutchell commented 3 years ago

https://www.raspberrypi.org/documentation/configuration/config-txt/conditional.md

klutchell commented 3 years ago

User now reports the latest supervisor crashes when encountering conditional config:

The latest version of the supervisor actually crashes the entire supervisor when it meets a conditional config section in the existing config.txt (can't handle the "[" leading a line) which is a bit unfortunate.

nbertram commented 3 years ago

Yes, to add some more detail over here, the error from supervisor's journal is:

Jan 26 22:25:33 4ba1d46 resin-supervisor[2315]: [info]    Internet Connectivity: OK
Jan 26 22:25:56 4ba1d46 resin-supervisor[2315]: [warn]    Could not parse config.txt entry: [all]. Ignoring.
Jan 26 22:25:56 4ba1d46 resin-supervisor[2315]: [error]   Error reporting initial configuration, will retry StatusError: Configuration variable names can only contain alphanumeric characters and underscores.
Jan 26 22:25:56 4ba1d46 resin-supervisor[2315]: [error]         at PinejsClientRequest._request (/usr/src/app/dist/app.js:10:1452318)
Jan 26 22:25:56 4ba1d46 resin-supervisor[2315]: [error]       at processTicksAndRejections (internal/process/task_queues.js:97:5)
Jan 26 22:25:56 4ba1d46 resin-supervisor[2315]: [error]       at async /usr/src/app/dist/app.js:10:4633
Jan 26 22:25:56 4ba1d46 resin-supervisor[2315]: [error]       at async reportInitialConfig (/usr/src/app/dist/app.js:10:3600)
Jan 26 22:25:56 4ba1d46 resin-supervisor[2315]: [error]       at async reportInitialConfig (/usr/src/app/dist/app.js:10:5436)
Jan 26 22:25:56 4ba1d46 resin-supervisor[2315]: [error]       at async Object.exports.start (/usr/src/app/dist/app.js:10:3510)
Jan 26 22:25:56 4ba1d46 resin-supervisor[2315]: [error]       at async Supervisor.init (/usr/src/app/dist/app.js:10:955579)

but this doesn't get forwarded to the Balena cloud, presumably because the exception prevents it. The cloud logs just see this:

27.01.21 11:25:23 (+1300) Supervisor starting

(the end)

This was the bundled config.txt on the boot partition that triggered this:

# While disabling the UART seems like a good idea, it prevents Balena from booting...
# (may be because of the kernel trying to attach a serial console to a missing device)
enable_uart=1

# We, however, don't need i2c, SPI, bluetooth or audio
dtparam=i2c_arm=off
dtparam=spi=off
dtparam=audio=off

# No rainbow on boot
disable_splash=1

# Don't show undervoltage or overtemp warnings (we'll take our chances)
avoid_warnings=1

# Increase GPU memory a bit so we can use hardware accelerated rendering
gpu_mem=128

# Enable composite output only if there's nothing connected to HDMI. Composite
# comes at a performance penalty on the pi4, but more to the point, with composite
# enabled, the pi4 won't look for anything connected to HDMI, so not auto-switching.
# See https://www.raspberrypi.org/forums/viewtopic.php?f=63&t=296243
enable_tvout=1
sdtv_mode=2

[EDID=*]
enable_tvout=0

[all]
# balena can write overrides here

note this was done before I understood how the supervisor rewrites the whole config.txt, so you can disregard my comments :grin:

The issue above would appear to be that [EDID=*] matches https://github.com/balena-io/balena-supervisor/blob/e5a1561bff19741534abd1f2d1b7ec59b1d29f44/src/config/backends/config-txt.ts#L78 and is therefore parsed as a key/value pair that includes the leading '['

nbertram commented 3 years ago

As an interim solution, would we be allowed to put colons in config var names, such that we can set things like hdmi_force_hotplug:1=1 as per https://www.raspberrypi.org/documentation/computers/configuration.html#setting-a-specific-hdmi-mode

As it stands, we can't target the second HDMI port at all with balena.

20k-ultra commented 3 years ago

hey @nbertram I opened https://github.com/balena-io/open-balena-api/issues/759 to push the conversation regarding configurations that have a colon character since the API rejects these and I think if it allowed them then the Supervisor would be able to set them correctly. If you want to see that issue progressed you should comment there to ask for updates.

Regarding the conditional configuration, some logs are kept on device if we think they will provide too much confusion and if a user really wanted to see what's going on then checking journal logs from the device is possible. However, in this situation I agree the logs in the dashboard are not helpful at all since it doesn't show any sign of an issue. I think the solution to this is to correctly handle errors and propagate them to the dashboard. I made https://github.com/balena-os/balena-supervisor/issues/1788 to track that since this issue is about adding support for conditional configurations.

I think we might be able to make the Supervisor ignore anything that isn't a key=value. This means you could modify the config.txt at any time to have conditionals for configs you set from the cloud. It's not a great idea to add such a workaround so doubt it'll happen though.

How critical are these conditionals for you ? do you have any workarounds ?

20k-ultra commented 3 years ago

I mentioned not being able to use index identifiers in config names internally to see what happen people think. Maybe we can unblock you on that one soon.

nbertram commented 2 years ago

Our immediate requirement for the conditional config of enable_tvout is gone now, we have tooling to set this explicitly on pis that are known to need composite output.

Being able to target the second HDMI port with config is still important and causes us some pain. At the moment we have documented issues with not being able to use exotic monitors or modulators on the second port, and so additional pis are used in locations that have these. It would be great to fix that, but we don't have an immediate requirement for it.

pipex commented 1 year ago

A PR allowing the use of the colon character in config vars has already been merged in 14.6.0. We are on the final steps of enabling this on the API side in the following PR https://github.com/balena-io/open-balena-api/issues/759