Repository: osbuild/osbuild-composer
Pull request #1653 (2021-08-20 15:54:52)

Filesystem implementation follow up PR
This pull request includes support for the following mount points and sub mounts (as per COMPOSER-1018):
- [x] /home
- [x] /opt
- [x] /srv
- [x] /usr
- [x] /var

And the following mountpoints (as per COMPOSER-1016):
- [x] /app
- [x] /data
  
Additionally, integration filesystem integration test was improved to test the fail case where not allowed mountpoints are set in the blueprint filesystem customizations.

- [x] adequate testing for the new functionality or fixed issue
- [x] adequate documentation informing people about the change such as
  - [x] create a file in [news/unreleased](https://github.com/osbuild/osbuild-composer/tree/main/docs/news/unreleased) directory if this change should be mentioned in the release news
  - [x] submit a PR for the [guides](https://github.com/osbuild/guides) repository if this PR changed any behavior described there: https://www.osbuild.org/guides/

<!--
Thanks for proposing a change to osbuild-composer!

Please don't remove the above check list. These are things that each pull
request must have before it is merged. It helps maintainers to not forget
anything.

If the reason for ticking any of the boxes is ambiguous, please add a short
note explaining why.

For user-visible changes, "adequate documentation" is an entry describing the
change for users in docs/news. Please refer to docs/news/README.md for details.

In addition, if this pull request fixes a downstream issue, please refer to
test/README.md and add these additional items:

- [ ] 1st commit of any `rhbz#` related PR contains bug reproducer; CI reports FAIL or
- [ ] PR contains automated tests for new functionality and
- [ ] QE has approved reproducer/new tests and
- [ ] Subsequent commits provide bug fixes without modifying the reproducer; CI reports PASS and
- [ ] QE approves this PR; RHBZ status is set to `MODIFIED + Verified=Tested`
-->



Patch:
diff --git a/internal/distro/rhel85/distro.go b/internal/distro/rhel85/distro.go
index b97b83ef6..f6fe0b910 100644
--- a/internal/distro/rhel85/distro.go
+++ b/internal/distro/rhel85/distro.go
@@ -55,7 +55,11 @@ const (
 
 type basePartitionTableMap map[string]disk.PartitionTable
 
-var mountpointAllowList = []string{"/", "/var", "/var/*", "/home", "/opt", "/srv", "/usr"}
+var mountpointAllowList = []string{
+	"/", "/var", "/var/*", "/home", "/home/*", "/opt", "/opt/*",
+	"/srv", "/srv/*", "/usr", "/usr/*", "/app", "/app/*",
+	"/data", "/data/*",
+}
 
 type distribution struct {
 	name             string
diff --git a/internal/distro/rhel85/pipelines.go b/internal/distro/rhel85/pipelines.go
index 702870f68..3452f1079 100644
--- a/internal/distro/rhel85/pipelines.go
+++ b/internal/distro/rhel85/pipelines.go
@@ -389,11 +389,13 @@ func ec2X86_64BaseTreePipeline(repos []rpmmd.RepoConfig, packages []rpmmd.Packag
 	}
 
 	// EC2 x86_64-specific stages
+	// Add 'nvme' driver to handle the case when initramfs is getting forcefully
+	// rebuild on a Xen instance (and not able to boot on Nitro after that).
 	treePipeline.AddStage(osbuild.NewDracutConfStage(&osbuild.DracutConfStageOptions{
-		Filename: "xen.conf",
+		Filename: "ec2.conf",
 		Config: osbuild.DracutConfigFile{
 			AddDrivers: []string{
-				"xen-netfront",
+				"nvme",
 				"xen-blkfront",
 			},
 		},
diff --git a/internal/distro/rhel90/pipelines.go b/internal/distro/rhel90/pipelines.go
index bd5ade2e7..3c4b7acd0 100644
--- a/internal/distro/rhel90/pipelines.go
+++ b/internal/distro/rhel90/pipelines.go
@@ -370,11 +370,13 @@ func ec2X86_64BaseTreePipeline(repos []rpmmd.RepoConfig, packages []rpmmd.Packag
 	}
 
 	// EC2 x86_64-specific stages
+	// Add 'nvme' driver to handle the case when initramfs is getting forcefully
+	// rebuild on a Xen instance (and not able to boot on Nitro after that).
 	treePipeline.AddStage(osbuild.NewDracutConfStage(&osbuild.DracutConfStageOptions{
-		Filename: "xen.conf",
+		Filename: "ec2.conf",
 		Config: osbuild.DracutConfigFile{
 			AddDrivers: []string{
-				"xen-netfront",
+				"nvme",
 				"xen-blkfront",
 			},
 		},
diff --git a/internal/distro/rhel90/stage_options.go b/internal/distro/rhel90/stage_options.go
index 95a20c6b3..20e9047b7 100644
--- a/internal/distro/rhel90/stage_options.go
+++ b/internal/distro/rhel90/stage_options.go
@@ -332,7 +332,8 @@ func grub2StageOptions(rootPartition *disk.Partition, bootPartition *disk.Partit
 
 	if uefi {
 		stageOptions.UEFI = &osbuild.GRUB2UEFI{
-			Vendor: "redhat",
+			Vendor:  "redhat",
+			Unified: legacy == "", // force unified grub scheme for pure efi systems
 		}
 	}
 
diff --git a/internal/osbuild2/grub2_stage.go b/internal/osbuild2/grub2_stage.go
index 7ed0a0a97..4f416a706 100644
--- a/internal/osbuild2/grub2_stage.go
+++ b/internal/osbuild2/grub2_stage.go
@@ -20,7 +20,9 @@ type GRUB2StageOptions struct {
 }
 
 type GRUB2UEFI struct {
-	Vendor string `json:"vendor"`
+	Vendor  string `json:"vendor"`
+	Install bool   `json:"install,omitempty"`
+	Unified bool   `json:"unified,omitempty"`
 }
 
 func (GRUB2StageOptions) isStageOptions() {}


Test patch:
diff --git a/internal/distro/rhel85/distro_internal_test.go b/internal/distro/rhel85/distro_internal_test.go
new file mode 100644
index 000000000..63054949d
--- /dev/null
+++ b/internal/distro/rhel85/distro_internal_test.go
@@ -0,0 +1,85 @@
+package rhel85
+
+import (
+	"math/rand"
+	"testing"
+
+	"github.com/osbuild/osbuild-composer/internal/blueprint"
+	"github.com/osbuild/osbuild-composer/internal/disk"
+	"github.com/osbuild/osbuild-composer/internal/distro"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+var testBasicImageType = imageType{
+	name:                "test",
+	basePartitionTables: defaultBasePartitionTables,
+}
+
+var testEc2ImageType = imageType{
+	name:                "test_ec2",
+	basePartitionTables: ec2BasePartitionTables,
+}
+
+var mountpoints = []blueprint.FilesystemCustomization{
+	{
+		MinSize:    1024,
+		Mountpoint: "/usr",
+	},
+}
+
+var rng = rand.New(rand.NewSource(0))
+
+func containsMountpoint(expected []disk.Partition, mountpoint string) bool {
+	for _, p := range expected {
+		if p.Filesystem == nil {
+			continue
+		}
+		if p.Filesystem.Mountpoint == mountpoint {
+			return true
+		}
+	}
+	return false
+}
+
+func TestDistro_UnsupportedArch(t *testing.T) {
+	testBasicImageType.arch = &architecture{
+		name: "unsupported_arch",
+	}
+	_, err := testBasicImageType.getPartitionTable(mountpoints, distro.ImageOptions{}, rng)
+	require.EqualError(t, err, "unknown arch: "+testBasicImageType.arch.name)
+}
+
+func TestDistro_DefaultPartitionTables(t *testing.T) {
+	rhel8distro := New()
+	for _, archName := range rhel8distro.ListArches() {
+		testBasicImageType.arch = &architecture{
+			name: archName,
+		}
+		pt, err := testBasicImageType.getPartitionTable(mountpoints, distro.ImageOptions{}, rng)
+		require.Nil(t, err)
+		for _, m := range mountpoints {
+			contains := containsMountpoint(pt.Partitions, m.Mountpoint)
+			assert.True(t, contains)
+		}
+	}
+}
+
+func TestDistro_Ec2PartitionTables(t *testing.T) {
+	rhel8distro := New()
+	for _, archName := range rhel8distro.ListArches() {
+		testEc2ImageType.arch = &architecture{
+			name: archName,
+		}
+		pt, err := testEc2ImageType.getPartitionTable(mountpoints, distro.ImageOptions{}, rng)
+		if _, exists := testEc2ImageType.basePartitionTables[archName]; exists {
+			require.Nil(t, err)
+			for _, m := range mountpoints {
+				contains := containsMountpoint(pt.Partitions, m.Mountpoint)
+				assert.True(t, contains)
+			}
+		} else {
+			require.EqualError(t, err, "unknown arch: "+testEc2ImageType.arch.name)
+		}
+	}
+}
diff --git a/test/cases/filesystem.sh b/test/cases/filesystem.sh
index 32d55f4fc..3211635ee 100644
--- a/test/cases/filesystem.sh
+++ b/test/cases/filesystem.sh
@@ -11,8 +11,6 @@ if [[ "${ID}-${VERSION_ID}" != "rhel-8.5" ]]; then
     exit 0
 fi
 
-set -xeuo pipefail
-
 # Provision the software under test.
 /usr/libexec/osbuild-composer-test/provision.sh
 
@@ -39,6 +37,7 @@ build_image() {
     blueprint_file=$1
     blueprint_name=$2
     image_type=$3
+    want_fail=$4
 
     # Prepare the blueprint for the compose.
     greenprint "📋 Preparing blueprint"
@@ -55,7 +54,21 @@ build_image() {
     # Start the compose.
     greenprint "🚀 Starting compose"
     sudo composer-cli --json compose start "$blueprint_name" "$image_type" | tee "$COMPOSE_START"
-    COMPOSE_ID=$(jq -r '.build_id' "$COMPOSE_START")
+    STATUS=$(jq -r '.status' "$COMPOSE_START")
+    
+    if [[ $want_fail == "$STATUS" ]]; then
+        echo "Something went wrong with the compose. 😢"
+        sudo pkill -P ${WORKER_JOURNAL_PID}
+        trap - EXIT
+        exit 1
+    elif [[ $want_fail == true && $STATUS == false ]]; then
+        sudo pkill -P ${WORKER_JOURNAL_PID}
+        trap - EXIT
+        ERROR_MSG=$(jq 'first(.errors[] | select(.id == "ManifestCreationFailed")) | .msg' "$COMPOSE_START")
+        return
+    else
+      COMPOSE_ID=$(jq -r '.build_id' "$COMPOSE_START")
+    fi
 
     # Wait for the compose to finish.
     greenprint "⏱ Waiting for compose to finish: ${COMPOSE_ID}"
@@ -94,10 +107,12 @@ clean_up () {
 
 ##################################################
 ##
-## RHEL8.5 custom filesystems test
+## RHEL8.5 custom filesystems test - success case
 ##
 ##################################################
 
+greenprint "🚀 Checking custom filesystems (success case)"
+
 # Write a basic blueprint for our image.
 tee "$BLUEPRINT_FILE" > /dev/null << EOF
 name = "rhel85-custom-filesystem"
@@ -129,7 +144,7 @@ greenprint "📥 Downloading the image"
 sudo composer-cli compose image "${COMPOSE_ID}" > /dev/null
 IMAGE_FILENAME="${COMPOSE_ID}-disk.qcow2"
 
-greenprint "📥 Checking mountpoints"
+greenprint "💬 Checking mountpoints"
 INFO="$(sudo /usr/libexec/osbuild-composer-test/image-info "${IMAGE_FILENAME}")"
 FAILED_MOUNTPOINTS=()
 
@@ -141,10 +156,57 @@ for MOUNTPOINT in '/var' '/usr' '/' '/var/log'; do
 done
 
 # Clean compose and blueprints.
-greenprint "Clean up osbuild-composer again"
+greenprint "🧼 Clean up osbuild-composer again"
 sudo composer-cli compose delete "${COMPOSE_ID}" > /dev/null
 sudo composer-cli blueprints delete rhel85-custom-filesystem > /dev/null
 
+##################################################
+##
+## RHEL8.5 custom filesystems test - fail case
+##
+##################################################
+
+greenprint "🚀 Checking custom filesystems (fail case)"
+
+# Write a basic blueprint for our image.
+tee "$BLUEPRINT_FILE" > /dev/null << EOF
+name = "rhel85-custom-filesystem-fail"
+description = "A base system with custom mountpoints"
+version = "0.0.1"
+
+[[customizations.filesystem]]
+mountpoint = "/"
+size = 2147483648
+
+[[customizations.filesystem]]
+mountpoint = "/etc"
+size = 131072000
+
+[[customizations.filesystem]]
+mountpoint = "/boot"
+size = 131072000
+
+EOF
+
+# build_image "$BLUEPRINT_FILE" rhel85-custom-filesystem-fail qcow2 true
+build_image "$BLUEPRINT_FILE" rhel85-custom-filesystem-fail qcow2 true
+
+# Check error message.
+FAILED_MOUNTPOINTS=()
+
+greenprint "💬 Checking expected failures"
+for MOUNTPOINT in '/etc' '/boot' ; do
+  if ! [[ $ERROR_MSG == *"$MOUNTPOINT"* ]]; then
+    FAILED_MOUNTPOINTS+=("$MOUNTPOINT")
+  fi
+done
+
+# Clean compose and blueprints.
+greenprint "🧼 Clean up osbuild-composer again"
+sudo composer-cli blueprints delete rhel85-custom-filesystem-fail > /dev/null
+
+clean_up
+
 if [ ${#FAILED_MOUNTPOINTS[@]} -eq 0 ]; then
   echo "🎉 All tests passed."
   exit 0
diff --git a/test/data/manifests/rhel_85-x86_64-ami-boot.json b/test/data/manifests/rhel_85-x86_64-ami-boot.json
index 310a36010..a564f6193 100644
--- a/test/data/manifests/rhel_85-x86_64-ami-boot.json
+++ b/test/data/manifests/rhel_85-x86_64-ami-boot.json
@@ -1073,10 +1073,10 @@
           {
             "type": "org.osbuild.dracut.conf",
             "options": {
-              "filename": "xen.conf",
+              "filename": "ec2.conf",
               "config": {
                 "add_drivers": [
-                  "xen-netfront",
+                  "nvme",
                   "xen-blkfront"
                 ]
               }
diff --git a/test/data/manifests/rhel_85-x86_64-ec2-boot.json b/test/data/manifests/rhel_85-x86_64-ec2-boot.json
index 4440dfe8c..f2e388c14 100644
--- a/test/data/manifests/rhel_85-x86_64-ec2-boot.json
+++ b/test/data/manifests/rhel_85-x86_64-ec2-boot.json
@@ -1077,10 +1077,10 @@
           {
             "type": "org.osbuild.dracut.conf",
             "options": {
-              "filename": "xen.conf",
+              "filename": "ec2.conf",
               "config": {
                 "add_drivers": [
-                  "xen-netfront",
+                  "nvme",
                   "xen-blkfront"
                 ]
               }
diff --git a/test/data/manifests/rhel_85-x86_64-ec2_ha-boot.json b/test/data/manifests/rhel_85-x86_64-ec2_ha-boot.json
index 15e357b25..60dc2761c 100644
--- a/test/data/manifests/rhel_85-x86_64-ec2_ha-boot.json
+++ b/test/data/manifests/rhel_85-x86_64-ec2_ha-boot.json
@@ -1251,10 +1251,10 @@
           {
             "type": "org.osbuild.dracut.conf",
             "options": {
-              "filename": "xen.conf",
+              "filename": "ec2.conf",
               "config": {
                 "add_drivers": [
-                  "xen-netfront",
+                  "nvme",
                   "xen-blkfront"
                 ]
               }
diff --git a/test/data/manifests/rhel_90-aarch64-ami-boot.json b/test/data/manifests/rhel_90-aarch64-ami-boot.json
index ceb40c7ef..5cc73a640 100644
--- a/test/data/manifests/rhel_90-aarch64-ami-boot.json
+++ b/test/data/manifests/rhel_90-aarch64-ami-boot.json
@@ -1018,6 +1018,7 @@
               "boot_fs_uuid": "0194fdc2-fa2f-4cc0-81d3-ff12045b73c8",
               "kernel_opts": "console=ttyS0,115200n8 console=tty0 net.ifnames=0 rd.blacklist=nouveau nvme_core.io_timeout=4294967295 iommu.strict=0 crashkernel=auto",
               "uefi": {
+                "unified": true,
                 "vendor": "redhat"
               },
               "saved_entry": "ffffffffffffffffffffffffffffffff-5.14.0-0.rc3.29.el9.aarch64"
@@ -9920,4 +9921,4 @@
     },
     "timezone": "UTC"
   }
-}
+}
\ No newline at end of file
diff --git a/test/data/manifests/rhel_90-aarch64-qcow2-boot.json b/test/data/manifests/rhel_90-aarch64-qcow2-boot.json
index d2bb32cb6..71b3c527d 100644
--- a/test/data/manifests/rhel_90-aarch64-qcow2-boot.json
+++ b/test/data/manifests/rhel_90-aarch64-qcow2-boot.json
@@ -920,6 +920,7 @@
               "root_fs_uuid": "0194fdc2-fa2f-4cc0-81d3-ff12045b73c8",
               "kernel_opts": "console=tty0 console=ttyS0,115200n8 no_timer_check net.ifnames=0 crashkernel=auto",
               "uefi": {
+                "unified": true,
                 "vendor": "redhat"
               },
               "saved_entry": "ffffffffffffffffffffffffffffffff-5.14.0-0.rc3.29.el9.aarch64"
@@ -9866,4 +9867,4 @@
     },
     "timezone": "New_York"
   }
-}
+}
\ No newline at end of file
diff --git a/test/data/manifests/rhel_90-x86_64-ami-boot.json b/test/data/manifests/rhel_90-x86_64-ami-boot.json
index 67edf0330..fd4e5e4d3 100644
--- a/test/data/manifests/rhel_90-x86_64-ami-boot.json
+++ b/test/data/manifests/rhel_90-x86_64-ami-boot.json
@@ -973,10 +973,10 @@
           {
             "type": "org.osbuild.dracut.conf",
             "options": {
-              "filename": "xen.conf",
+              "filename": "ec2.conf",
               "config": {
                 "add_drivers": [
-                  "xen-netfront",
+                  "nvme",
                   "xen-blkfront"
                 ]
               }