Skip to content

MGMT-19100: Install to the current boot device when CoreosImage is set #1003

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.

Sign up for GitHub

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

Merged
openshift-merge-bot merged 2 commits into from
Jan 22, 2025
Merged

MGMT-19100: Install to the current boot device when CoreosImage is set #1003

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b866489
Install to the existing root when CoreosImage is set
carbonin Oct 9, 2024
2bde274
Add a constant for docker auth file
carbonin Jan 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ type Config struct {
SkipInstallationDiskCleanup bool
EnableSkipMcoReboot bool
NotifyNumReboots bool
CoreosImage string
}

func printHelpAndExit(err error) {
Expand Down Expand Up @@ -79,6 +80,7 @@ func (c *Config) ProcessArgs(args []string) {
flagSet.BoolVar(&c.SkipInstallationDiskCleanup, "skip-installation-disk-cleanup", false, "Skip installation disk cleanup gives disk management to coreos-installer in case needed")
flagSet.BoolVar(&c.EnableSkipMcoReboot, "enable-skip-mco-reboot", false, "indicate assisted installer to generate settings to match MCO requirements for skipping reboot after firstboot")
flagSet.BoolVar(&c.NotifyNumReboots, "notify-num-reboots", false, "indicate number of reboots should be notified as event")
flagSet.StringVar(&c.CoreosImage, "coreos-image", "", "CoreOS image to install to the existing root")
Copy link
Contributor

@eranco74 eranco74 Jan 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider renaming CoreosImage to OciImage


var installerArgs string
flagSet.StringVar(&installerArgs, "installer-args", "", "JSON array of additional coreos-installer arguments")
Expand Down
6 changes: 6 additions & 0 deletions src/config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ var _ = Describe("ProcessArgs", func() {
Expect(config.InfraEnvID).To(Equal("9f2a26d7-10a6-4be0-b1c2-e895ad3b04b8"))
})

It("should set coreos image when provided", func() {
config := &Config{}
arguments := []string{"--role", string(models.HostRoleBootstrap), "--infra-env-id", "9f2a26d7-10a6-4be0-b1c2-e895ad3b04b8", "--cluster-id", "0ae63135-5f7c-431e-9c72-0efaf2cb83b8", "--coreos-image", "example.com/coreos/os:latest"}
config.ProcessArgs(arguments)
Expect(config.CoreosImage).To(Equal("example.com/coreos/os:latest"))
})
})

var _ = Describe("SetInstallerArgs", func() {
Expand Down
31 changes: 21 additions & 10 deletions src/installer/installer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,14 +155,17 @@ func (i *installer) InstallNode() error {
}
}

if i.EnableSkipMcoReboot {
i.skipMcoReboot(ignitionPath)
}
// we can't do any of this if we're installing to the existing root
if i.Config.CoreosImage == "" {
if i.EnableSkipMcoReboot {
i.skipMcoReboot(ignitionPath)
}

if err = i.ops.SetBootOrder(i.Device); err != nil {
i.log.WithError(err).Warnf("Failed to set boot order")
// Ignore the error for now so it doesn't fail the installation in case it fails
//return err
if err = i.ops.SetBootOrder(i.Device); err != nil {
i.log.WithError(err).Warnf("Failed to set boot order")
// Ignore the error for now so it doesn't fail the installation in case it fails
//return err
}
}

if isBootstrap {
Expand Down Expand Up @@ -321,7 +324,7 @@ func convertToOverwriteKargs(args []string) []string {
}

func (i *installer) cleanupInstallDevice() {
if i.DryRunEnabled || i.Config.SkipInstallationDiskCleanup {
if i.DryRunEnabled || i.Config.SkipInstallationDiskCleanup || i.Config.CoreosImage != "" {
i.log.Infof("skipping installation disk cleanup")
} else {
err := i.cleanupDevice.CleanupInstallDevice(i.Config.Device)
Expand Down Expand Up @@ -365,7 +368,11 @@ func (i *installer) writeImageToDisk(ignitionPath string) error {
interval := time.Second
liveLogger := coreos_logger.NewCoreosInstallerLogWriter(i.log, i.inventoryClient, i.Config.InfraEnvID, i.Config.HostID)
err := utils.Retry(3, interval, i.log, func() error {
return i.ops.WriteImageToDisk(liveLogger, ignitionPath, i.Device, i.Config.InstallerArgs)
if i.Config.CoreosImage == "" {
return i.ops.WriteImageToDisk(liveLogger, ignitionPath, i.Device, i.Config.InstallerArgs)
} else {
return i.ops.WriteImageToExistingRoot(liveLogger, ignitionPath)
Copy link
Contributor

@eranco74 eranco74 Jan 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should get the i.Config.CoreosImage, no?

}
})
if err != nil {
i.log.WithError(err).Error("Failed to write image to disk")
Expand Down Expand Up @@ -1044,7 +1051,11 @@ func RunInstaller(installerConfig *config.Config, logger *logrus.Logger) error {
)

// Try to format requested disks. May fail formatting some disks, this is not an error.
ai.FormatDisks()
if installerConfig.CoreosImage == "" {
ai.FormatDisks()
} else {
logger.Info("not attempting disk format when installing to boot device")
}

if err = ai.InstallNode(); err != nil {
ai.UpdateHostInstallProgress(models.HostStageFailed, err.Error())
Expand Down
85 changes: 56 additions & 29 deletions src/installer/installer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ var _ = Describe("installer HostRoleMaster role", func() {
mockops.EXPECT().WriteImageToDisk(gomock.Any(), filepath.Join(InstallDir, "master-host-id.ign"), device, extra).Return(nil).Times(1)
}

setBootOrderSuccess := func(extra interface{}) {
setBootOrderSuccess := func() {
mockops.EXPECT().SetBootOrder(device).Return(nil).Times(1)
}

Expand Down Expand Up @@ -389,7 +389,7 @@ var _ = Describe("installer HostRoleMaster role", func() {
downloadHostIgnitionSuccess(infraEnvId, hostId, "master-host-id.ign")
writeToDiskSuccess(gomock.Any())
reportLogProgressSuccess()
setBootOrderSuccess(gomock.Any())
setBootOrderSuccess()
uploadLogsSuccess(true)
ironicAgentDoesntExist()
rebootSuccess()
Expand Down Expand Up @@ -425,7 +425,7 @@ var _ = Describe("installer HostRoleMaster role", func() {
downloadHostIgnitionSuccess(infraEnvId, hostId, "master-host-id.ign")
writeToDiskSuccess(gomock.Any())
reportLogProgressSuccess()
setBootOrderSuccess(gomock.Any())
setBootOrderSuccess()
uploadLogsSuccess(true)
ironicAgentDoesntExist()
rebootSuccess()
Expand All @@ -449,7 +449,7 @@ var _ = Describe("installer HostRoleMaster role", func() {
//HostRoleMaster flow:
downloadHostIgnitionSuccess(infraEnvId, hostId, "master-host-id.ign")
writeToDiskSuccess(gomock.Any())
setBootOrderSuccess(gomock.Any())
setBootOrderSuccess()
getEncapsulatedMcSuccess(nil)
overwriteImageSuccess()
ret := installerObj.InstallNode()
Expand Down Expand Up @@ -480,7 +480,7 @@ var _ = Describe("installer HostRoleMaster role", func() {
downloadHostIgnitionSuccess(infraEnvId, hostId, "master-host-id.ign")
writeToDiskSuccess(gomock.Any())
reportLogProgressSuccess()
setBootOrderSuccess(gomock.Any())
setBootOrderSuccess()
uploadLogsSuccess(true)
ironicAgentDoesntExist()
rebootSuccess()
Expand Down Expand Up @@ -513,7 +513,7 @@ var _ = Describe("installer HostRoleMaster role", func() {
//HostRoleMaster flow:
downloadHostIgnitionSuccess(infraEnvId, hostId, "master-host-id.ign")
writeToDiskSuccess(gomock.Any())
setBootOrderSuccess(gomock.Any())
setBootOrderSuccess()
uploadLogsSuccess(true)
reportLogProgressSuccess()
ironicAgentDoesntExist()
Expand Down Expand Up @@ -554,7 +554,7 @@ var _ = Describe("installer HostRoleMaster role", func() {
downloadHostIgnitionSuccess(infraEnvId, hostId, "master-host-id.ign")
writeToDiskSuccess(gomock.Any())
reportLogProgressSuccess()
setBootOrderSuccess(gomock.Any())
setBootOrderSuccess()
uploadLogsSuccess(true)
ironicAgentDoesntExist()
rebootSuccess()
Expand Down Expand Up @@ -584,7 +584,7 @@ var _ = Describe("installer HostRoleMaster role", func() {
//HostRoleMaster flow:
downloadHostIgnitionSuccess(infraEnvId, hostId, "master-host-id.ign")
writeToDiskSuccess(gomock.Any())
setBootOrderSuccess(gomock.Any())
setBootOrderSuccess()
getEncapsulatedMcSuccess(nil)
overwriteImageSuccess()
ret := installerObj.InstallNode()
Expand Down Expand Up @@ -615,7 +615,7 @@ var _ = Describe("installer HostRoleMaster role", func() {
//HostRoleMaster flow:
downloadHostIgnitionSuccess(infraEnvId, hostId, "master-host-id.ign")
writeToDiskSuccess(gomock.Any())
setBootOrderSuccess(gomock.Any())
setBootOrderSuccess()
uploadLogsSuccess(true)
reportLogProgressSuccess()
ironicAgentDoesntExist()
Expand All @@ -637,7 +637,7 @@ var _ = Describe("installer HostRoleMaster role", func() {
downloadFileSuccess(bootstrapIgn)
downloadHostIgnitionSuccess(infraEnvId, hostId, "master-host-id.ign")
writeToDiskSuccess(gomock.Any())
setBootOrderSuccess(gomock.Any())
setBootOrderSuccess()
extractSecretFromIgnitionSuccess()
getEncapsulatedMcSuccess(nil)
overwriteImageSuccess()
Expand All @@ -661,7 +661,7 @@ var _ = Describe("installer HostRoleMaster role", func() {
//HostRoleMaster flow:
downloadHostIgnitionSuccess(infraEnvId, hostId, "master-host-id.ign")
writeToDiskSuccess(gomock.Any())
setBootOrderSuccess(gomock.Any())
setBootOrderSuccess()
getEncapsulatedMcSuccess(nil)
overwriteImageSuccess()
ret := installerObj.InstallNode()
Expand Down Expand Up @@ -748,7 +748,7 @@ var _ = Describe("installer HostRoleMaster role", func() {
//HostRoleMaster flow:
downloadHostIgnitionSuccess(infraEnvId, hostId, "master-host-id.ign")
writeToDiskSuccess(gomock.Any())
setBootOrderSuccess(gomock.Any())
setBootOrderSuccess()
uploadLogsSuccess(true)
reportLogProgressSuccess()
ironicAgentDoesntExist()
Expand Down Expand Up @@ -847,18 +847,23 @@ var _ = Describe("installer HostRoleMaster role", func() {
})
})
Context("Master role", func() {
installerArgs := []string{"-n", "--append-karg", "nameserver=8.8.8.8"}
conf := config.Config{Role: string(models.HostRoleMaster),
ClusterID: "cluster-id",
InfraEnvID: "infra-env-id",
HostID: "host-id",
Device: "/dev/vda",
URL: "https://assisted-service.com:80",
OpenshiftVersion: openShiftVersion,
InstallerArgs: installerArgs,
EnableSkipMcoReboot: withEnableSkipMcoReboot,
}
var (
conf config.Config
installerArgs []string
)

BeforeEach(func() {
installerArgs = []string{"-n", "--append-karg", "nameserver=8.8.8.8"}
conf = config.Config{Role: string(models.HostRoleMaster),
ClusterID: "cluster-id",
InfraEnvID: "infra-env-id",
HostID: "host-id",
Device: "/dev/vda",
URL: "https://assisted-service.com:80",
OpenshiftVersion: openShiftVersion,
InstallerArgs: installerArgs,
EnableSkipMcoReboot: withEnableSkipMcoReboot,
}
installerObj = NewAssistedInstaller(l, conf, mockops, mockbmclient, k8sBuilder, mockIgnition, cleanupDevice)
evaluateDiskSymlinkSuccess()

Expand All @@ -873,7 +878,7 @@ var _ = Describe("installer HostRoleMaster role", func() {
mkdirSuccess(InstallDir)
downloadHostIgnitionSuccess(infraEnvId, hostId, "master-host-id.ign")
writeToDiskSuccess(installerArgs)
setBootOrderSuccess(gomock.Any())
setBootOrderSuccess()
uploadLogsSuccess(false)
reportLogProgressSuccess()
ironicAgentDoesntExist()
Expand All @@ -884,6 +889,28 @@ var _ = Describe("installer HostRoleMaster role", func() {
Expect(ret).Should(BeNil())
})

It("installs to the existing root and does not skip reboot, clean install device, or set boot order when coreosImage is set", func() {
installerObj.Config.CoreosImage = "example.com/coreos/os:latest"
updateProgressSuccess([][]string{{string(models.HostStageStartingInstallation), conf.Role},
{string(models.HostStageInstalling), conf.Role},
{string(models.HostStageWritingImageToDisk)},
{string(models.HostStageRebooting)},
})
cleanupDevice.EXPECT().CleanupInstallDevice(device).Times(0)
mkdirSuccess(InstallDir)
downloadHostIgnitionSuccess(infraEnvId, hostId, "master-host-id.ign")
mockops.EXPECT().WriteImageToExistingRoot(gomock.Any(), filepath.Join(InstallDir, "master-host-id.ign")).Return(nil).Times(1)
mockops.EXPECT().SetBootOrder(device).Times(0)
uploadLogsSuccess(false)
reportLogProgressSuccess()
ironicAgentDoesntExist()
rebootSuccess()
mockops.EXPECT().GetEncapsulatedMC(gomock.Any()).Times(0)
mockops.EXPECT().OverwriteOsImage(gomock.Any(), gomock.Any(), gomock.Any()).Times(0)
ret := installerObj.InstallNode()
Expect(ret).Should(BeNil())
})

It("HostRoleMaster role happy flow with skipping disk cleanup", func() {
installerObj.Config.SkipInstallationDiskCleanup = true
cleanupDevice.EXPECT().CleanupInstallDevice(gomock.Any()).Return(nil).Times(0)
Expand All @@ -896,7 +923,7 @@ var _ = Describe("installer HostRoleMaster role", func() {
mkdirSuccess(InstallDir)
downloadHostIgnitionSuccess(infraEnvId, hostId, "master-host-id.ign")
writeToDiskSuccess(installerArgs)
setBootOrderSuccess(gomock.Any())
setBootOrderSuccess()
uploadLogsSuccess(false)
reportLogProgressSuccess()
ironicAgentDoesntExist()
Expand Down Expand Up @@ -963,7 +990,7 @@ var _ = Describe("installer HostRoleMaster role", func() {
mkdirSuccess(InstallDir)
downloadHostIgnitionSuccess(infraEnvId, hostId, "master-host-id.ign")
writeToDiskSuccess(installerArgs)
setBootOrderSuccess(gomock.Any())
setBootOrderSuccess()
uploadLogsSuccess(false)
reportLogProgressSuccess()
getEncapsulatedMcSuccess(nil)
Expand Down Expand Up @@ -1018,7 +1045,7 @@ var _ = Describe("installer HostRoleMaster role", func() {
uploadLogsSuccess(false)
reportLogProgressSuccess()
writeToDiskSuccess(installerArgs)
setBootOrderSuccess(gomock.Any())
setBootOrderSuccess()
getEncapsulatedMcSuccess(nil)
overwriteImageSuccess()
ironicAgentDoesntExist()
Expand Down Expand Up @@ -1070,7 +1097,7 @@ var _ = Describe("installer HostRoleMaster role", func() {
mkdirSuccess(InstallDir)
downloadHostIgnitionSuccess(infraEnvId, hostId, "worker-host-id.ign")
mockops.EXPECT().WriteImageToDisk(gomock.Any(), filepath.Join(InstallDir, "worker-host-id.ign"), device, nil).Return(nil).Times(1)
setBootOrderSuccess(gomock.Any())
setBootOrderSuccess()
// failure must do nothing
reportLogProgressSuccess()
mockops.EXPECT().UploadInstallationLogs(false).Return("", errors.Errorf("Dummy")).Times(1)
Expand Down Expand Up @@ -1176,7 +1203,7 @@ var _ = Describe("installer HostRoleMaster role", func() {
singleNodeMergeIgnitionSuccess()
downloadHostIgnitionSuccess(infraEnvId, hostId, "master-host-id.ign")
mockops.EXPECT().WriteImageToDisk(gomock.Any(), singleNodeMasterIgnitionPath, device, nil).Return(nil).Times(1)
setBootOrderSuccess(gomock.Any())
setBootOrderSuccess()
uploadLogsSuccess(true)
reportLogProgressSuccess()
ironicAgentDoesntExist()
Expand Down
14 changes: 14 additions & 0 deletions src/ops/mock_ops.go
View file
Open in desktop

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

Loading