Avoid race conditions due to using shortnames in enterprise linux bui…

…lds (#2220)

* Avoid race conditions due to using shortnames in enterprise linux builds

* Pass the name of the gce disk to be installed to the bootstrap stage through metadata

* Modify the kickstart config to reference this disk directly through the /dev/disk/by-id subsystem instead of hoping it appears second on /dev/sdb

* Comply with pycodestyle

* Append disk name in enterprise linux build_installer.py instead of overwriting

* Use persistent disk naming symlinks to reference disks in EL builds, including nvme devices

* Copy gce-disk-naming udev rule and dependencies into the anaconda environment

* Edit dependencies to remove non-coreutils dependency if necessary

* Re-generate udev rules in the anaconda environment

* comply with pycodestyle

* Add whitespace after function definition in build_installer.py

* Use el-install-disk for the name of the install disk in enterprise linux builds
	This Avoids setting it everytime

Document alternatives considered to the udev approach for disk selection

daisy_workflows/image_build/enterprise_linux/almalinux_8.wf.json

@@ -13,10 +13,6 @@
       "Value": "${TIMESTAMP}",
       "Description": "Build datestamp used to version the image."
     },
-    "install_disk": {
-      "Value": "disk-almalinux-8",
-      "Description": "Name of the disk to install onto."
-    },
     "publish_project": {
       "Value": "${PROJECT}",
       "Description": "A project to publish the resulting image to."
@@ -31,7 +27,6 @@
           "el_release": "almalinux-8",
           "kickstart_config": "./kickstart/almalinux_8.cfg",
           "google_cloud_repo": "${google_cloud_repo}",
-          "install_disk": "${install_disk}",
           "installer_iso": "${installer_iso}"
         }
       }
@@ -40,7 +35,7 @@
       "CreateImages": [
         {
           "Name": "almalinux-8-v${build_date}",
-          "SourceDisk": "${install_disk}",
+          "SourceDisk": "el-install-disk",
           "Licenses": [
             "projects/almalinux-cloud/global/licenses/almalinux-8"
           ],

daisy_workflows/image_build/enterprise_linux/almalinux_9.wf.json

@@ -13,10 +13,6 @@
       "Value": "${TIMESTAMP}",
       "Description": "Build datestamp used to version the image."
     },
-    "install_disk": {
-      "Value": "disk-almalinux-9",
-      "Description": "Name of the disk to install onto."
-    },
     "publish_project": {
       "Value": "${PROJECT}",
       "Description": "A project to publish the resulting image to."
@@ -31,7 +27,6 @@
           "el_release": "almalinux-9",
           "kickstart_config": "./kickstart/almalinux_9.cfg",
           "google_cloud_repo": "${google_cloud_repo}",
-          "install_disk": "${install_disk}",
           "installer_iso": "${installer_iso}"
         }
       }
@@ -40,7 +35,7 @@
       "CreateImages": [
         {
           "Name": "almalinux-9-v${build_date}",
-          "SourceDisk": "${install_disk}",
+          "SourceDisk": "el-install-disk",
           "Licenses": [
             "projects/almalinux-cloud/global/licenses/almalinux-9"
           ],

daisy_workflows/image_build/enterprise_linux/almalinux_9_arm64.wf.json

@@ -13,10 +13,6 @@
       "Value": "${TIMESTAMP}",
       "Description": "Build datestamp used to version the image."
     },
-    "install_disk": {
-      "Value": "disk-almalinux-9",
-      "Description": "Name of the disk to install onto."
-    },
     "publish_project": {
       "Value": "${PROJECT}",
       "Description": "A project to publish the resulting image to."
@@ -31,7 +27,6 @@
           "el_release": "almalinux-9",
           "kickstart_config": "./kickstart/almalinux_9_arm64.cfg",
           "google_cloud_repo": "${google_cloud_repo}",
-          "install_disk": "${install_disk}",
           "installer_iso": "${installer_iso}"
         }
       }
@@ -40,7 +35,7 @@
       "CreateImages": [
         {
           "Name": "almalinux-9-arm64-v${build_date}",
-          "SourceDisk": "${install_disk}",
+          "SourceDisk": "el-install-disk",
           "Licenses": [
             "projects/almalinux-cloud/global/licenses/almalinux-9"
           ],

daisy_workflows/image_build/enterprise_linux/build_installer.py

@@ -78,6 +78,17 @@ def main():
   utils.Execute(['cp', iso_file, 'installer/'])
   utils.Execute(['cp', ks_cfg, 'installer/'])
 
+  # The kickstart config contains a preinstall script copying, reloading, and
+  # triggering this rule in the install environment. This allows us to use
+  # predictable names for block devices. It would be perferable to take a
+  # simpler approach such as selecting the disk with an unkown partition table
+  # but kickstart does not believe the default google nvme device names are
+  # are deterministic and refuses to use them without user input.
+
+  utils.Execute(['cp', '-L', '/usr/lib/udev/rules.d/65-gce-disk-naming.rules',
+  'installer/'])
+  utils.Execute(['cp', '-L', '/usr/lib/udev/google_nvme_id', 'installer/'])
+
   # Modify boot config.
   with open('boot/EFI/BOOT/grub.cfg', 'r+') as f:
     oldcfg = f.read()
@@ -113,6 +124,17 @@ def main():
     f.write(cfg)
     f.truncate()
 
+  # Update google_nvme_id to remove xxd dependency, if necessary
+  # The current worker uses an older version
+  with open('installer/google_nvme_id', 'r+') as f:
+    old = f.read()
+    new = re.sub(r'xxd -p -seek 384 \| xxd -p -r',
+    'dd bs=1 skip=384 2>/dev/null',
+    old)
+    f.seek(0)
+    f.write(new)
+    f.truncate()
+
   utils.Execute(['umount', 'installer'])
   utils.Execute(['umount', 'iso'])
   utils.Execute(['umount', 'boot'])

daisy_workflows/image_build/enterprise_linux/centos_7.wf.json

@@ -13,10 +13,6 @@
       "Value": "${TIMESTAMP}",
       "Description": "Build datestamp used to version the image."
     },
-    "install_disk": {
-      "Value": "disk-centos-7",
-      "Description": "Name of the disk to install onto."
-    },
     "publish_project": {
       "Value": "${PROJECT}",
       "Description": "A project to publish the resulting image to."
@@ -30,8 +26,7 @@
         "Vars": {
           "el_release": "centos-7",
           "kickstart_config": "./kickstart/centos_7.cfg",
-          "google_cloud_repo": "${google_cloud_repo}",
-          "install_disk": "${install_disk}",
+          "google_cloud_repo": "${google_cloud_repo}", 
           "installer_iso": "${installer_iso}"
         }
       }
@@ -40,7 +35,7 @@
       "CreateImages": [
         {
           "Name": "centos-7-v${build_date}",
-          "SourceDisk": "${install_disk}",
+          "SourceDisk": "el-install-disk",
           "Licenses": [
             "projects/centos-cloud/global/licenses/centos-7"
           ],

daisy_workflows/image_build/enterprise_linux/centos_8.wf.json

@@ -13,10 +13,6 @@
       "Value": "${TIMESTAMP}",
       "Description": "Build datestamp used to version the image."
     },
-    "install_disk": {
-      "Value": "disk-centos-8",
-      "Description": "Name of the disk to install onto."
-    },
     "publish_project": {
       "Value": "${PROJECT}",
       "Description": "A project to publish the resulting image to."
@@ -31,7 +27,6 @@
           "el_release": "centos-8",
           "kickstart_config": "./kickstart/centos_8.cfg",
           "google_cloud_repo": "${google_cloud_repo}",
-          "install_disk": "${install_disk}",
           "installer_iso": "${installer_iso}"
         }
       }
@@ -40,7 +35,7 @@
       "CreateImages": [
         {
           "Name": "centos-8-v${build_date}",
-          "SourceDisk": "${install_disk}",
+          "SourceDisk": "el-install-disk",
           "Licenses": [
             "projects/centos-cloud/global/licenses/centos-8"
           ],

daisy_workflows/image_build/enterprise_linux/centos_stream_8.wf.json

@@ -13,10 +13,6 @@
       "Value": "${TIMESTAMP}",
       "Description": "Build datestamp used to version the image."
     },
-    "install_disk": {
-      "Value": "disk-centos-stream-8",
-      "Description": "Name of the disk to install onto."
-    },
     "publish_project": {
       "Value": "${PROJECT}",
       "Description": "A project to publish the resulting image to."
@@ -31,7 +27,6 @@
           "el_release": "centos-stream-8",
           "kickstart_config": "./kickstart/centos_stream_8.cfg",
           "google_cloud_repo": "${google_cloud_repo}",
-          "install_disk": "${install_disk}",
           "installer_iso": "${installer_iso}"
         }
       }
@@ -40,7 +35,7 @@
       "CreateImages": [
         {
           "Name": "centos-stream-8-v${build_date}",
-          "SourceDisk": "${install_disk}",
+          "SourceDisk": "el-install-disk",
           "Licenses": [
             "projects/centos-cloud/global/licenses/centos-stream"
           ],

daisy_workflows/image_build/enterprise_linux/centos_stream_9.wf.json

@@ -13,10 +13,6 @@
       "Value": "${TIMESTAMP}",
       "Description": "Build datestamp used to version the image."
     },
-    "install_disk": {
-      "Value": "disk-centos-stream-9",
-      "Description": "Name of the disk to install onto."
-    },
     "publish_project": {
       "Value": "${PROJECT}",
       "Description": "A project to publish the resulting image to."
@@ -31,7 +27,6 @@
           "el_release": "centos-stream-9",
           "kickstart_config": "./kickstart/centos_stream_9.cfg",
           "google_cloud_repo": "${google_cloud_repo}",
-          "install_disk": "${install_disk}",
           "installer_iso": "${installer_iso}"
         }
       }
@@ -40,7 +35,7 @@
       "CreateImages": [
         {
           "Name": "centos-stream-9-v${build_date}",
-          "SourceDisk": "${install_disk}",
+          "SourceDisk": "el-install-disk",
           "Licenses": [
             "projects/centos-cloud/global/licenses/centos-stream-9"
           ],

daisy_workflows/image_build/enterprise_linux/enterprise_linux.wf.json

@@ -22,10 +22,6 @@
       "Value": "stable",
       "Description": "The Google Cloud Repo branch to use."
     },
-    "install_disk": {
-      "Required": true,
-      "Description": "Name of the disk to install onto."
-    },
     "installer_iso": {
       "Required": true,
       "Description": "The path to the EL installation ISO."
@@ -62,7 +58,7 @@
           "GuestOsFeatures": [{"type": "UEFI_COMPATIBLE"}]
         },
         {
-          "Name": "${install_disk}",
+          "Name": "el-install-disk",
           "SizeGb": "20",
           "Type": "pd-ssd",
           "GuestOsFeatures": [{"type": "UEFI_COMPATIBLE"}]
@@ -115,7 +111,7 @@
       "CreateInstances": [
         {
           "Name": "inst-build",
-          "Disks": [{"Source": "disk-installer"}, {"Source": "${install_disk}"}],
+          "Disks": [{"Source": "disk-installer"}, {"Source": "el-install-disk"}],
           "MachineType": "${machine_type}"
         }
       ]

daisy_workflows/image_build/enterprise_linux/kickstart/almalinux_8.cfg

@@ -11,14 +11,6 @@ poweroff
 # Network configuration
 network --bootproto=dhcp --device=link
 
-### Disk configuration.
-# The bootloader must be set to sdb since sda is the installer.
-bootloader --boot-drive=sdb --timeout=0 --append="net.ifnames=0 biosdevname=0 scsi_mod.use_blk_mq=Y"
-# EFI partitioning, creates a GPT partitioned disk.
-clearpart --drives=sdb --all
-part /boot/efi --size=200 --fstype=efi --ondrive=sdb
-part / --size=100 --grow --ondrive=sdb --label=root --fstype=xfs
-
 ### Installed system configuration.
 firewall --enabled
 services --enabled=sshd,rngd --disabled=sshd-keygen@
@@ -28,6 +20,27 @@ rootpw --iscrypted --lock *
 firstboot --disabled
 user --name=gce --lock
 
+### Disk configuration.
+# Disk configuration is done by including a separate file with disk configuration, otherwise anaconda will try to validate that the disk exists before we configure udev rules.
+%pre --interpreter=/usr/bin/bash
+cp /run/install/isodir/65-gce-disk-naming.rules /etc/udev/rules.d/
+cp /run/install/isodir/google_nvme_id /usr/lib/udev/
+chmod +x /usr/lib/udev/google_nvme_id
+# Wait for coldplug events from boot to settle, or we won't generate new events for the reload/trigger
+udevadm settle
+udevadm control --reload
+udevadm trigger --settle
+tee -a /tmp/disk-config << EOM
+# build_installer.py will replace with the id of the install disk to avoid race conditions
+bootloader --boot-drive=/dev/disk/by-id/google-el-install-disk --timeout=0 --append="net.ifnames=0 biosdevname=0 scsi_mod.use_blk_mq=Y"
+# EFI partitioning, creates a GPT partitioned disk.
+clearpart --drives=/dev/disk/by-id/google-el-install-disk --all
+part /boot/efi --size=200 --fstype=efi --ondrive=/dev/disk/by-id/google-el-install-disk
+part / --size=100 --grow --ondrive=/dev/disk/by-id/google-el-install-disk --label=root --fstype=xfs
+EOM
+%end
+%include /tmp/disk-config
+
 # packages.cfg
 # Contains a list of packages to be installed, or not, on all flavors.
 # The %package command begins the package selection section of kickstart.

daisy_workflows/image_build/enterprise_linux/kickstart/almalinux_9.cfg

@@ -11,14 +11,6 @@ poweroff
 # Network configuration
 network --bootproto=dhcp --device=link
 
-### Disk configuration.
-# The bootloader must be set to sdb since sda is the installer.
-bootloader --boot-drive=sdb --timeout=0 --append="net.ifnames=0 biosdevname=0 scsi_mod.use_blk_mq=Y"
-# EFI partitioning, creates a GPT partitioned disk.
-clearpart --drives=sdb --all
-part /boot/efi --size=200 --fstype=efi --ondrive=sdb
-part / --size=100 --grow --ondrive=sdb --label=root --fstype=xfs
-
 ### Installed system configuration.
 firewall --enabled
 services --enabled=sshd,rngd --disabled=sshd-keygen@
@@ -28,6 +20,27 @@ rootpw --iscrypted --lock *
 firstboot --disabled
 user --name=gce --lock
 
+### Disk configuration.
+# Disk configuration is done by including a separate file with disk configuration, otherwise anaconda will try to validate that the disk exists before we configure udev rules.
+%pre --interpreter=/usr/bin/bash
+cp /run/install/isodir/65-gce-disk-naming.rules /etc/udev/rules.d/
+cp /run/install/isodir/google_nvme_id /usr/lib/udev/
+chmod +x /usr/lib/udev/google_nvme_id
+# Wait for coldplug events from boot to settle, or we won't generate new events for the reload/trigger
+udevadm settle
+udevadm control --reload
+udevadm trigger --settle
+tee -a /tmp/disk-config << EOM
+# build_installer.py will replace with the id of the install disk to avoid race conditions
+bootloader --boot-drive=/dev/disk/by-id/google-el-install-disk --timeout=0 --append="net.ifnames=0 biosdevname=0 scsi_mod.use_blk_mq=Y"
+# EFI partitioning, creates a GPT partitioned disk.
+clearpart --drives=/dev/disk/by-id/google-el-install-disk --all
+part /boot/efi --size=200 --fstype=efi --ondrive=/dev/disk/by-id/google-el-install-disk
+part / --size=100 --grow --ondrive=/dev/disk/by-id/google-el-install-disk --label=root --fstype=xfs
+EOM
+%end
+%include /tmp/disk-config
+
 # packages.cfg
 # Contains a list of packages to be installed, or not, on all flavors.
 # The %package command begins the package selection section of kickstart.

daisy_workflows/image_build/enterprise_linux/kickstart/almalinux_9_arm64.cfg

@@ -11,14 +11,6 @@ poweroff
 # Network configuration
 network --bootproto=dhcp --device=link
 
-### Disk configuration.
-# The bootloader must be set to nvme0n2 since sda is the installer.
-bootloader --boot-drive=nvme0n2 --timeout=0 --append="net.ifnames=0 biosdevname=0 scsi_mod.use_blk_mq=Y"
-# EFI partitioning, creates a GPT partitioned disk.
-clearpart --drives=nvme0n2 --all
-part /boot/efi --size=200 --fstype=efi --ondrive=nvme0n2
-part / --size=100 --grow --ondrive=nvme0n2 --label=root --fstype=xfs
-
 ### Installed system configuration.
 firewall --enabled
 services --enabled=sshd,rngd --disabled=sshd-keygen@
@@ -28,6 +20,27 @@ rootpw --iscrypted --lock *
 firstboot --disabled
 user --name=gce --lock
 
+### Disk configuration.
+# Disk configuration is done by including a separate file with disk configuration, otherwise anaconda will try to validate that the disk exists before we configure udev rules.
+%pre --interpreter=/usr/bin/bash
+cp /run/install/isodir/65-gce-disk-naming.rules /etc/udev/rules.d/
+cp /run/install/isodir/google_nvme_id /usr/lib/udev/
+chmod +x /usr/lib/udev/google_nvme_id
+# Wait for coldplug events from boot to settle, or we won't generate new events for the reload/trigger
+udevadm settle
+udevadm control --reload
+udevadm trigger --settle
+tee -a /tmp/disk-config << EOM
+# build_installer.py will replace with the id of the install disk to avoid race conditions
+bootloader --boot-drive=/dev/disk/by-id/google-el-install-disk --timeout=0 --append="net.ifnames=0 biosdevname=0 scsi_mod.use_blk_mq=Y"
+# EFI partitioning, creates a GPT partitioned disk.
+clearpart --drives=/dev/disk/by-id/google-el-install-disk --all
+part /boot/efi --size=200 --fstype=efi --ondrive=/dev/disk/by-id/google-el-install-disk
+part / --size=100 --grow --ondrive=/dev/disk/by-id/google-el-install-disk --label=root --fstype=xfs
+EOM
+%end
+%include /tmp/disk-config
+
 # packages.cfg
 # Contains a list of packages to be installed, or not, on all flavors.
 # The %package command begins the package selection section of kickstart.