Repository: osbuild/osbuild
Pull request #1448 (2023-11-14 11:42:17)

RPM stage: link /proc/self/fd to /dev/fd
Link **/proc/self/fd** to **/dev/fd** within the tree to avoid **"'/dev/fd/63': No such file or directory"** errors

Signed-off-by: Miguel Martín <mmartinv@redhat.com>


supakeen: Thanks for splitting this out, I'll take a look.
mmartinv: > Since this is being done in other stages as well (`org.osbuild.mkinitcpio`, some others) I'm fine with this.
> 
> However, since various stages now do their own implementations of this it might be a good idea to eventually move this into the runner(s) (in a followup).

Moving this to the runner to make it available for all the stages implies mounting `/proc` and probably `/dev/` within the tree. If we do that I believe we could also mount `/sys` to avoid some other annoying messages. However we must bear in mind that currently `/proc` `/dev` and `/sys` are mounted by some stages only, if we move this to the runner it will be mounted and unmounted for each stage the runner executes.

supakeen: > > Since this is being done in other stages as well (`org.osbuild.mkinitcpio`, some others) I'm fine with this.
> > However, since various stages now do their own implementations of this it might be a good idea to eventually move this into the runner(s) (in a followup).
> 
> Moving this to the runner to make it available for all the stages implies mounting `/proc` and probably `/dev/` within the tree. If we do that I believe we could also mount `/sys` to avoid some other annoying messages. However we must bear in mind that currently `/proc` `/dev` and `/sys` are mounted by some stages only, if we move this to the runner it will be mounted and unmounted for each stage the runner executes.

Yea, that's exactly what needs to be considered. Currently we have different stages doing this to different degrees in different ways.

Patch:
diff --git a/stages/org.osbuild.kickstart b/stages/org.osbuild.kickstart
index 3994aa04..c3286a1f 100755
--- a/stages/org.osbuild.kickstart
+++ b/stages/org.osbuild.kickstart
@@ -198,6 +198,10 @@ SCHEMA = r"""
         }
       }
     }]
+  },
+  "display_mode": {
+    "description": "Perform the Kickstart installation in the given display mode",
+    "enum": ["text", "graphical", "cmdline"]
   }
 }
 """
@@ -342,6 +346,9 @@ def main(tree, options):
     clearpart = make_clearpart(options)
     if clearpart:
         config += [clearpart]
+    display_mode = options.get("display_mode")
+    if display_mode:
+        config += [display_mode]
 
     reboot = make_reboot(options)
     if reboot:
diff --git a/stages/org.osbuild.rpm b/stages/org.osbuild.rpm
index 24828a60..bdc33779 100755
--- a/stages/org.osbuild.rpm
+++ b/stages/org.osbuild.rpm
@@ -320,6 +320,8 @@ def main(tree, inputs, options):
         os.makedirs(target, exist_ok=True)
         mount(source, target, ro=False)
 
+    os.symlink("/proc/self/fd", f"{tree}/dev/fd")
+
     machine_id_created = create_machine_id_if_needed(tree)
 
     ostree_booted = None


Test patch:
diff --git a/stages/test/test_kickstart.py b/stages/test/test_kickstart.py
index 45d8956e..ff2d3856 100644
--- a/stages/test/test_kickstart.py
+++ b/stages/test/test_kickstart.py
@@ -98,13 +98,15 @@ TEST_INPUT = [
         },
         "lang en_US.UTF-8\nkeyboard us\ntimezone UTC\nzerombr\nclearpart --all --drives=sd*|hd*|vda,/dev/vdc",
     ),
-    # no reboot for an empty dict
     ({"reboot": True}, "reboot"),
     ({"reboot": {"eject": False}}, "reboot"),
     ({"reboot": {"eject": True}}, "reboot --eject"),
     ({"reboot": {"kexec": False}}, "reboot"),
     ({"reboot": {"kexec": True}}, "reboot --kexec"),
     ({"reboot": {"eject": True, "kexec": True}}, "reboot --eject --kexec"),
+    ({"display_mode": "text"}, "text"),
+    ({"display_mode": "graphical"}, "graphical"),
+    ({"display_mode": "cmdline"}, "cmdline"),
 ]
 
 
@@ -178,10 +180,12 @@ def test_kickstart_valid(tmp_path, test_input, expected):  # pylint: disable=unu
         ({"clearpart": {"list": ["\n%pre not allowed"]}}, "not allowed' does not match"),
         ({"clearpart": {"list": ["no,comma"]}}, "no,comma' does not match"),
         ({"clearpart": {"disklabel": "\n%pre not allowed"}}, "not allowed' does not match"),
+        ({"clearpart": {"random": "option"}}, "is not valid "),
         # schema ensures reboot has at least one option set
         ({"reboot": {}}, "{} is not valid under any of the given schemas"),
         ({"reboot": "random-string"}, "'random-string' is not valid "),
         ({"reboot": {"random": "option"}}, "{'random': 'option'} is not valid "),
+        ({"display_mode": "invalid-mode"}, "'invalid-mode' is not one of "),
     ],
 )
 def test_schema_validation_bad_apples(test_data, expected_err):