Wipe Disks Improvements #885
Conversation
PR introduces two new fields in the harvester config wipeDisksList and wipeAllDisks and drops the old wipeDisks introduced in v1.3 There is an extra panel in disk configuration page. The extra panel contains a drop down menu which gets rendered and populated if installer finds additional disks with existing elemental based installations which are not already used in install device or additional data device
| ForceGPT bool `json:"forceGpt,omitempty"` | ||
| Role string `json:"role,omitempty"` | ||
| WithNetImages bool `json:"withNetImages,omitempty"` | ||
| WipeDisks bool `json:"wipeDisks,omitempty"` |
There was a problem hiding this comment.
Do we need any doc to indicate that harvester.install.wipe_disks is now deprecated? Presumably, with this change, all existing references to harvester.install.wipe_disks won't work anymore, right?
| // when WipeAllDisks is enabled then find all non installation disks with COS_ prefixed labels | ||
| // and add them to a list for wiping |
There was a problem hiding this comment.
From a UX perspective, we might need to document that this setting wipe all disks except for the installation and data disks.
| return err | ||
| } | ||
| if len(options) != 0 { | ||
| if slices.Contains(c.config.WipeDisksList, c.config.Device) || slices.Contains(c.config.WipeDisksList, c.config.DataDisk) { |
There was a problem hiding this comment.
Doesn't getWipeDisksOptions() already perform this filtering?
| // if there are no additional disks to wipe then it checks if MBR needs to be enabled | ||
| // else will move on to the next apge | ||
| isWipeDisksNeeded := func(g *gocui.Gui, v *gocui.View) error { | ||
| options, err := getWipeDisksOptions(c.config) |
There was a problem hiding this comment.
Is there an option for the user to not select a disk, and leave this blank? Or is it required to always provide at least one disk to wipe?
|
This pull request is now in conflict. Could you fix it @ibrokethecloud? 🙏 |
Problem:
We introduced a field in Harvester config named
wipeDiskswhich when set ran a script in the installer to iterate over all disk devices visible to the host and wipe them.This ended up being too aggressive, not to mention not all disks may contain old Harvester installations.
Solution:
PR introduces two new fields in the harvester config wipeDisksList and wipeAllDisks and drops the old wipeDisks introduced in v1.3
There is an extra panel in disk configuration page. The extra panel contains a drop down menu which gets rendered and populated if installer finds additional disks with existing elemental based installations which are not already used in install device or additional data device.
The extra
Wipe Diskspanels will only be available when there are additional disks visible to the host which have a partition withCOS_OEMlabel and such disk is not already being used inInstall DeviceorInstall Data Device.User can also pass these fields via ipxe using the following two new fields
or
wipeAllDisksuses the same logic as the installersWipe Disks Panelwhere it will identify additional disks withCOS_OEMpartition which are not already being used asInstall DeviceorInstall Data Device.Related Issue:
harvester/harvester#6304
harvester/harvester#6533
Test plan: