Skip to content

Commit

Permalink
MGMT-19100: Allow install to the existing root filesystem (#7197)
Browse files Browse the repository at this point in the history
* Add coreos image arg to install command

* Add coreos image to install command when INSTALL_TO_DISK is set

* Don't check disk performance for installs to the existing root

Use INSTALL_TO_EXISTING_ROOT to determine if we should check disk performance.
Running this check breaks the disk and fails the install when installing
to the booted disk.
  • Loading branch information
carbonin authored Jan 21, 2025
1 parent f913284 commit 6bdc133
Show file tree
Hide file tree
Showing 20 changed files with 200 additions and 55 deletions.

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

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

5 changes: 5 additions & 0 deletions internal/host/conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ func (v *validator) isInstallationDiskSpeedCheckSuccessful(c *validationContext)
return true
}

// Disk speed check is skipped when installing to the local disk
if v.installToExistingRoot {
return true
}

info, err := v.getBootDeviceInfo(c.host)
return err == nil && info != nil && info.DiskSpeed != nil && info.DiskSpeed.Tested && info.DiskSpeed.ExitCode == 0
}
Expand Down
51 changes: 51 additions & 0 deletions internal/host/conditions_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package host

import (
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
"github.com/openshift/assisted-service/internal/common"
"github.com/openshift/assisted-service/internal/hardware"
"github.com/openshift/assisted-service/models"
)

var _ = Describe("isInstallationDiskSpeedCheckSuccessful", func() {
var (
validationCtx *validationContext
v *validator
)

BeforeEach(func() {
validationCtx = &validationContext{
host: &models.Host{},
}
v = &validator{
log: common.GetTestLog(),
hwValidator: hardware.NewValidator(common.GetTestLog(), hardware.ValidatorCfg{}, nil, nil),
}
})

It("returns false when the infraenv is set", func() {
validationCtx.infraEnv = &common.InfraEnv{}
Expect(v.isInstallationDiskSpeedCheckSuccessful(validationCtx)).To(BeFalse())
})

It("returns true when disk partitions are being saved", func() {
validationCtx.host.InstallerArgs = "--save-partindex 1"
Expect(v.isInstallationDiskSpeedCheckSuccessful(validationCtx)).To(BeTrue())
})

It("returns true when installToExistingRoot is set", func() {
v.installToExistingRoot = true
Expect(v.isInstallationDiskSpeedCheckSuccessful(validationCtx)).To(BeTrue())
})

It("fails when install disk path can't be found", func() {
Expect(v.isInstallationDiskSpeedCheckSuccessful(validationCtx)).To(BeFalse())
})

It("succeeds when disk speed has been checked", func() {
validationCtx.host.InstallationDiskPath = "/dev/sda"
validationCtx.host.DisksInfo = "{\"/dev/sda\": {\"disk_speed\": {\"tested\": true, \"exit_code\": 0}, \"path\": \"/dev/sda\"}}"
Expect(v.isInstallationDiskSpeedCheckSuccessful(validationCtx)).To(BeTrue())
})
})
1 change: 1 addition & 0 deletions internal/host/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type Config struct {
BootstrapHostMAC string `envconfig:"BOOTSTRAP_HOST_MAC" default:""` // For ephemeral installer to ensure the bootstrap for the (single) cluster lands on the same host as assisted-service
MaxHostDisconnectionTime time.Duration `envconfig:"HOST_MAX_DISCONNECTION_TIME" default:"3m"`
EnableVirtualInterfaces bool `envconfig:"ENABLE_VIRTUAL_INTERFACES" default:"false"`
InstallToExistingRoot bool `envconfig:"INSTALL_TO_EXISTING_ROOT" default:"false"`

// hostStageTimeouts contains the values of the host stage timeouts. Don't use this
// directly, use the HostStageTimeout method instead.
Expand Down
2 changes: 1 addition & 1 deletion internal/host/host.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func NewManager(log logrus.FieldLogger, db *gorm.DB, notificationStream stream.N
hwValidator: hwValidator,
eventsHandler: eventsHandler,
sm: sm,
rp: newRefreshPreprocessor(log, hwValidatorCfg, hwValidator, operatorsApi, config.DisabledHostvalidations, providerRegistry, versionHandler),
rp: newRefreshPreprocessor(log, hwValidatorCfg, hwValidator, operatorsApi, config.DisabledHostvalidations, providerRegistry, versionHandler, config.InstallToExistingRoot),
metricApi: metricApi,
Config: *config,
leaderElector: leaderElector,
Expand Down
21 changes: 13 additions & 8 deletions internal/host/hostcommands/disk_performance_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,26 @@ import (

type diskPerfCheckCmd struct {
baseCmd
hwValidator hardware.Validator
diskPerfCheckImage string
timeoutSeconds float64
hwValidator hardware.Validator
diskPerfCheckImage string
timeoutSeconds float64
installToExistingRoot bool
}

func NewDiskPerfCheckCmd(log logrus.FieldLogger, diskPerfCheckImage string, hwValidator hardware.Validator, timeoutSeconds float64) *diskPerfCheckCmd {
func NewDiskPerfCheckCmd(log logrus.FieldLogger, diskPerfCheckImage string, hwValidator hardware.Validator, timeoutSeconds float64, installToExistingRoot bool) *diskPerfCheckCmd {
return &diskPerfCheckCmd{
baseCmd: baseCmd{log: log},
diskPerfCheckImage: diskPerfCheckImage,
hwValidator: hwValidator,
timeoutSeconds: timeoutSeconds,
baseCmd: baseCmd{log: log},
diskPerfCheckImage: diskPerfCheckImage,
hwValidator: hwValidator,
timeoutSeconds: timeoutSeconds,
installToExistingRoot: installToExistingRoot,
}
}

func (c *diskPerfCheckCmd) GetSteps(_ context.Context, host *models.Host) ([]*models.Step, error) {
if c.installToExistingRoot {
return nil, nil
}
bootDevice, err := hardware.GetBootDevice(c.hwValidator, host)
if err != nil {
return nil, err
Expand Down
9 changes: 8 additions & 1 deletion internal/host/hostcommands/disk_performance_cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ var _ = Describe("disk_performance", func() {
ctrl = gomock.NewController(GinkgoT())
mockValidator = hardware.NewMockValidator(ctrl)
mockValidator.EXPECT().GetHostInstallationPath(gomock.Any()).Return("/dev/sda").AnyTimes()
dCmd = NewDiskPerfCheckCmd(common.GetTestLog(), "quay.io/example/agent:latest", mockValidator, 600)
dCmd = NewDiskPerfCheckCmd(common.GetTestLog(), "quay.io/example/agent:latest", mockValidator, 600, false)

id = strfmt.UUID(uuid.New().String())
clusterId = strfmt.UUID(uuid.New().String())
Expand All @@ -56,6 +56,13 @@ var _ = Describe("disk_performance", func() {
Expect(stepReply).To(BeNil())
})

It("returns no steps when installToExistingRoot is true", func() {
dCmd.installToExistingRoot = true
steps, err := dCmd.GetSteps(ctx, &host)
Expect(steps).To(BeNil())
Expect(err).ToNot(HaveOccurred())
})

AfterEach(func() {
// cleanup
common.DeleteTestDB(db, dbName)
Expand Down
48 changes: 30 additions & 18 deletions internal/host/hostcommands/install_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,28 +41,31 @@ const (

type installCmd struct {
baseCmd
db *gorm.DB
hwValidator hardware.Validator
ocRelease oc.Release
instructionConfig InstructionConfig
eventsHandler eventsapi.Handler
versionsHandler versions.Handler
enableSkipMcoReboot bool
notifyNumReboots bool
db *gorm.DB
hwValidator hardware.Validator
ocRelease oc.Release
instructionConfig InstructionConfig
eventsHandler eventsapi.Handler
versionsHandler versions.Handler
enableSkipMcoReboot bool
notifyNumReboots bool
installToExistingRoot bool
}

func NewInstallCmd(log logrus.FieldLogger, db *gorm.DB, hwValidator hardware.Validator, ocRelease oc.Release,
instructionConfig InstructionConfig, eventsHandler eventsapi.Handler, versionsHandler versions.Handler, enableSkipMcoReboot, notifyNumReboots bool) *installCmd {
instructionConfig InstructionConfig, eventsHandler eventsapi.Handler, versionsHandler versions.Handler,
enableSkipMcoReboot, notifyNumReboots, installToExistingRoot bool) *installCmd {
return &installCmd{
baseCmd: baseCmd{log: log},
db: db,
hwValidator: hwValidator,
ocRelease: ocRelease,
instructionConfig: instructionConfig,
eventsHandler: eventsHandler,
versionsHandler: versionsHandler,
enableSkipMcoReboot: enableSkipMcoReboot,
notifyNumReboots: notifyNumReboots,
baseCmd: baseCmd{log: log},
db: db,
hwValidator: hwValidator,
ocRelease: ocRelease,
instructionConfig: instructionConfig,
eventsHandler: eventsHandler,
versionsHandler: versionsHandler,
enableSkipMcoReboot: enableSkipMcoReboot,
notifyNumReboots: notifyNumReboots,
installToExistingRoot: installToExistingRoot,
}
}

Expand Down Expand Up @@ -149,6 +152,15 @@ func (i *installCmd) getFullInstallerCommand(ctx context.Context, cluster *commo
if err != nil {
return "", err
}

if i.installToExistingRoot {
request.CoreosImage, err = i.ocRelease.GetCoreOSImage(i.log, *releaseImage.URL, i.instructionConfig.ReleaseImageMirror, cluster.PullSecret)
if err != nil {
return "", err
}
i.log.Infof("installing to disk with CoreOS image %s", request.CoreosImage)
}

i.log.Infof("Install command releaseImage: %s, mcoImage: %s", *releaseImage.URL, request.McoImage)

mustGatherMap, err := i.versionsHandler.GetMustGatherImages(cluster.OpenshiftVersion, cluster.CPUArchitecture, cluster.PullSecret)
Expand Down
39 changes: 27 additions & 12 deletions internal/host/hostcommands/install_cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ var _ = Describe("installcmd", func() {
mockEvents = eventsapi.NewMockHandler(ctrl)
mockVersions = versions.NewMockHandler(ctrl)
mockRelease = oc.NewMockRelease(ctrl)
installCmd = NewInstallCmd(common.GetTestLog(), db, mockValidator, mockRelease, instructionConfig, mockEvents, mockVersions, true, true)
installCmd = NewInstallCmd(common.GetTestLog(), db, mockValidator, mockRelease, instructionConfig, mockEvents, mockVersions, true, true, false)
cluster = createClusterInDb(db, models.ClusterHighAvailabilityModeFull)
clusterId = *cluster.ID
infraEnv = createInfraEnvInDb(db, clusterId)
Expand Down Expand Up @@ -109,7 +109,7 @@ var _ = Describe("installcmd", func() {
})
DescribeTable("enable MCO reboot values",
func(enableMcoReboot bool, version string, architecture string, expected bool) {
installCommand := NewInstallCmd(common.GetTestLog(), db, mockValidator, mockRelease, instructionConfig, mockEvents, mockVersions, enableMcoReboot, true)
installCommand := NewInstallCmd(common.GetTestLog(), db, mockValidator, mockRelease, instructionConfig, mockEvents, mockVersions, enableMcoReboot, true, false)
mockValidator.EXPECT().GetHostInstallationPath(gomock.Any()).Return(common.TestDiskId).Times(1)
mockGetReleaseImage(1)
mockImages(1)
Expand All @@ -133,7 +133,7 @@ var _ = Describe("installcmd", func() {

DescribeTable("notify num reboots",
func(notifyNumReboots bool) {
installCommand := NewInstallCmd(common.GetTestLog(), db, mockValidator, mockRelease, instructionConfig, mockEvents, mockVersions, true, notifyNumReboots)
installCommand := NewInstallCmd(common.GetTestLog(), db, mockValidator, mockRelease, instructionConfig, mockEvents, mockVersions, true, notifyNumReboots, false)
mockValidator.EXPECT().GetHostInstallationPath(gomock.Any()).Return(common.TestDiskId).Times(1)
mockGetReleaseImage(1)
mockImages(1)
Expand Down Expand Up @@ -423,7 +423,7 @@ var _ = Describe("installcmd arguments", func() {
Context("configuration_params", func() {
It("check_cluster_version_is_false_by_default", func() {
config := &InstructionConfig{}
installCmd := NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, *config, mockEvents, mockVersions, true, true)
installCmd := NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, *config, mockEvents, mockVersions, true, true, false)
stepReply, err := installCmd.GetSteps(ctx, &host)
Expect(err).NotTo(HaveOccurred())
Expect(stepReply).NotTo(BeNil())
Expand All @@ -435,7 +435,7 @@ var _ = Describe("installcmd arguments", func() {
config := &InstructionConfig{
CheckClusterVersion: false,
}
installCmd := NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, *config, mockEvents, mockVersions, true, true)
installCmd := NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, *config, mockEvents, mockVersions, true, true, false)
stepReply, err := installCmd.GetSteps(ctx, &host)
Expect(err).NotTo(HaveOccurred())
Expect(stepReply).NotTo(BeNil())
Expand All @@ -447,7 +447,7 @@ var _ = Describe("installcmd arguments", func() {
config := &InstructionConfig{
CheckClusterVersion: true,
}
installCmd := NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, *config, mockEvents, mockVersions, true, true)
installCmd := NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, *config, mockEvents, mockVersions, true, true, false)
stepReply, err := installCmd.GetSteps(ctx, &host)
Expect(err).NotTo(HaveOccurred())
Expect(stepReply).NotTo(BeNil())
Expand All @@ -456,7 +456,7 @@ var _ = Describe("installcmd arguments", func() {
})

It("verify high-availability-mode is None", func() {
installCmd := NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, InstructionConfig{}, mockEvents, mockVersions, true, true)
installCmd := NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, InstructionConfig{}, mockEvents, mockVersions, true, true, false)
stepReply, err := installCmd.GetSteps(ctx, &host)
Expect(err).NotTo(HaveOccurred())
Expect(stepReply).NotTo(BeNil())
Expand All @@ -469,7 +469,7 @@ var _ = Describe("installcmd arguments", func() {
mockRelease.EXPECT().GetMCOImage(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return("", nil).AnyTimes()
mockVersions.EXPECT().GetMustGatherImages(gomock.Any(), gomock.Any(), gomock.Any()).Return(defaultMustGatherVersion, nil).AnyTimes()

installCmd := NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, InstructionConfig{}, mockEvents, mockVersions, true, true)
installCmd := NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, InstructionConfig{}, mockEvents, mockVersions, true, true, false)
stepReply, err := installCmd.GetSteps(ctx, &host)
Expect(err).NotTo(HaveOccurred())
Expect(stepReply).NotTo(BeNil())
Expand All @@ -479,7 +479,7 @@ var _ = Describe("installcmd arguments", func() {

It("no must-gather , mco and openshift version in day2 installation", func() {
db.Model(&cluster).Update("kind", models.ClusterKindAddHostsCluster)
installCmd := NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, InstructionConfig{}, mockEvents, mockVersions, true, true)
installCmd := NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, InstructionConfig{}, mockEvents, mockVersions, true, true, false)
stepReply, err := installCmd.GetSteps(ctx, &host)
Expect(err).NotTo(HaveOccurred())
Expect(stepReply).NotTo(BeNil())
Expand All @@ -488,6 +488,21 @@ var _ = Describe("installcmd arguments", func() {
Expect(request.OpenshiftVersion).To(BeEmpty())
Expect(request.MustGatherImage).To(BeEmpty())
})

It("provides the coreos image when installToExistingRoot is set", func() {
testCoreOSImage := "example.com/coreos/image:latest"
mockRelease = oc.NewMockRelease(ctrl)
mockRelease.EXPECT().GetMCOImage(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return("", nil).AnyTimes()
mockVersions.EXPECT().GetMustGatherImages(gomock.Any(), gomock.Any(), gomock.Any()).Return(defaultMustGatherVersion, nil).AnyTimes()
mockRelease.EXPECT().GetCoreOSImage(gomock.Any(), *common.TestDefaultConfig.ReleaseImage.URL, gomock.Any(), gomock.Any()).Return(testCoreOSImage, nil).AnyTimes()

installCmd := NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, InstructionConfig{}, mockEvents, mockVersions, true, true, true)
stepReply, err := installCmd.GetSteps(ctx, &host)
Expect(err).NotTo(HaveOccurred())
Expect(stepReply).NotTo(BeNil())
request := generateRequestForStep(stepReply[0])
Expect(request.CoreosImage).To(Equal(testCoreOSImage))
})
})

Context("installer args", func() {
Expand All @@ -498,7 +513,7 @@ var _ = Describe("installcmd arguments", func() {

BeforeEach(func() {
instructionConfig = DefaultInstructionConfig
installCmd = NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, instructionConfig, mockEvents, mockVersions, true, true)
installCmd = NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, instructionConfig, mockEvents, mockVersions, true, true, false)
})

It("valid installer args", func() {
Expand Down Expand Up @@ -578,7 +593,7 @@ var _ = Describe("installcmd arguments", func() {

BeforeEach(func() {
instructionConfig = DefaultInstructionConfig
installCmd = NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, instructionConfig, mockEvents, mockVersions, true, true)
installCmd = NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, instructionConfig, mockEvents, mockVersions, true, true, false)
})

It("single argument with ocp image only", func() {
Expand Down Expand Up @@ -610,7 +625,7 @@ var _ = Describe("installcmd arguments", func() {

BeforeEach(func() {
instructionConfig = DefaultInstructionConfig
installCmd = NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, instructionConfig, mockEvents, mockVersions, true, true)
installCmd = NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, instructionConfig, mockEvents, mockVersions, true, true, false)
})
It("no-proxy without httpProxy", func() {
args := installCmd.getProxyArguments("t-cluster", "proxy.org", "", "", "domain.com,192.168.1.0/24")
Expand Down
5 changes: 3 additions & 2 deletions internal/host/hostcommands/instruction_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ type InstructionConfig struct {
DiskCheckTimeout time.Duration `envconfig:"DISK_CHECK_TIMEOUT" default:"8m"`
ImageAvailabilityTimeout time.Duration `envconfig:"IMAGE_AVAILABILITY_TIMEOUT" default:"16m"`
DisabledSteps []models.StepType `envconfig:"DISABLED_STEPS" default:""`
InstallToExistingRoot bool `envconfig:"INSTALL_TO_EXISTING_ROOT" default:"false"`
ReleaseImageMirror string
CheckClusterVersion bool
HostFSMountDir string
Expand All @@ -78,7 +79,7 @@ func NewInstructionManager(log logrus.FieldLogger, db *gorm.DB, hwValidator hard
instructionConfig InstructionConfig, connectivityValidator connectivity.Validator, eventsHandler eventsapi.Handler,
versionHandler versions.Handler, osImages versions.OSImages, kubeApiEnabled bool) *InstructionManager {
connectivityCmd := NewConnectivityCheckCmd(log, db, connectivityValidator, instructionConfig.AgentImage)
installCmd := NewInstallCmd(log, db, hwValidator, ocRelease, instructionConfig, eventsHandler, versionHandler, instructionConfig.EnableSkipMcoReboot, !kubeApiEnabled)
installCmd := NewInstallCmd(log, db, hwValidator, ocRelease, instructionConfig, eventsHandler, versionHandler, instructionConfig.EnableSkipMcoReboot, !kubeApiEnabled, instructionConfig.InstallToExistingRoot)
inventoryCmd := NewInventoryCmd(log, instructionConfig.AgentImage)
freeAddressesCmd := newFreeAddressesCmd(log, kubeApiEnabled)
stopCmd := NewStopInstallationCmd(log)
Expand All @@ -87,7 +88,7 @@ func NewInstructionManager(log logrus.FieldLogger, db *gorm.DB, hwValidator hard
apivipConnectivityCmd := NewAPIVIPConnectivityCheckCmd(log, db, instructionConfig.AgentImage)
tangConnectivityCmd := NewTangConnectivityCheckCmd(log, db, instructionConfig.AgentImage)
ntpSynchronizerCmd := NewNtpSyncCmd(log, instructionConfig.AgentImage, db)
diskPerfCheckCmd := NewDiskPerfCheckCmd(log, instructionConfig.AgentImage, hwValidator, instructionConfig.DiskCheckTimeout.Seconds())
diskPerfCheckCmd := NewDiskPerfCheckCmd(log, instructionConfig.AgentImage, hwValidator, instructionConfig.DiskCheckTimeout.Seconds(), instructionConfig.InstallToExistingRoot)
imageAvailabilityCmd := NewImageAvailabilityCmd(log, db, ocRelease, versionHandler, instructionConfig, instructionConfig.ImageAvailabilityTimeout.Seconds())
domainNameResolutionCmd := NewDomainNameResolutionCmd(log, instructionConfig.AgentImage, versionHandler, db)
noopCmd := NewNoopCmd()
Expand Down
Loading

0 comments on commit 6bdc133

Please sign in to comment.