Merge pull request #1921 from ogayot/nvmeotcp-rootfs

filesystem: accept to place the rootfs on remote storage for NVMe-o-TCP

examples/answers/nvme-over-tcp-rootfs.yaml

@@ -0,0 +1,58 @@
+#machine-config: examples/machines/nvme-over-tcp.json
+Source:
+  source: ubuntu-server
+Welcome:
+  lang: en_US
+Refresh:
+  update: no
+Keyboard:
+  layout: us
+Network:
+  accept-default: yes
+Proxy:
+  proxy: ""
+Mirror:
+  mirror: "http://fr.archive.ubuntu.com"
+Filesystem:
+  # 1. Reformat both disks
+  # 2. Create the root filesystem on disk 1 (which is a remote disk)
+  # 3. Create the /boot filesystem on disk 0 (which is a local disk)
+  manual:
+    - obj: [disk index 0]
+      action: REFORMAT
+    - obj: [disk index 1]
+      action: REFORMAT
+    - obj: [disk index 1]
+      action: PARTITION
+      data:
+        fstype: ext4
+        mount: /
+    - obj: [disk index 0]
+      action: PARTITION
+      data:
+        fstype: ext4
+        mount: /boot
+    - action: done
+Identity:
+  realname: Ubuntu
+  username: ubuntu
+  hostname: ubuntu-server
+  # ubuntu
+  password: '$6$wdAcoXrU039hKYPd$508Qvbe7ObUnxoj15DRCkzC3qO7edjH0VV7BPNRDYK4QR8ofJaEEF2heacn0QgD.f8pO8SNp83XNdWG6tocBM1'
+UbuntuPro:
+  token: ""
+SSH:
+  install_server: true
+  pwauth: false
+  authorized_keys:
+    - |
+      ssh-rsa AAAAAAAAAAAAAAAAAAAAAAAAA # ssh-import-id lp:subiquity
+SnapList:
+  snaps:
+    hello:
+      channel: stable
+      classic: false
+InstallProgress:
+  reboot: yes
+Drivers:
+  install: no

snapcraft.yaml

@@ -70,7 +70,7 @@ parts:
 
     source: https://git.launchpad.net/curtin
     source-type: git
-    source-commit: 3b17bab260ffa7efc1a2eb303a14aaf881cc3df0
+    source-commit: 5094d95fa5b8506412979e21548069bc12681dc7
 
     override-pull: |
       craftctl default

subiquity/models/filesystem.py

@@ -2235,8 +2235,27 @@ def _mount_for_path(self, path):
     def is_root_mounted(self):
         return self._mount_for_path("/") is not None
 
-    def can_install(self):
-        return self.is_root_mounted() and not self.needs_bootloader_partition()
+    def is_rootfs_on_remote_storage(self) -> bool:
+        return self._mount_for_path("/").device.volume.on_remote_storage()
+
+    def is_boot_mounted(self) -> bool:
+        return self._mount_for_path("/boot") is not None
+
+    def is_bootfs_on_remote_storage(self) -> bool:
+        return self._mount_for_path("/boot").device.volume.on_remote_storage()
+
+    def can_install(self) -> bool:
+        if not self.is_root_mounted():
+            return False
+
+        if self.is_rootfs_on_remote_storage():
+            if not self.is_boot_mounted() or self.is_bootfs_on_remote_storage():
+                return False
+
+        if self.needs_bootloader_partition():
+            return False
+
+        return True
 
     def should_add_swapfile(self):
         mount = self._mount_for_path("/")

subiquity/ui/mount.py

@@ -79,16 +79,16 @@ def enable_common_mountpoints(self):
             if isinstance(opt.value, str):
                 opt.enabled = opt.value in common_mountpoints
 
-    def disable_all_mountpoints_but_home(self):
-        """Currently, we only want filesystems of non-local disks mounted at
-        /home. This is not completely enforced by the UI though. Users can
-        still select "other" and type "/", "/boot", "/var" or anything else."""
+    def disable_boot_boot_efi_mountpoints(self):
+        """Currently, we want /boot and /boot/efi filesystems to be stored on
+        local disks. This is not completely enforced by the UI though.
+        Note that users can still select "other" and type "/boot",
+        "/boot/efi"."""
         for opt in self._selector._options:
             if not isinstance(opt.value, str):
                 continue
-            if opt.value == "/home":
-                continue
-            opt.enabled = False
+            if opt.value == "/boot" or opt.value == "/boot/efi":
+                opt.enabled = False
 
     def _showhide_other(self, show):
         if show and not self._other_showing:

subiquity/ui/views/filesystem/filesystem.py

@@ -488,6 +488,13 @@ def _guidance(self):
         todos = []
         if not self.model.is_root_mounted():
             todos.append(_("Mount a filesystem at /"))
+        elif self.model.is_rootfs_on_remote_storage():
+            # We need a /boot partition on local storage
+            if (
+                not self.model.is_boot_mounted()
+                or self.model.is_bootfs_on_remote_storage()
+            ):
+                todos.append(_("Mount a local filesystem at /boot"))
         if self.model.needs_bootloader_partition():
             todos.append(_("Select a boot disk"))
         if not todos:

subiquity/ui/views/filesystem/partition.py

@@ -180,8 +180,6 @@ def __init__(
             if ofstype:
                 self.existing_fs_type = ofstype
         initial_path = initial.get("mount")
-        if not initial_path and remote_storage:
-            initial["mount"] = "/home"
         self.mountpoints = {
             m.path: m.device.volume
             for m in self.model.all_mounts()
@@ -209,7 +207,7 @@ def select_fstype(self, sender, fstype):
             self.mount.widget.enable_common_mountpoints()
             self.mount.value = self.mount.value
         if self.remote_storage:
-            self.mount.widget.disable_all_mountpoints_but_home()
+            self.mount.widget.disable_boot_boot_efi_mountpoints()
             self.mount.value = self.mount.value
         if fstype is None:
             if self.existing_fs_type == "swap":
@@ -312,14 +310,14 @@ def validate_mount(self):
                                 ).format(mountpoint=mount),
                             )
                         )
-        if self.remote_storage and mount != "/home":
+        if self.remote_storage and mount in ("/boot", "/boot/efi"):
             self.mount.show_extra(
                 (
                     "info_error",
                     _(
-                        "This filesystem is on remote storage. It is usually a "
-                        "bad idea to mount it anywhere but at /home, proceed "
-                        "only with caution."
+                        f"The filesystem for {mount} should be stored on local"
+                        " storage. Storing it on remote storage will likely"
+                        " prevent the system for booting."
                     ),
                 )
             )