MGMT-19100: Allow install to the existing root filesystem #897

carbonin · 2025-01-17T21:02:19Z

This PR allows for installing coreos content to the existing root filesystem instead of only to an entire device.

When the install command contains the coreos image field it will be passed through to the installer.

Additionally the validation on eligible disks for iso type partitions is dropped.
This was done because it is not a valid way to detect installation media, but it is a way that config data can be provided to the host during a CAPI install.

For a full ISO the current install media partitions look like this:

[root@localhost core]# fdisk -l /dev/sr0
Disk /dev/sr0: 1.16 GiB, 1244659712 bytes, 607744 sectors
Disk model: QEMU DVD-ROM
Units: sectors of 1 * 2048 = 2048 bytes
Sector size (logical/physical): 2048 bytes / 2048 bytes
I/O size (minimum/optimal): 2048 bytes / 2048 bytes
Disklabel type: dos
Disk identifier: 0x47ad035f

Device     Boot Start     End Sectors  Size Id Type
/dev/sr0p1 *        0 2430975 2430976  4.6G  0 Empty
/dev/sr0p2        180   10139    9960 19.5M ef EFI (FAT-12/16/32)

Neither of these partitions are of type iso9660 so this check is doing nothing but making CAPI installs more difficult.

~~Leaving this as WIP until openshift/assisted-service#7197 and openshift/assisted-installer#1003 are merged~~ Merged

This is not a valid way to detect installation media, but it is a way that config data can be provided to the host during a CAPI install. For a full ISO the current install media partitions look like this: ``` [root@localhost core]# fdisk -l /dev/sr0 Disk /dev/sr0: 1.16 GiB, 1244659712 bytes, 607744 sectors Disk model: QEMU DVD-ROM Units: sectors of 1 * 2048 = 2048 bytes Sector size (logical/physical): 2048 bytes / 2048 bytes I/O size (minimum/optimal): 2048 bytes / 2048 bytes Disklabel type: dos Disk identifier: 0x47ad035f Device Boot Start End Sectors Size Id Type /dev/sr0p1 * 0 2430975 2430976 4.6G 0 Empty /dev/sr0p2 180 10139 9960 19.5M ef EFI (FAT-12/16/32) ``` Neither of these partitions are of type iso9660 so this check is doing nothing but making CAPI installs more difficult.

openshift-ci-robot · 2025-01-17T21:02:23Z

@carbonin: This pull request references MGMT-19100 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.19.0" version, but no target version was set.

In response to this:

This PR allows for installing coreos content to the existing root filesystem instead of only to an entire device.

When the install command contains the coreos image field it will be passed through to the installer.

Additionally the validation on eligible disks for iso type partitions is dropped.
This was done because it is not a valid way to detect installation media, but it is a way that config data can be provided to the host during a CAPI install.

For a full ISO the current install media partitions look like this:
[root@localhost core]# fdisk -l /dev/sr0
Disk /dev/sr0: 1.16 GiB, 1244659712 bytes, 607744 sectors
Disk model: QEMU DVD-ROM
Units: sectors of 1 * 2048 = 2048 bytes
Sector size (logical/physical): 2048 bytes / 2048 bytes
I/O size (minimum/optimal): 2048 bytes / 2048 bytes
Disklabel type: dos
Disk identifier: 0x47ad035f

Device     Boot Start     End Sectors  Size Id Type
/dev/sr0p1 *        0 2430975 2430976  4.6G  0 Empty
/dev/sr0p2        180   10139    9960 19.5M ef EFI (FAT-12/16/32)
Neither of these partitions are of type iso9660 so this check is doing nothing but making CAPI installs more difficult.

Leaving this as WIP until openshift/assisted-service#7197 and openshift/assisted-installer#1003 are merged

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci · 2025-01-17T21:03:36Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: carbonin

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [carbonin]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

codecov · 2025-01-17T21:25:18Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.43%. Comparing base (19cf7f6) to head (6a02366).
Report is 5 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #897      +/-   ##
==========================================
- Coverage   60.47%   60.43%   -0.05%     
==========================================
  Files          75       75              
  Lines        4003     3998       -5     
==========================================
- Hits         2421     2416       -5     
  Misses       1410     1410              
  Partials      172      172

Files with missing lines	Coverage Δ
src/commands/actions/install_cmd.go	`75.43% <100.00%> (+0.29%)`	⬆️
src/inventory/disks.go	`91.24% <ø> (-0.16%)`	⬇️

paul-maidment · 2025-01-19T13:06:45Z

src/inventory/disks.go

@@ -360,15 +360,6 @@ func (d *disks) checkEligibility(disk *ghw.Disk) (notEligibleReasons []string, i

 	// Check disk partitions for type, name, and mount points:
 	for _, partition := range disk.Partitions {
-		if partition.Type == "iso9660" {


Is there ever a case where the installation media would be ISO9660 ?
Just for clarification, it's accurate to say that isInstallationMedia will only be set if the mountpoint of a partition has the suffix 'iso' is this the only scenario where we would consider the partition to be installation media?

The installation media can be an iso but the partition never is.
I explain this in the commit and PR description, or are you asking about something else?

Just for clarification, it's accurate to say that isInstallationMedia will only be set if the mountpoint of a partition has the suffix 'iso' is this the only scenario where we would consider the partition to be installation media?

Yes, that will be the situation after this commit is merged. And this will successfully eliminate the cd drive device. You can see this in the following example from a machine with the full ISO booted:

[root@localhost core]# mount | grep iso /dev/sr0 on /run/media/iso type iso9660 (ro,relatime,nojoliet,check=s,map=n,blocksize=2048)

rccrdpccl · 2025-01-20T08:36:07Z

/lgtm

rccrdpccl · 2025-01-20T08:36:19Z

/hold

rccrdpccl · 2025-01-21T15:32:02Z

/retest

openshift-ci · 2025-01-22T01:05:24Z

@carbonin: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/edge-e2e-metal-assisted-single-node-4-18	`6a02366`	link	false	`/test edge-e2e-metal-assisted-single-node-4-18`
ci/prow/edge-e2e-oci-assisted-4-18	`6a02366`	link	false	`/test edge-e2e-oci-assisted-4-18`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

openshift-ci-robot · 2025-01-22T16:03:58Z

@carbonin: This pull request references MGMT-19100 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.19.0" version, but no target version was set.

In response to this:

This PR allows for installing coreos content to the existing root filesystem instead of only to an entire device.

When the install command contains the coreos image field it will be passed through to the installer.

Additionally the validation on eligible disks for iso type partitions is dropped.
This was done because it is not a valid way to detect installation media, but it is a way that config data can be provided to the host during a CAPI install.

For a full ISO the current install media partitions look like this:
[root@localhost core]# fdisk -l /dev/sr0
Disk /dev/sr0: 1.16 GiB, 1244659712 bytes, 607744 sectors
Disk model: QEMU DVD-ROM
Units: sectors of 1 * 2048 = 2048 bytes
Sector size (logical/physical): 2048 bytes / 2048 bytes
I/O size (minimum/optimal): 2048 bytes / 2048 bytes
Disklabel type: dos
Disk identifier: 0x47ad035f

Device     Boot Start     End Sectors  Size Id Type
/dev/sr0p1 *        0 2430975 2430976  4.6G  0 Empty
/dev/sr0p2        180   10139    9960 19.5M ef EFI (FAT-12/16/32)
Neither of these partitions are of type iso9660 so this check is doing nothing but making CAPI installs more difficult.

~~Leaving this as WIP until openshift/assisted-service#7197 and openshift/assisted-installer#1003 are merged~~ Merged

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

rccrdpccl · 2025-01-23T08:26:09Z

/lgtm

carbonin added 2 commits January 16, 2025 15:24

add coreos image installer cli arg

89cbaa5

carbonin requested review from avishayt and rccrdpccl January 17, 2025 21:02

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jan 17, 2025

openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 17, 2025

openshift-ci bot requested review from gamli75 and jhernand January 17, 2025 21:03

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 17, 2025

paul-maidment reviewed Jan 19, 2025

View reviewed changes

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 20, 2025

openshift-ci bot assigned rccrdpccl Jan 20, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 20, 2025

carbonin mentioned this pull request Jan 21, 2025

MGMT-19100: Allow install to the existing root filesystem openshift/assisted-service#7197

Merged

20 tasks

Update assisted-service dependency

6a02366

carbonin force-pushed the image-install branch from 6bf9ed0 to 6a02366 Compare January 21, 2025 22:01

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Jan 21, 2025

carbonin changed the title ~~[WIP] MGMT-19100: Allow install to the existing root filesystem~~ MGMT-19100: Allow install to the existing root filesystem Jan 21, 2025

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 23, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MGMT-19100: Allow install to the existing root filesystem #897

MGMT-19100: Allow install to the existing root filesystem #897

carbonin commented Jan 17, 2025 •

edited

Loading

openshift-ci-robot commented Jan 17, 2025 •

edited by openshift-ci bot

Loading

openshift-ci bot commented Jan 17, 2025

codecov bot commented Jan 17, 2025 •

edited

Loading

paul-maidment Jan 19, 2025 •

edited

Loading

carbonin Jan 20, 2025 •

edited

Loading

rccrdpccl commented Jan 20, 2025

rccrdpccl commented Jan 20, 2025

rccrdpccl commented Jan 21, 2025

openshift-ci bot commented Jan 22, 2025

openshift-ci-robot commented Jan 22, 2025 •

edited by openshift-ci bot

Loading

rccrdpccl commented Jan 23, 2025

MGMT-19100: Allow install to the existing root filesystem #897

Are you sure you want to change the base?

MGMT-19100: Allow install to the existing root filesystem #897

Conversation

carbonin commented Jan 17, 2025 • edited Loading

openshift-ci-robot commented Jan 17, 2025 • edited by openshift-ci bot Loading

openshift-ci bot commented Jan 17, 2025

codecov bot commented Jan 17, 2025 • edited Loading

Codecov Report

paul-maidment Jan 19, 2025 • edited Loading

Choose a reason for hiding this comment

carbonin Jan 20, 2025 • edited Loading

Choose a reason for hiding this comment

rccrdpccl commented Jan 20, 2025

rccrdpccl commented Jan 20, 2025

rccrdpccl commented Jan 21, 2025

openshift-ci bot commented Jan 22, 2025

openshift-ci-robot commented Jan 22, 2025 • edited by openshift-ci bot Loading

rccrdpccl commented Jan 23, 2025

carbonin commented Jan 17, 2025 •

edited

Loading

openshift-ci-robot commented Jan 17, 2025 •

edited by openshift-ci bot

Loading

codecov bot commented Jan 17, 2025 •

edited

Loading

paul-maidment Jan 19, 2025 •

edited

Loading

carbonin Jan 20, 2025 •

edited

Loading

openshift-ci-robot commented Jan 22, 2025 •

edited by openshift-ci bot

Loading