From 843373ccff95820a06ded0cc3fa64d7a878debb6 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Mon, 7 Oct 2024 14:20:28 -0400 Subject: [PATCH 1/3] Add coreos image arg to install command --- .../assisted-service/models/install_cmd_request.go | 3 +++ .../assisted-service/models/install_cmd_request.go | 3 +++ models/install_cmd_request.go | 3 +++ restapi/embedded_spec.go | 8 ++++++++ swagger.yaml | 3 +++ .../assisted-service/models/install_cmd_request.go | 3 +++ 6 files changed, 23 insertions(+) diff --git a/api/vendor/github.com/openshift/assisted-service/models/install_cmd_request.go b/api/vendor/github.com/openshift/assisted-service/models/install_cmd_request.go index 4a9b4eae6eb..4ec6e462f6e 100644 --- a/api/vendor/github.com/openshift/assisted-service/models/install_cmd_request.go +++ b/api/vendor/github.com/openshift/assisted-service/models/install_cmd_request.go @@ -38,6 +38,9 @@ type InstallCmdRequest struct { // Pattern: ^(([a-zA-Z0-9\-\.]+)(:[0-9]+)?\/)?[a-z0-9\._\-\/@]+[?::a-zA-Z0-9_\-.]+$ ControllerImage *string `json:"controller_image"` + // CoreOS container image to use if installing to the local device + CoreosImage string `json:"coreos_image,omitempty"` + // List of disks to format DisksToFormat []string `json:"disks_to_format"` diff --git a/client/vendor/github.com/openshift/assisted-service/models/install_cmd_request.go b/client/vendor/github.com/openshift/assisted-service/models/install_cmd_request.go index 4a9b4eae6eb..4ec6e462f6e 100644 --- a/client/vendor/github.com/openshift/assisted-service/models/install_cmd_request.go +++ b/client/vendor/github.com/openshift/assisted-service/models/install_cmd_request.go @@ -38,6 +38,9 @@ type InstallCmdRequest struct { // Pattern: ^(([a-zA-Z0-9\-\.]+)(:[0-9]+)?\/)?[a-z0-9\._\-\/@]+[?::a-zA-Z0-9_\-.]+$ ControllerImage *string `json:"controller_image"` + // CoreOS container image to use if installing to the local device + CoreosImage string `json:"coreos_image,omitempty"` + // List of disks to format DisksToFormat []string `json:"disks_to_format"` diff --git a/models/install_cmd_request.go b/models/install_cmd_request.go index 4a9b4eae6eb..4ec6e462f6e 100644 --- a/models/install_cmd_request.go +++ b/models/install_cmd_request.go @@ -38,6 +38,9 @@ type InstallCmdRequest struct { // Pattern: ^(([a-zA-Z0-9\-\.]+)(:[0-9]+)?\/)?[a-z0-9\._\-\/@]+[?::a-zA-Z0-9_\-.]+$ ControllerImage *string `json:"controller_image"` + // CoreOS container image to use if installing to the local device + CoreosImage string `json:"coreos_image,omitempty"` + // List of disks to format DisksToFormat []string `json:"disks_to_format"` diff --git a/restapi/embedded_spec.go b/restapi/embedded_spec.go index a4110cfc9ea..cecefe033be 100644 --- a/restapi/embedded_spec.go +++ b/restapi/embedded_spec.go @@ -8979,6 +8979,10 @@ func init() { "type": "string", "pattern": "^(([a-zA-Z0-9\\-\\.]+)(:[0-9]+)?\\/)?[a-z0-9\\._\\-\\/@]+[?::a-zA-Z0-9_\\-.]+$" }, + "coreos_image": { + "description": "CoreOS container image to use if installing to the local device", + "type": "string" + }, "disks_to_format": { "description": "List of disks to format", "type": "array", @@ -19891,6 +19895,10 @@ func init() { "type": "string", "pattern": "^(([a-zA-Z0-9\\-\\.]+)(:[0-9]+)?\\/)?[a-z0-9\\._\\-\\/@]+[?::a-zA-Z0-9_\\-.]+$" }, + "coreos_image": { + "description": "CoreOS container image to use if installing to the local device", + "type": "string" + }, "disks_to_format": { "description": "List of disks to format", "type": "array", diff --git a/swagger.yaml b/swagger.yaml index b10b90ce5ef..14251fd035b 100644 --- a/swagger.yaml +++ b/swagger.yaml @@ -6751,6 +6751,9 @@ definitions: notify_num_reboots: type: boolean description: If true, notify number of reboots by assisted controller + coreos_image: + type: string + description: CoreOS container image to use if installing to the local device ntp_synchronization_request: type: object diff --git a/vendor/github.com/openshift/assisted-service/models/install_cmd_request.go b/vendor/github.com/openshift/assisted-service/models/install_cmd_request.go index 4a9b4eae6eb..4ec6e462f6e 100644 --- a/vendor/github.com/openshift/assisted-service/models/install_cmd_request.go +++ b/vendor/github.com/openshift/assisted-service/models/install_cmd_request.go @@ -38,6 +38,9 @@ type InstallCmdRequest struct { // Pattern: ^(([a-zA-Z0-9\-\.]+)(:[0-9]+)?\/)?[a-z0-9\._\-\/@]+[?::a-zA-Z0-9_\-.]+$ ControllerImage *string `json:"controller_image"` + // CoreOS container image to use if installing to the local device + CoreosImage string `json:"coreos_image,omitempty"` + // List of disks to format DisksToFormat []string `json:"disks_to_format"` From 932435806293ad923ed5f93c3f50349995a3f6e5 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Mon, 6 Jan 2025 16:17:24 -0500 Subject: [PATCH 2/3] Add coreos image to install command when INSTALL_TO_DISK is set --- internal/host/hostcommands/install_cmd.go | 48 ++++++++++++------- .../host/hostcommands/install_cmd_test.go | 39 ++++++++++----- .../host/hostcommands/instruction_manager.go | 3 +- .../host/refresh_status_preprocessor_test.go | 1 + internal/oc/mock_release.go | 15 ++++++ internal/oc/release.go | 7 +++ 6 files changed, 82 insertions(+), 31 deletions(-) diff --git a/internal/host/hostcommands/install_cmd.go b/internal/host/hostcommands/install_cmd.go index 385007e17f6..20cad8a6ec4 100644 --- a/internal/host/hostcommands/install_cmd.go +++ b/internal/host/hostcommands/install_cmd.go @@ -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, } } @@ -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) diff --git a/internal/host/hostcommands/install_cmd_test.go b/internal/host/hostcommands/install_cmd_test.go index 395849e8116..2973df1c2c4 100644 --- a/internal/host/hostcommands/install_cmd_test.go +++ b/internal/host/hostcommands/install_cmd_test.go @@ -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) @@ -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) @@ -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) @@ -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()) @@ -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()) @@ -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()) @@ -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()) @@ -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()) @@ -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()) @@ -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() { @@ -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() { @@ -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() { @@ -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") diff --git a/internal/host/hostcommands/instruction_manager.go b/internal/host/hostcommands/instruction_manager.go index d16b6b6b57a..2301a9a177e 100644 --- a/internal/host/hostcommands/instruction_manager.go +++ b/internal/host/hostcommands/instruction_manager.go @@ -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 @@ -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) diff --git a/internal/host/refresh_status_preprocessor_test.go b/internal/host/refresh_status_preprocessor_test.go index 9ccb7f8f48d..2b95acd17ef 100644 --- a/internal/host/refresh_status_preprocessor_test.go +++ b/internal/host/refresh_status_preprocessor_test.go @@ -77,6 +77,7 @@ var _ = Describe("Cluster Refresh Status Preprocessor", func() { disabledHostValidations, mockProviderRegistry, mockVersions, + false, ) }) diff --git a/internal/oc/mock_release.go b/internal/oc/mock_release.go index d930df38c8d..26decb86915 100644 --- a/internal/oc/mock_release.go +++ b/internal/oc/mock_release.go @@ -49,6 +49,21 @@ func (mr *MockReleaseMockRecorder) Extract(log, releaseImage, releaseImageMirror return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Extract", reflect.TypeOf((*MockRelease)(nil).Extract), log, releaseImage, releaseImageMirror, cacheDir, pullSecret, ocpVersion) } +// GetCoreOSImage mocks base method. +func (m *MockRelease) GetCoreOSImage(log logrus.FieldLogger, releaseImage, releaseImageMirror, pullSecret string) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetCoreOSImage", log, releaseImage, releaseImageMirror, pullSecret) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetCoreOSImage indicates an expected call of GetCoreOSImage. +func (mr *MockReleaseMockRecorder) GetCoreOSImage(log, releaseImage, releaseImageMirror, pullSecret interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetCoreOSImage", reflect.TypeOf((*MockRelease)(nil).GetCoreOSImage), log, releaseImage, releaseImageMirror, pullSecret) +} + // GetImageArchitecture mocks base method. func (m *MockRelease) GetImageArchitecture(log logrus.FieldLogger, image, pullSecret string) ([]string, error) { m.ctrl.T.Helper() diff --git a/internal/oc/release.go b/internal/oc/release.go index 806a28b2acf..5327225524a 100644 --- a/internal/oc/release.go +++ b/internal/oc/release.go @@ -33,6 +33,7 @@ const ( ironicAgentImageName = "ironic-agent" mustGatherImageName = "must-gather" okdRPMSImageName = "okd-rpms" + coreosImageName = "rhel-coreos" DefaultTries = 5 DefaltRetryDelay = time.Second * 5 staticInstallerRequiredVersion = "4.16.0-0.alpha" @@ -57,6 +58,7 @@ type Release interface { GetIronicAgentImage(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, pullSecret string) (string, error) GetOKDRPMSImage(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, pullSecret string) (string, error) GetMustGatherImage(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, pullSecret string) (string, error) + GetCoreOSImage(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, pullSecret string) (string, error) GetOpenshiftVersion(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, pullSecret string) (string, error) GetMajorMinorVersion(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, pullSecret string) (string, error) GetReleaseArchitecture(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, pullSecret string) ([]string, error) @@ -122,6 +124,11 @@ func (r *release) GetMustGatherImage(log logrus.FieldLogger, releaseImage string return r.getImageByName(log, mustGatherImageName, releaseImage, releaseImageMirror, pullSecret) } +// GetCoreOSImage gets rhel-coreos image URL from the release image or releaseImageMirror, if provided. +func (r *release) GetCoreOSImage(log logrus.FieldLogger, releaseImage string, releaseImageMirror string, pullSecret string) (string, error) { + return r.getImageByName(log, coreosImageName, releaseImage, releaseImageMirror, pullSecret) +} + func (r *release) getImageByName(log logrus.FieldLogger, imageName, releaseImage, releaseImageMirror, pullSecret string) (string, error) { var image string var err error From 5dbaa6cb10bb6a9ebdaa48743536ba7fbf2fc9c7 Mon Sep 17 00:00:00 2001 From: Nick Carboni Date: Mon, 21 Oct 2024 17:00:18 -0400 Subject: [PATCH 3/3] 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. --- internal/host/conditions.go | 5 ++ internal/host/conditions_test.go | 51 +++++++++++++++++++ internal/host/config.go | 1 + internal/host/host.go | 2 +- .../host/hostcommands/disk_performance_cmd.go | 21 +++++--- .../hostcommands/disk_performance_cmd_test.go | 9 +++- .../host/hostcommands/instruction_manager.go | 2 +- internal/host/refresh_status_preprocessor.go | 15 +++--- internal/host/validator.go | 13 ++--- 9 files changed, 95 insertions(+), 24 deletions(-) create mode 100644 internal/host/conditions_test.go diff --git a/internal/host/conditions.go b/internal/host/conditions.go index 6316cc9831f..61b095ee12f 100644 --- a/internal/host/conditions.go +++ b/internal/host/conditions.go @@ -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 } diff --git a/internal/host/conditions_test.go b/internal/host/conditions_test.go new file mode 100644 index 00000000000..af2d16b764f --- /dev/null +++ b/internal/host/conditions_test.go @@ -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()) + }) +}) diff --git a/internal/host/config.go b/internal/host/config.go index 2640587c6d6..3b305a530d6 100644 --- a/internal/host/config.go +++ b/internal/host/config.go @@ -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. diff --git a/internal/host/host.go b/internal/host/host.go index 885a29609ab..7079db59f88 100644 --- a/internal/host/host.go +++ b/internal/host/host.go @@ -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, diff --git a/internal/host/hostcommands/disk_performance_cmd.go b/internal/host/hostcommands/disk_performance_cmd.go index ca87c4a2fcc..3b9267aad17 100644 --- a/internal/host/hostcommands/disk_performance_cmd.go +++ b/internal/host/hostcommands/disk_performance_cmd.go @@ -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 diff --git a/internal/host/hostcommands/disk_performance_cmd_test.go b/internal/host/hostcommands/disk_performance_cmd_test.go index 4556bd989ca..e8f171e6298 100644 --- a/internal/host/hostcommands/disk_performance_cmd_test.go +++ b/internal/host/hostcommands/disk_performance_cmd_test.go @@ -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()) @@ -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) diff --git a/internal/host/hostcommands/instruction_manager.go b/internal/host/hostcommands/instruction_manager.go index 2301a9a177e..21c139f76dc 100644 --- a/internal/host/hostcommands/instruction_manager.go +++ b/internal/host/hostcommands/instruction_manager.go @@ -88,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() diff --git a/internal/host/refresh_status_preprocessor.go b/internal/host/refresh_status_preprocessor.go index 25a09e60030..559a6c1b753 100644 --- a/internal/host/refresh_status_preprocessor.go +++ b/internal/host/refresh_status_preprocessor.go @@ -43,14 +43,15 @@ type refreshPreprocessor struct { func newRefreshPreprocessor(log logrus.FieldLogger, hwValidatorCfg *hardware.ValidatorCfg, hwValidator hardware.Validator, operatorsApi operators.API, disabledHostValidations DisabledHostValidations, providerRegistry registry.ProviderRegistry, - versionHandler versions.Handler) *refreshPreprocessor { + versionHandler versions.Handler, installToExistingRoot bool) *refreshPreprocessor { v := &validator{ - log: log, - hwValidatorCfg: hwValidatorCfg, - hwValidator: hwValidator, - operatorsAPI: operatorsApi, - providerRegistry: providerRegistry, - versionHandler: versionHandler, + log: log, + hwValidatorCfg: hwValidatorCfg, + hwValidator: hwValidator, + operatorsAPI: operatorsApi, + providerRegistry: providerRegistry, + versionHandler: versionHandler, + installToExistingRoot: installToExistingRoot, } return &refreshPreprocessor{ log: log, diff --git a/internal/host/validator.go b/internal/host/validator.go index b8ca45adf6b..207e1615224 100644 --- a/internal/host/validator.go +++ b/internal/host/validator.go @@ -284,12 +284,13 @@ func boolValue(b bool) ValidationStatus { } type validator struct { - log logrus.FieldLogger - hwValidatorCfg *hardware.ValidatorCfg - hwValidator hardware.Validator - operatorsAPI operators.API - providerRegistry registry.ProviderRegistry - versionHandler versions.Handler + log logrus.FieldLogger + hwValidatorCfg *hardware.ValidatorCfg + hwValidator hardware.Validator + operatorsAPI operators.API + providerRegistry registry.ProviderRegistry + versionHandler versions.Handler + installToExistingRoot bool } func (v *validator) isMediaConnected(c *validationContext) (ValidationStatus, string) {