[2/2] Warn if systemd-firstboot misses configurations

Message ID 20221119182031.2005807-3-tobias.schaffner@siemens.com
State Accepted, archived
Headers show
Series Mask systemd-firstboot | expand

Commit Message

Tobias Schaffner Nov. 19, 2022, 6:20 p.m. UTC
From: Tobias Schaffner <tobias.schaffner@siemens.com>

systemd-firstboot checks the existence of different system configurations
like locale or hostname. Debian packages may trust that these configurations
are in the location that systemd-firstboot enforced.

Warn the user in the image postproc step if systemd-firstboot misses any
configurations in the image.

Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
---
 meta/classes/image-postproc-extension.bbclass | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Anton Mikanovich Nov. 22, 2022, 10:04 a.m. UTC | #1
19.11.2022 21:20, T. Schaffner wrote
> From: Tobias Schaffner <tobias.schaffner@siemens.com>
>
> systemd-firstboot checks the existence of different system configurations
> like locale or hostname. Debian packages may trust that these configurations
> are in the location that systemd-firstboot enforced.
>
> Warn the user in the image postproc step if systemd-firstboot misses any
> configurations in the image.
>
> Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>

If we have a warning here, do we really need to mask the service in p1?
User should fix the reason but not hide it, and masking can be optional just
in case there are no other ways.
Florian Bezdeka Nov. 22, 2022, 10:24 a.m. UTC | #2
On Tue, 2022-11-22 at 13:04 +0300, Anton Mikanovich wrote:
> 19.11.2022 21:20, T. Schaffner wrote
> > From: Tobias Schaffner <tobias.schaffner@siemens.com>
> > 
> > systemd-firstboot checks the existence of different system configurations
> > like locale or hostname. Debian packages may trust that these configurations
> > are in the location that systemd-firstboot enforced.
> > 
> > Warn the user in the image postproc step if systemd-firstboot misses any
> > configurations in the image.
> > 
> > Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
> 
> If we have a warning here, do we really need to mask the service in p1?
> User should fix the reason but not hide it, and masking can be optional just
> in case there are no other ways.

I didn't have the time to review the patches in detail yet, so I might
be wrong. But: If systemd-firstboot is enabled (not masked) and we miss
one of the config settings that it checks the image won't boot up.

It will enter a interactive mode and ask you questions on how to
configure your system (locale, keyboard layout, ...). Especially for CI
and embedded systems that's a real issue.

So I guess the idea was to simply mask systemd-firstboot (we don't like
interactive mode) and warn the user that the system might not be fully
configured.

Tobias, please correct me if need to.

Florian

>
Uladzimir Bely Feb. 23, 2023, 11:43 a.m. UTC | #3
In the email from Tuesday, 22 November 2022 13:24:53 +03 user Florian Bezdeka wrote:
> On Tue, 2022-11-22 at 13:04 +0300, Anton Mikanovich wrote:
> > 19.11.2022 21:20, T. Schaffner wrote
> > > From: Tobias Schaffner <tobias.schaffner@siemens.com>
> > > 
> > > systemd-firstboot checks the existence of different system configurations
> > > like locale or hostname. Debian packages may trust that these configurations
> > > are in the location that systemd-firstboot enforced.
> > > 
> > > Warn the user in the image postproc step if systemd-firstboot misses any
> > > configurations in the image.
> > > 
> > > Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
> > 
> > If we have a warning here, do we really need to mask the service in p1?
> > User should fix the reason but not hide it, and masking can be optional just
> > in case there are no other ways.
> 
> I didn't have the time to review the patches in detail yet, so I might
> be wrong. But: If systemd-firstboot is enabled (not masked) and we miss
> one of the config settings that it checks the image won't boot up.
> 
> It will enter a interactive mode and ask you questions on how to
> configure your system (locale, keyboard layout, ...). Especially for CI
> and embedded systems that's a real issue.
> 
> So I guess the idea was to simply mask systemd-firstboot (we don't like
> interactive mode) and warn the user that the system might not be fully
> configured.
> 
> Tobias, please correct me if need to.
> 
> Florian
> 
> > 
> 
> 

We are still waiting for the answer, so the merge process is delayed.

Anyway, CI on recent `next` is still passed OK with the patch, so if no one against, we would merge it soon.
Tobias Schaffner Feb. 24, 2023, 11:36 a.m. UTC | #4
On 23.02.23 12:43, Uladzimir Bely wrote:
> In the email from Tuesday, 22 November 2022 13:24:53 +03 user Florian Bezdeka wrote:
>> On Tue, 2022-11-22 at 13:04 +0300, Anton Mikanovich wrote:
>>> 19.11.2022 21:20, T. Schaffner wrote
>>>> From: Tobias Schaffner <tobias.schaffner@siemens.com>
>>>>
>>>> systemd-firstboot checks the existence of different system configurations
>>>> like locale or hostname. Debian packages may trust that these configurations
>>>> are in the location that systemd-firstboot enforced.
>>>>
>>>> Warn the user in the image postproc step if systemd-firstboot misses any
>>>> configurations in the image.
>>>>
>>>> Signed-off-by: Tobias Schaffner <tobias.schaffner@siemens.com>
>>>
>>> If we have a warning here, do we really need to mask the service in p1?
>>> User should fix the reason but not hide it, and masking can be optional just
>>> in case there are no other ways.
>>
>> I didn't have the time to review the patches in detail yet, so I might
>> be wrong. But: If systemd-firstboot is enabled (not masked) and we miss
>> one of the config settings that it checks the image won't boot up.
>>
>> It will enter a interactive mode and ask you questions on how to
>> configure your system (locale, keyboard layout, ...). Especially for CI
>> and embedded systems that's a real issue.
>>
>> So I guess the idea was to simply mask systemd-firstboot (we don't like
>> interactive mode) and warn the user that the system might not be fully
>> configured.
>>
>> Tobias, please correct me if need to.
>>
>> Florian
>>
>>>
>>
>>
> 
> We are still waiting for the answer, so the merge process is delayed.
> 
> Anyway, CI on recent `next` is still passed OK with the patch, so if no one against, we would merge it soon.

Hi Uladzimir,

sorry for the late reply. It behaves as Florian described it. We want to 
avoid a non booting image in any case but utilize systemd-firstboot to 
warn the user about possibly missing configurations.

Tobias

Patch

diff --git a/meta/classes/image-postproc-extension.bbclass b/meta/classes/image-postproc-extension.bbclass
index 942b929..f7f762c 100644
--- a/meta/classes/image-postproc-extension.bbclass
+++ b/meta/classes/image-postproc-extension.bbclass
@@ -88,5 +88,10 @@  image_posprocess_disable_systemd_firstboot() {
 
     if dpkg --compare-versions "$SYSTEMD_VERSION" "ge" "251"; then
         sudo chroot '${ROOTFSDIR}' systemctl mask systemd-firstboot
+        if ! cmd_output=$(sudo chroot '${ROOTFSDIR}' systemd-firstboot \
+               --prompt --welcome=false </dev/null 2>/dev/null); then
+            bbwarn "Your image is not configured completely according to systemd-firstboot."
+            bbwarn "It prompted: \"${cmd_output}\""
+        fi
     fi
 }