Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MGMT-15699: Service changes for avoiding MCO reboot #5453

Merged
merged 1 commit into from
Sep 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view

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.

3 changes: 3 additions & 0 deletions internal/feature/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,7 @@ type Flags struct {
// EnableRejectUnknownFields is a boolean flag to enable or disable rejecting unknown fields
// in JSON request bodies.
EnableRejectUnknownFields bool `envconfig:"ENABLE_REJECT_UNKNOWN_FIELDS" default:"true"`

// EnableSkipMcoReboot is a boolean flag to enable MCO reboot by assisted installer
EnableSkipMcoReboot bool `envconfig:"ENABLE_SKIP_MCO_REBOOT" default:"true"`
}
31 changes: 17 additions & 14 deletions internal/host/hostcommands/install_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,24 +26,26 @@ import (

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

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

Expand Down Expand Up @@ -112,6 +114,7 @@ func (i *installCmd) getFullInstallerCommand(ctx context.Context, cluster *commo
CheckCvo: swag.Bool(i.instructionConfig.CheckClusterVersion),
InstallerImage: swag.String(i.instructionConfig.InstallerImage),
BootDevice: swag.String(bootdevice),
EnableSkipMcoReboot: i.enableSkipMcoReboot,
}

// those flags are not used on day2 installation
Expand Down
52 changes: 33 additions & 19 deletions internal/host/hostcommands/install_cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/golang/mock/gomock"
"github.com/google/uuid"
. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/extensions/table"
. "github.com/onsi/gomega"
"github.com/openshift/assisted-service/internal/common"
eventgen "github.com/openshift/assisted-service/internal/common/events"
Expand Down Expand Up @@ -70,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)
installCmd = NewInstallCmd(common.GetTestLog(), db, mockValidator, mockRelease, instructionConfig, mockEvents, mockVersions, true)
cluster = createClusterInDb(db, models.ClusterHighAvailabilityModeFull)
clusterId = *cluster.ID
infraEnv = createInfraEnvInDb(db, clusterId)
Expand Down Expand Up @@ -106,14 +107,26 @@ var _ = Describe("installcmd", func() {
Expect(hostFromDb.InstallerVersion).Should(BeEmpty())
})
})
DescribeTable("enable MCO reboot values",
func(enableMcoReboot bool) {
installCommand := NewInstallCmd(common.GetTestLog(), db, mockValidator, mockRelease, instructionConfig, mockEvents, mockVersions, enableMcoReboot)
mockValidator.EXPECT().GetHostInstallationPath(gomock.Any()).Return(common.TestDiskId).Times(1)
mockGetReleaseImage(1)
mockImages(1)
installCmdSteps, stepErr = installCommand.GetSteps(ctx, &host)
validateInstallCommand(installCommand, installCmdSteps[0], models.HostRoleMaster, infraEnvId, clusterId, *host.ID, common.TestDiskId, nil, models.ClusterHighAvailabilityModeFull, enableMcoReboot)
},
Entry("Enbale MCO reboot is false", false),
Entry("Enbale MCO reboot is true", true),
)

It("get_step_one_master_success", func() {
mockValidator.EXPECT().GetHostInstallationPath(gomock.Any()).Return(common.TestDiskId).Times(1)
mockGetReleaseImage(1)
mockImages(1)
installCmdSteps, stepErr = installCmd.GetSteps(ctx, &host)
postvalidation(false, false, installCmdSteps[0], stepErr, models.HostRoleMaster)
validateInstallCommand(installCmd, installCmdSteps[0], models.HostRoleMaster, infraEnvId, clusterId, *host.ID, common.TestDiskId, nil, models.ClusterHighAvailabilityModeFull)
validateInstallCommand(installCmd, installCmdSteps[0], models.HostRoleMaster, infraEnvId, clusterId, *host.ID, common.TestDiskId, nil, models.ClusterHighAvailabilityModeFull, true)
hostFromDb := hostutil.GetHostFromDB(*host.ID, infraEnvId, db)
Expect(hostFromDb.InstallerVersion).Should(Equal(DefaultInstructionConfig.InstallerImage))
})
Expand All @@ -126,13 +139,13 @@ var _ = Describe("installcmd", func() {
mockImages(3)
installCmdSteps, stepErr = installCmd.GetSteps(ctx, &host)
postvalidation(false, false, installCmdSteps[0], stepErr, models.HostRoleMaster)
validateInstallCommand(installCmd, installCmdSteps[0], models.HostRoleMaster, infraEnvId, clusterId, *host.ID, common.TestDiskId, nil, models.ClusterHighAvailabilityModeFull)
validateInstallCommand(installCmd, installCmdSteps[0], models.HostRoleMaster, infraEnvId, clusterId, *host.ID, common.TestDiskId, nil, models.ClusterHighAvailabilityModeFull, true)
installCmdSteps, stepErr = installCmd.GetSteps(ctx, &host2)
postvalidation(false, false, installCmdSteps[0], stepErr, models.HostRoleMaster)
validateInstallCommand(installCmd, installCmdSteps[0], models.HostRoleMaster, infraEnvId, clusterId, *host2.ID, common.TestDiskId, nil, models.ClusterHighAvailabilityModeFull)
validateInstallCommand(installCmd, installCmdSteps[0], models.HostRoleMaster, infraEnvId, clusterId, *host2.ID, common.TestDiskId, nil, models.ClusterHighAvailabilityModeFull, true)
installCmdSteps, stepErr = installCmd.GetSteps(ctx, &host3)
postvalidation(false, false, installCmdSteps[0], stepErr, models.HostRoleBootstrap)
validateInstallCommand(installCmd, installCmdSteps[0], models.HostRoleBootstrap, infraEnvId, clusterId, *host3.ID, common.TestDiskId, nil, models.ClusterHighAvailabilityModeFull)
validateInstallCommand(installCmd, installCmdSteps[0], models.HostRoleBootstrap, infraEnvId, clusterId, *host3.ID, common.TestDiskId, nil, models.ClusterHighAvailabilityModeFull, true)
})
It("invalid_inventory", func() {
host.Inventory = "blah"
Expand Down Expand Up @@ -218,7 +231,7 @@ var _ = Describe("installcmd", func() {
prepareGetStep(sdb)
installCmdSteps, stepErr = installCmd.GetSteps(ctx, &host)
postvalidation(false, false, installCmdSteps[0], stepErr, models.HostRoleMaster)
validateInstallCommand(installCmd, installCmdSteps[0], models.HostRoleMaster, infraEnvId, clusterId, *host.ID, sdb.ID, getBootableDiskNames(disks), models.ClusterHighAvailabilityModeFull)
validateInstallCommand(installCmd, installCmdSteps[0], models.HostRoleMaster, infraEnvId, clusterId, *host.ID, sdb.ID, getBootableDiskNames(disks), models.ClusterHighAvailabilityModeFull, true)
hostFromDb := hostutil.GetHostFromDB(*host.ID, infraEnvId, db)
Expect(hostFromDb.InstallerVersion).Should(Equal(DefaultInstructionConfig.InstallerImage))
verifyDiskFormatCommand(installCmdSteps[0], sda.ID, true)
Expand All @@ -239,7 +252,7 @@ var _ = Describe("installcmd", func() {
prepareGetStep(sddd)
installCmdSteps, stepErr = installCmd.GetSteps(ctx, &host)
postvalidation(false, false, installCmdSteps[0], stepErr, models.HostRoleMaster)
validateInstallCommand(installCmd, installCmdSteps[0], models.HostRoleMaster, infraEnvId, clusterId, *host.ID, sddd.ID, getBootableDiskNames(disks), models.ClusterHighAvailabilityModeFull)
validateInstallCommand(installCmd, installCmdSteps[0], models.HostRoleMaster, infraEnvId, clusterId, *host.ID, sddd.ID, getBootableDiskNames(disks), models.ClusterHighAvailabilityModeFull, true)
hostFromDb := hostutil.GetHostFromDB(*host.ID, infraEnvId, db)
Expect(hostFromDb.InstallerVersion).Should(Equal(DefaultInstructionConfig.InstallerImage))
verifyDiskFormatCommand(installCmdSteps[0], sda.ID, true)
Expand All @@ -261,7 +274,7 @@ var _ = Describe("installcmd", func() {
prepareGetStep(sddd)
installCmdSteps, stepErr = installCmd.GetSteps(ctx, &host)
postvalidation(false, false, installCmdSteps[0], stepErr, models.HostRoleMaster)
validateInstallCommand(installCmd, installCmdSteps[0], models.HostRoleMaster, infraEnvId, clusterId, *host.ID, sddd.ID, getBootableDiskNames(disks), models.ClusterHighAvailabilityModeFull)
validateInstallCommand(installCmd, installCmdSteps[0], models.HostRoleMaster, infraEnvId, clusterId, *host.ID, sddd.ID, getBootableDiskNames(disks), models.ClusterHighAvailabilityModeFull, true)
hostFromDb := hostutil.GetHostFromDB(*host.ID, infraEnvId, db)
Expect(hostFromDb.InstallerVersion).Should(Equal(DefaultInstructionConfig.InstallerImage))
verifyDiskFormatCommand(installCmdSteps[0], sda.ID, true)
Expand Down Expand Up @@ -307,7 +320,7 @@ var _ = Describe("installcmd", func() {
prepareGetStep(sdb)
installCmdSteps, stepErr = installCmd.GetSteps(ctx, &host)
postvalidation(false, false, installCmdSteps[0], stepErr, models.HostRoleMaster)
validateInstallCommand(installCmd, installCmdSteps[0], models.HostRoleMaster, infraEnvId, clusterId, *host.ID, sdb.ID, []string{sda.ID, sdc.ID}, models.ClusterHighAvailabilityModeFull)
validateInstallCommand(installCmd, installCmdSteps[0], models.HostRoleMaster, infraEnvId, clusterId, *host.ID, sdb.ID, []string{sda.ID, sdc.ID}, models.ClusterHighAvailabilityModeFull, true)
hostFromDb := hostutil.GetHostFromDB(*host.ID, infraEnvId, db)
Expect(hostFromDb.InstallerVersion).Should(Equal(DefaultInstructionConfig.InstallerImage))
verifyDiskFormatCommand(installCmdSteps[0], sda.ID, true)
Expand Down Expand Up @@ -374,7 +387,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)
installCmd := NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, *config, mockEvents, mockVersions, true)
stepReply, err := installCmd.GetSteps(ctx, &host)
Expect(err).NotTo(HaveOccurred())
Expect(stepReply).NotTo(BeNil())
Expand All @@ -386,7 +399,7 @@ var _ = Describe("installcmd arguments", func() {
config := &InstructionConfig{
CheckClusterVersion: false,
}
installCmd := NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, *config, mockEvents, mockVersions)
installCmd := NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, *config, mockEvents, mockVersions, true)
stepReply, err := installCmd.GetSteps(ctx, &host)
Expect(err).NotTo(HaveOccurred())
Expect(stepReply).NotTo(BeNil())
Expand All @@ -398,7 +411,7 @@ var _ = Describe("installcmd arguments", func() {
config := &InstructionConfig{
CheckClusterVersion: true,
}
installCmd := NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, *config, mockEvents, mockVersions)
installCmd := NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, *config, mockEvents, mockVersions, true)
stepReply, err := installCmd.GetSteps(ctx, &host)
Expect(err).NotTo(HaveOccurred())
Expect(stepReply).NotTo(BeNil())
Expand All @@ -407,7 +420,7 @@ var _ = Describe("installcmd arguments", func() {
})

It("verify high-availability-mode is None", func() {
installCmd := NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, InstructionConfig{}, mockEvents, mockVersions)
installCmd := NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, InstructionConfig{}, mockEvents, mockVersions, true)
stepReply, err := installCmd.GetSteps(ctx, &host)
Expect(err).NotTo(HaveOccurred())
Expect(stepReply).NotTo(BeNil())
Expand All @@ -420,7 +433,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)
installCmd := NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, InstructionConfig{}, mockEvents, mockVersions, true)
stepReply, err := installCmd.GetSteps(ctx, &host)
Expect(err).NotTo(HaveOccurred())
Expect(stepReply).NotTo(BeNil())
Expand All @@ -430,7 +443,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)
installCmd := NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, InstructionConfig{}, mockEvents, mockVersions, true)
stepReply, err := installCmd.GetSteps(ctx, &host)
Expect(err).NotTo(HaveOccurred())
Expect(stepReply).NotTo(BeNil())
Expand All @@ -449,7 +462,7 @@ var _ = Describe("installcmd arguments", func() {

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

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

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

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

BeforeEach(func() {
instructionConfig = DefaultInstructionConfig
installCmd = NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, instructionConfig, mockEvents, mockVersions)
installCmd = NewInstallCmd(common.GetTestLog(), db, validator, mockRelease, instructionConfig, mockEvents, mockVersions, true)
})
It("no-proxy without httpProxy", func() {
args := installCmd.getProxyArguments("t-cluster", "proxy.org", "", "", "domain.com,192.168.1.0/24")
Expand Down Expand Up @@ -1174,7 +1187,7 @@ func postvalidation(isstepreplynil bool, issteperrnil bool, expectedstepreply *m
}

func validateInstallCommand(installCmd *installCmd, reply *models.Step, role models.HostRole, infraEnvId, clusterId, hostId strfmt.UUID,
bootDevice string, bootableDisks []string, haMode string) {
bootDevice string, bootableDisks []string, haMode string, enableSkipMcoReboot bool) {
ExpectWithOffset(1, reply.StepType).To(Equal(models.StepTypeInstall))
mustGatherImage, _ := installCmd.getMustGatherArgument(defaultMustGatherVersion)
request := models.InstallCmdRequest{}
Expand All @@ -1190,4 +1203,5 @@ func validateInstallCommand(installCmd *installCmd, reply *models.Step, role mod
Expect(request.McoImage).To(Equal(defaultMCOImage))
Expect(swag.StringValue(request.ControllerImage)).To(Equal(installCmd.instructionConfig.ControllerImage))
Expect(request.MustGatherImage).To(Equal(mustGatherImage))
Expect(request.EnableSkipMcoReboot).To(Equal(enableSkipMcoReboot))
}
2 changes: 1 addition & 1 deletion internal/host/hostcommands/instruction_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,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)
installCmd := NewInstallCmd(log, db, hwValidator, ocRelease, instructionConfig, eventsHandler, versionHandler, instructionConfig.EnableSkipMcoReboot)
inventoryCmd := NewInventoryCmd(log, instructionConfig.AgentImage)
freeAddressesCmd := newFreeAddressesCmd(log, kubeApiEnabled)
stopCmd := NewStopInstallationCmd(log)
Expand Down
3 changes: 3 additions & 0 deletions models/install_cmd_request.go

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

8 changes: 8 additions & 0 deletions restapi/embedded_spec.go

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

3 changes: 3 additions & 0 deletions swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6447,6 +6447,9 @@ definitions:
skip_installation_disk_cleanup:
type: boolean
description: Skip formatting installation disk
enable_skip_mco_reboot:
type: boolean
description: If true, assisted service will attempt to skip MCO reboot

ntp_synchronization_request:
type: object
Expand Down

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