Skip to content

Fixes for image sub commands #564

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
merged 3 commits into from
Jan 17, 2025
Merged

Fixes for image sub commands #564

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8c62bf2
fix: output of describe command
bahkauv70 Jan 17, 2025
f158c45
fix: require rescue bus and device as pair, remove unsupported local-…
bahkauv70 Jan 17, 2025
892009c
fix: added testcases for flag groups
bahkauv70 Jan 17, 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
1 change: 1 addition & 0 deletions internal/cmd/beta/image/create/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ func configureFlags(cmd *cobra.Command) {
if err := flags.MarkFlagsRequired(cmd, nameFlag, diskFormatFlag, localFilePathFlag); err != nil {
cobra.CheckErr(err)
}
cmd.MarkFlagsRequiredTogether(rescueBusFlag, rescueDeviceFlag)
}

func parseInput(p *print.Printer, cmd *cobra.Command) (*inputModel, error) {
Expand Down
33 changes: 33 additions & 0 deletions internal/cmd/beta/image/create/create_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,32 @@ func TestParseInput(t *testing.T) {
}
}),
},
{
description: "only rescue bus is invalid",
flagValues: fixtureFlagValues(func(flagValues map[string]string) {
delete(flagValues, rescueDeviceFlag)
}),
isValid: false,
},
{
description: "only rescue device is invalid",
flagValues: fixtureFlagValues(func(flagValues map[string]string) {
delete(flagValues, rescueBusFlag)
}),
isValid: false,
},
{
description: "no rescue device and no bus is valid",
flagValues: fixtureFlagValues(func(flagValues map[string]string) {
delete(flagValues, rescueBusFlag)
delete(flagValues, rescueDeviceFlag)
}),
isValid: true,
expectedModel: fixtureInputModel(func(model *inputModel) {
model.Config.RescueBus = nil
model.Config.RescueDevice = nil
}),
},
}

for _, tt := range tests {
Expand All @@ -253,6 +279,13 @@ func TestParseInput(t *testing.T) {
}
}

if err := cmd.ValidateFlagGroups(); err != nil {
if !tt.isValid {
return
}
t.Fatalf("error validating flag groups: %v", err)
}

if err := cmd.ValidateRequiredFlags(); err != nil {
if !tt.isValid {
return
Expand Down
10 changes: 5 additions & 5 deletions internal/cmd/beta/image/describe/describe.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ func outputResult(p *print.Printer, model *inputModel, resp *iaas.Image) error {

return nil
case print.YAMLOutputFormat:
details, err := yaml.MarshalWithOptions(resp, yaml.IndentSequence(true))
details, err := yaml.MarshalWithOptions(resp, yaml.IndentSequence(true), yaml.UseJSONMarshaler())
if err != nil {
return fmt.Errorf("marshal image: %w", err)
}
Expand Down Expand Up @@ -141,12 +141,12 @@ func outputResult(p *print.Printer, model *inputModel, resp *iaas.Image) error {
table.AddRow("OPERATING SYSTEM", *os)
table.AddSeparator()
}
if distro := config.OperatingSystemDistro; distro != nil {
table.AddRow("OPERATING SYSTEM DISTRIBUTION", *distro)
if distro := config.OperatingSystemDistro; distro != nil && distro.IsSet() {
table.AddRow("OPERATING SYSTEM DISTRIBUTION", *distro.Get())
table.AddSeparator()
}
if version := config.OperatingSystemVersion; version != nil {
table.AddRow("OPERATING SYSTEM VERSION", *version)
if version := config.OperatingSystemVersion; version != nil && version.IsSet() {
table.AddRow("OPERATING SYSTEM VERSION", *version.Get())
table.AddSeparator()
}
if uefi := config.Uefi; uefi != nil {
Expand Down
31 changes: 14 additions & 17 deletions internal/cmd/beta/image/update/update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,21 +53,19 @@ func (ic *imageConfig) isEmpty() bool {
type inputModel struct {
*globalflags.GlobalFlagModel

Id string
Name *string
DiskFormat *string
LocalFilePath *string
Labels *map[string]string
Config *imageConfig
MinDiskSize *int64
MinRam *int64
Protected *bool
Id string
Name *string
DiskFormat *string
Labels *map[string]string
Config *imageConfig
MinDiskSize *int64
MinRam *int64
Protected *bool
}

func (im *inputModel) isEmpty() bool {
return im.Name == nil &&
im.DiskFormat == nil &&
im.LocalFilePath == nil &&
im.Labels == nil &&
(im.Config == nil || im.Config.isEmpty()) &&
im.MinDiskSize == nil &&
Expand All @@ -78,9 +76,8 @@ func (im *inputModel) isEmpty() bool {
const imageIdArg = "IMAGE_ID"

const (
nameFlag = "name"
diskFormatFlag = "disk-format"
localFilePathFlag = "local-file-path"
nameFlag = "name"
diskFormatFlag = "disk-format"

bootMenuFlag = "boot-menu"
cdromBusFlag = "cdrom-bus"
Expand Down Expand Up @@ -167,7 +164,6 @@ func NewCmd(p *print.Printer) *cobra.Command {
func configureFlags(cmd *cobra.Command) {
cmd.Flags().String(nameFlag, "", "The name of the image.")
cmd.Flags().String(diskFormatFlag, "", "The disk format of the image. ")
cmd.Flags().String(localFilePathFlag, "", "The path to the local disk image file.")

cmd.Flags().Bool(bootMenuFlag, false, "Enables the BIOS bootmenu.")
cmd.Flags().String(cdromBusFlag, "", "Sets CDROM bus controller type.")
Expand All @@ -188,6 +184,8 @@ func configureFlags(cmd *cobra.Command) {
cmd.Flags().Int64(minDiskSizeFlag, 0, "Size in Gigabyte.")
cmd.Flags().Int64(minRamFlag, 0, "Size in Megabyte.")
cmd.Flags().Bool(protectedFlag, false, "Protected VM.")

cmd.MarkFlagsRequiredTogether(rescueBusFlag, rescueDeviceFlag)
}

func parseInput(p *print.Printer, cmd *cobra.Command, cliArgs []string) (*inputModel, error) {
Expand All @@ -201,9 +199,8 @@ func parseInput(p *print.Printer, cmd *cobra.Command, cliArgs []string) (*inputM
Id: cliArgs[0],
Name: flags.FlagToStringPointer(p, cmd, nameFlag),

DiskFormat: flags.FlagToStringPointer(p, cmd, diskFormatFlag),
LocalFilePath: flags.FlagToStringPointer(p, cmd, localFilePathFlag),
Labels: flags.FlagToStringToStringPointer(p, cmd, labelsFlag),
DiskFormat: flags.FlagToStringPointer(p, cmd, diskFormatFlag),
Labels: flags.FlagToStringToStringPointer(p, cmd, labelsFlag),
Config: &imageConfig{
BootMenu: flags.FlagToBoolPointer(p, cmd, bootMenuFlag),
CdromBus: flags.FlagToStringPointer(p, cmd, cdromBusFlag),
Expand Down
39 changes: 36 additions & 3 deletions internal/cmd/beta/image/update/update_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ var (
testProjectId = uuid.NewString()

testImageId = []string{uuid.NewString()}
testLocalImagePath = "/does/not/exist"
testDiskFormat = "raw"
testDiskSize int64 = 16 * 1024 * 1024 * 1024
testRamSize int64 = 8 * 1024 * 1024 * 1024
Expand Down Expand Up @@ -54,7 +53,6 @@ func fixtureFlagValues(mods ...func(flagValues map[string]string)) map[string]st

nameFlag: testName,
diskFormatFlag: testDiskFormat,
localFilePathFlag: testLocalImagePath,
bootMenuFlag: strconv.FormatBool(testBootmenu),
cdromBusFlag: testCdRomBus,
diskBusFlag: testDiskBus,
Expand Down Expand Up @@ -95,7 +93,6 @@ func fixtureInputModel(mods ...func(model *inputModel)) *inputModel {
Id: testImageId[0],
Name: &testName,
DiskFormat: &testDiskFormat,
LocalFilePath: &testLocalImagePath,
Labels: utils.Ptr(parseLabels(testLabels)),
Config: &imageConfig{
BootMenu: &testBootmenu,
Expand Down Expand Up @@ -271,6 +268,35 @@ func TestParseInput(t *testing.T) {
args: []string{uuid.NewString(), uuid.NewString()},
isValid: false,
},
{
description: "only rescue bus is invalid",
flagValues: fixtureFlagValues(func(flagValues map[string]string) {
delete(flagValues, rescueDeviceFlag)
}),
args: []string{testImageId[0]},
isValid: false,
},
{
description: "only rescue device is invalid",
flagValues: fixtureFlagValues(func(flagValues map[string]string) {
delete(flagValues, rescueBusFlag)
}),
args: []string{testImageId[0]},
isValid: false,
},
{
description: "no rescue device and no bus is valid",
flagValues: fixtureFlagValues(func(flagValues map[string]string) {
delete(flagValues, rescueBusFlag)
delete(flagValues, rescueDeviceFlag)
}),
isValid: true,
args: []string{testImageId[0]},
expectedModel: fixtureInputModel(func(model *inputModel) {
model.Config.RescueBus = nil
model.Config.RescueDevice = nil
}),
},
}

for _, tt := range tests {
Expand All @@ -297,6 +323,13 @@ func TestParseInput(t *testing.T) {
t.Fatalf("error validating flags: %v", err)
}

if err := cmd.ValidateFlagGroups(); err != nil {
if !tt.isValid {
return
}
t.Fatalf("error validating flag groups: %v", err)
}

if err := cmd.ValidateArgs(tt.args); err != nil {
if !tt.isValid {
return
Expand Down
Loading