Skip to content

Commit

Permalink
MGMT-16273: Allow installing on iSCSI disks
Browse files Browse the repository at this point in the history
Allow booting from iSCSI for x86_64 OpenShift versions at least 4.15.0.
Multipath is not supported at this time.
  • Loading branch information
avishayt committed Nov 22, 2023
1 parent 7ec01c3 commit 381e8c8
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 15 deletions.
8 changes: 4 additions & 4 deletions internal/hardware/mock_validator.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 21 additions & 6 deletions internal/hardware/validator.go
Expand Up @@ -37,7 +37,7 @@ type Validator interface {
GetInfraEnvHostRequirements(ctx context.Context, infraEnv *common.InfraEnv) (*models.ClusterHostRequirements, error)
DiskIsEligible(ctx context.Context, disk *models.Disk, infraEnv *common.InfraEnv, cluster *common.Cluster, host *models.Host, hostArchitecture string, allDisks []*models.Disk) ([]string, error)
ListEligibleDisks(inventory *models.Inventory) []*models.Disk
IsValidStorageDeviceType(disk *models.Disk, hostArchitecture string) bool
IsValidStorageDeviceType(disk *models.Disk, hostArchitecture string, cluster *common.Cluster) bool
GetInstallationDiskSpeedThresholdMs(ctx context.Context, cluster *common.Cluster, host *models.Host) (int64, error)
// GetPreflightHardwareRequirements provides hardware (host) requirements that can be calculated only using cluster information.
// Returned information describe requirements coming from OCP and OLM operators.
Expand Down Expand Up @@ -126,9 +126,9 @@ func (v *validator) DiskIsEligible(ctx context.Context, disk *models.Disk, infra
humanize.Bytes(uint64(disk.SizeBytes)), humanize.Bytes(uint64(minSizeBytes))))
}

if !v.IsValidStorageDeviceType(disk, hostArchitecture) {
if !v.IsValidStorageDeviceType(disk, hostArchitecture, cluster) {
notEligibleReasons = append(notEligibleReasons,
fmt.Sprintf(wrongDriveTypeTemplate, disk.DriveType, strings.Join(v.getValidDeviceStorageTypes(hostArchitecture), ", ")))
fmt.Sprintf(wrongDriveTypeTemplate, disk.DriveType, strings.Join(v.getValidDeviceStorageTypes(hostArchitecture, cluster), ", ")))
}

// We only allow multipath if all paths are FC
Expand All @@ -147,8 +147,8 @@ func (v *validator) DiskIsEligible(ctx context.Context, disk *models.Disk, infra
return notEligibleReasons, nil
}

func (v *validator) IsValidStorageDeviceType(disk *models.Disk, hostArchitecture string) bool {
return funk.ContainsString(v.getValidDeviceStorageTypes(hostArchitecture), string(disk.DriveType))
func (v *validator) IsValidStorageDeviceType(disk *models.Disk, hostArchitecture string, cluster *common.Cluster) bool {
return funk.ContainsString(v.getValidDeviceStorageTypes(hostArchitecture, cluster), string(disk.DriveType))
}

func (v *validator) purgeServiceReasons(reasons []string) []string {
Expand Down Expand Up @@ -424,11 +424,26 @@ func (v *validator) getOCPRequirementsForVersion(openshiftVersion string) (*mode
return v.VersionedRequirements.GetVersionedHostRequirements(openshiftVersion)
}

func (v *validator) getValidDeviceStorageTypes(hostArchitecture string) []string {
func (v *validator) getValidDeviceStorageTypes(hostArchitecture string, cluster *common.Cluster) []string {
validTypes := []string{string(models.DriveTypeHDD), string(models.DriveTypeSSD), string(models.DriveTypeMultipath)}

if hostArchitecture == models.ClusterCPUArchitectureX8664 {
iSCSISupported := true
if cluster != nil {
isGreater, _ := common.BaseVersionGreaterOrEqual("4.15.0", cluster.OpenshiftVersion)
if !isGreater {
iSCSISupported = false
}
}
if iSCSISupported {
validTypes = append(validTypes, string(models.DriveTypeISCSI))
}
}

if hostArchitecture == models.ClusterCPUArchitectureS390x {
validTypes = append(validTypes, string(models.DriveTypeFC), string(models.DriveTypeECKDESE), string(models.DriveTypeECKD), string(models.DriveTypeFBA))
}

return validTypes
}

Expand Down
15 changes: 15 additions & 0 deletions internal/hardware/validator_test.go
Expand Up @@ -157,6 +157,21 @@ var _ = Describe("Disk eligibility", func() {
Expect(eligible).To(BeEmpty())
})

It("Check if iSCSI is eligible", func() {
testDisk.DriveType = models.DriveTypeISCSI

eligible, err := hwvalidator.DiskIsEligible(ctx, &testDisk, infraEnv, &cluster, &host, models.ClusterCPUArchitectureX8664, []*models.Disk{&testDisk})

Expect(err).ToNot(HaveOccurred())
Expect(eligible).To(BeEmpty())

By("Check infra env iSCSI is eligible")
eligible, err = hwvalidator.DiskIsEligible(ctx, &testDisk, infraEnv, nil, &host, models.ClusterCPUArchitectureX8664, []*models.Disk{&testDisk})

Expect(err).ToNot(HaveOccurred())
Expect(eligible).To(BeEmpty())
})

It("Check that FC multipath is eligible", func() {
testDisk.Name = "dm-0"
testDisk.DriveType = models.DriveTypeMultipath
Expand Down
8 changes: 6 additions & 2 deletions internal/host/hostcommands/install_cmd.go
Expand Up @@ -303,8 +303,12 @@ func constructHostInstallerArgs(cluster *common.Cluster, host *models.Host, inve
}

for _, disk := range inventory.Disks {
if disk.DriveType == models.DriveTypeMultipath && disk.ID == host.InstallationDiskID {
installerArgs = append(installerArgs, "--append-karg", "root=/dev/disk/by-label/dm-mpath-root", "--append-karg", "rw", "--append-karg", "rd.multipath=default")
if disk.ID == host.InstallationDiskID {
if disk.DriveType == models.DriveTypeMultipath {
installerArgs = append(installerArgs, "--append-karg", "root=/dev/disk/by-label/dm-mpath-root", "--append-karg", "rw", "--append-karg", "rd.multipath=default")
} else if disk.DriveType == models.DriveTypeISCSI {
installerArgs = append(installerArgs, "--append-karg", "rd.iscsi.firmware=1", "--append-karg", "ip=ibft")
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion internal/host/transition_test.go
Expand Up @@ -1577,7 +1577,7 @@ var _ = Describe("Refresh Host", func() {
)).AnyTimes()

mockDefaultClusterHostRequirements(mockHwValidator)
mockHwValidator.EXPECT().IsValidStorageDeviceType(gomock.Any(), gomock.Any()).Return(true).AnyTimes()
mockHwValidator.EXPECT().IsValidStorageDeviceType(gomock.Any(), gomock.Any(), nil).Return(true).AnyTimes()
mockHwValidator.EXPECT().ListEligibleDisks(gomock.Any()).Return([]*models.Disk{}).AnyTimes()
mockHwValidator.EXPECT().GetHostInstallationPath(gomock.Any()).Return("/dev/sda").AnyTimes()
}
Expand Down
2 changes: 1 addition & 1 deletion internal/host/validations_test.go
Expand Up @@ -82,7 +82,7 @@ var _ = Describe("Validations test", func() {

mockAndRefreshStatusWithoutEvents := func(h *models.Host) {
mockDefaultClusterHostRequirements(mockHwValidator)
mockHwValidator.EXPECT().IsValidStorageDeviceType(gomock.Any(), gomock.Any()).Return(true).AnyTimes()
mockHwValidator.EXPECT().IsValidStorageDeviceType(gomock.Any(), gomock.Any(), gomock.Any()).Return(true).AnyTimes()
mockHwValidator.EXPECT().ListEligibleDisks(gomock.Any()).Return([]*models.Disk{}).AnyTimes()
mockHwValidator.EXPECT().GetHostInstallationPath(gomock.Any()).Return("/dev/sda").AnyTimes()
mockOperators.EXPECT().ValidateHost(gomock.Any(), gomock.Any(), gomock.Any()).Return([]api.ValidationResult{
Expand Down
4 changes: 3 additions & 1 deletion internal/host/validator.go
Expand Up @@ -1703,7 +1703,9 @@ func (v *validator) isVSphereDiskUUIDEnabled(c *validationContext) (ValidationSt
// if any of them doesn't have that flag, it's likely because the user has forgotten to
// enable `disk.EnableUUID` for this virtual machine
// See https://access.redhat.com/solutions/4606201
if v.hwValidator.IsValidStorageDeviceType(disk, c.inventory.CPU.Architecture) && !disk.HasUUID {
if v.hwValidator.IsValidStorageDeviceType(disk, c.inventory.CPU.Architecture, nil) &&
funk.ContainsString([]string{string(models.DriveTypeHDD), string(models.DriveTypeSSD)}, string(disk.DriveType)) && // skip network disks
!disk.HasUUID {
return ValidationFailure, "VSphere disk.EnableUUID isn't enabled for this virtual machine, it's necessary for disks to be mounted properly"
}
}
Expand Down

0 comments on commit 381e8c8

Please sign in to comment.