Skip to content

Commit

Permalink
MGMT-15699: Service changes for avoiding MCO reboot (#5453)
Browse files Browse the repository at this point in the history 
Send the feature flag through the install command to indicate if the skip mco reboot feature is enabled.
The relevant logic is logic is done by the assisted installer.
  • Loading branch information
@ori-amizur
ori-amizur committed Sep 28, 2023
1 parent 41a00a0 commit efc20a5
Show file tree
Hide file tree
Showing 10 changed files with 77 additions and 34 deletions.
3 changes: 3 additions & 0 deletions api/vendor/github.com/openshift/assisted-service/models/install_cmd_request.go
View file

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

3 changes: 3 additions & 0 deletions client/vendor/github.com/openshift/assisted-service/models/install_cmd_request.go
View file

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
View file
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
View file
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
View file
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
View file
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
View file

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
View file

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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6449,6 +6449,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
3 changes: 3 additions & 0 deletions vendor/github.com/openshift/assisted-service/models/install_cmd_request.go
View file

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

0 comments on commit efc20a5

Please sign in to comment.