Skip to content

Commit c5b3de6

Browse files
bahkauv70bahkauv70
authored
Fixes for image sub commands (#564)
* fix: output of describe command * fix: require rescue bus and device as pair, remove unsupported local-image-path from update command, fix describe for undefined fields * fix: added testcases for flag groups
1 parent 3d5a916 commit c5b3de6

File tree

5 files changed

+89
-25
lines changed

5 files changed

+89
-25
lines changed

‎internal/cmd/beta/image/create/create.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,7 @@ func configureFlags(cmd *cobra.Command) {
283283
if err := flags.MarkFlagsRequired(cmd, nameFlag, diskFormatFlag, localFilePathFlag); err != nil {
284284
cobra.CheckErr(err)
285285
}
286+
cmd.MarkFlagsRequiredTogether(rescueBusFlag, rescueDeviceFlag)
286287
}
287288

288289
func parseInput(p *print.Printer, cmd *cobra.Command) (*inputModel, error) {

‎internal/cmd/beta/image/create/create_test.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,32 @@ func TestParseInput(t *testing.T) {
233233
}
234234
}),
235235
},
236+
{
237+
description: "only rescue bus is invalid",
238+
flagValues: fixtureFlagValues(func(flagValues map[string]string) {
239+
delete(flagValues, rescueDeviceFlag)
240+
}),
241+
isValid: false,
242+
},
243+
{
244+
description: "only rescue device is invalid",
245+
flagValues: fixtureFlagValues(func(flagValues map[string]string) {
246+
delete(flagValues, rescueBusFlag)
247+
}),
248+
isValid: false,
249+
},
250+
{
251+
description: "no rescue device and no bus is valid",
252+
flagValues: fixtureFlagValues(func(flagValues map[string]string) {
253+
delete(flagValues, rescueBusFlag)
254+
delete(flagValues, rescueDeviceFlag)
255+
}),
256+
isValid: true,
257+
expectedModel: fixtureInputModel(func(model *inputModel) {
258+
model.Config.RescueBus = nil
259+
model.Config.RescueDevice = nil
260+
}),
261+
},
236262
}
237263

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

282+
if err := cmd.ValidateFlagGroups(); err != nil {
283+
if !tt.isValid {
284+
return
285+
}
286+
t.Fatalf("error validating flag groups: %v", err)
287+
}
288+
256289
if err := cmd.ValidateRequiredFlags(); err != nil {
257290
if !tt.isValid {
258291
return

‎internal/cmd/beta/image/describe/describe.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ func outputResult(p *print.Printer, model *inputModel, resp *iaas.Image) error {
106106

107107
return nil
108108
case print.YAMLOutputFormat:
109-
details, err := yaml.MarshalWithOptions(resp, yaml.IndentSequence(true))
109+
details, err := yaml.MarshalWithOptions(resp, yaml.IndentSequence(true), yaml.UseJSONMarshaler())
110110
if err != nil {
111111
return fmt.Errorf("marshal image: %w", err)
112112
}
@@ -141,12 +141,12 @@ func outputResult(p *print.Printer, model *inputModel, resp *iaas.Image) error {
141141
table.AddRow("OPERATING SYSTEM", *os)
142142
table.AddSeparator()
143143
}
144-
if distro := config.OperatingSystemDistro; distro != nil {
145-
table.AddRow("OPERATING SYSTEM DISTRIBUTION", *distro)
144+
if distro := config.OperatingSystemDistro; distro != nil && distro.IsSet() {
145+
table.AddRow("OPERATING SYSTEM DISTRIBUTION", *distro.Get())
146146
table.AddSeparator()
147147
}
148-
if version := config.OperatingSystemVersion; version != nil {
149-
table.AddRow("OPERATING SYSTEM VERSION", *version)
148+
if version := config.OperatingSystemVersion; version != nil && version.IsSet() {
149+
table.AddRow("OPERATING SYSTEM VERSION", *version.Get())
150150
table.AddSeparator()
151151
}
152152
if uefi := config.Uefi; uefi != nil {

‎internal/cmd/beta/image/update/update.go

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -53,21 +53,19 @@ func (ic *imageConfig) isEmpty() bool {
5353
type inputModel struct {
5454
*globalflags.GlobalFlagModel
5555

56-
Id string
57-
Name *string
58-
DiskFormat *string
59-
LocalFilePath *string
60-
Labels *map[string]string
61-
Config *imageConfig
62-
MinDiskSize *int64
63-
MinRam *int64
64-
Protected *bool
56+
Id string
57+
Name *string
58+
DiskFormat *string
59+
Labels *map[string]string
60+
Config *imageConfig
61+
MinDiskSize *int64
62+
MinRam *int64
63+
Protected *bool
6564
}
6665

6766
func (im *inputModel) isEmpty() bool {
6867
return im.Name == nil &&
6968
im.DiskFormat == nil &&
70-
im.LocalFilePath == nil &&
7169
im.Labels == nil &&
7270
(im.Config == nil || im.Config.isEmpty()) &&
7371
im.MinDiskSize == nil &&
@@ -78,9 +76,8 @@ func (im *inputModel) isEmpty() bool {
7876
const imageIdArg = "IMAGE_ID"
7977

8078
const (
81-
nameFlag = "name"
82-
diskFormatFlag = "disk-format"
83-
localFilePathFlag = "local-file-path"
79+
nameFlag = "name"
80+
diskFormatFlag = "disk-format"
8481

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

172168
cmd.Flags().Bool(bootMenuFlag, false, "Enables the BIOS bootmenu.")
173169
cmd.Flags().String(cdromBusFlag, "", "Sets CDROM bus controller type.")
@@ -188,6 +184,8 @@ func configureFlags(cmd *cobra.Command) {
188184
cmd.Flags().Int64(minDiskSizeFlag, 0, "Size in Gigabyte.")
189185
cmd.Flags().Int64(minRamFlag, 0, "Size in Megabyte.")
190186
cmd.Flags().Bool(protectedFlag, false, "Protected VM.")
187+
188+
cmd.MarkFlagsRequiredTogether(rescueBusFlag, rescueDeviceFlag)
191189
}
192190

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

204-
DiskFormat: flags.FlagToStringPointer(p, cmd, diskFormatFlag),
205-
LocalFilePath: flags.FlagToStringPointer(p, cmd, localFilePathFlag),
206-
Labels: flags.FlagToStringToStringPointer(p, cmd, labelsFlag),
202+
DiskFormat: flags.FlagToStringPointer(p, cmd, diskFormatFlag),
203+
Labels: flags.FlagToStringToStringPointer(p, cmd, labelsFlag),
207204
Config: &imageConfig{
208205
BootMenu: flags.FlagToBoolPointer(p, cmd, bootMenuFlag),
209206
CdromBus: flags.FlagToStringPointer(p, cmd, cdromBusFlag),

‎internal/cmd/beta/image/update/update_test.go

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ var (
2626
testProjectId = uuid.NewString()
2727

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

5554
nameFlag: testName,
5655
diskFormatFlag: testDiskFormat,
57-
localFilePathFlag: testLocalImagePath,
5856
bootMenuFlag: strconv.FormatBool(testBootmenu),
5957
cdromBusFlag: testCdRomBus,
6058
diskBusFlag: testDiskBus,
@@ -95,7 +93,6 @@ func fixtureInputModel(mods ...func(model *inputModel)) *inputModel {
9593
Id: testImageId[0],
9694
Name: &testName,
9795
DiskFormat: &testDiskFormat,
98-
LocalFilePath: &testLocalImagePath,
9996
Labels: utils.Ptr(parseLabels(testLabels)),
10097
Config: &imageConfig{
10198
BootMenu: &testBootmenu,
@@ -271,6 +268,35 @@ func TestParseInput(t *testing.T) {
271268
args: []string{uuid.NewString(), uuid.NewString()},
272269
isValid: false,
273270
},
271+
{
272+
description: "only rescue bus is invalid",
273+
flagValues: fixtureFlagValues(func(flagValues map[string]string) {
274+
delete(flagValues, rescueDeviceFlag)
275+
}),
276+
args: []string{testImageId[0]},
277+
isValid: false,
278+
},
279+
{
280+
description: "only rescue device is invalid",
281+
flagValues: fixtureFlagValues(func(flagValues map[string]string) {
282+
delete(flagValues, rescueBusFlag)
283+
}),
284+
args: []string{testImageId[0]},
285+
isValid: false,
286+
},
287+
{
288+
description: "no rescue device and no bus is valid",
289+
flagValues: fixtureFlagValues(func(flagValues map[string]string) {
290+
delete(flagValues, rescueBusFlag)
291+
delete(flagValues, rescueDeviceFlag)
292+
}),
293+
isValid: true,
294+
args: []string{testImageId[0]},
295+
expectedModel: fixtureInputModel(func(model *inputModel) {
296+
model.Config.RescueBus = nil
297+
model.Config.RescueDevice = nil
298+
}),
299+
},
274300
}
275301

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

326+
if err := cmd.ValidateFlagGroups(); err != nil {
327+
if !tt.isValid {
328+
return
329+
}
330+
t.Fatalf("error validating flag groups: %v", err)
331+
}
332+
300333
if err := cmd.ValidateArgs(tt.args); err != nil {
301334
if !tt.isValid {
302335
return

0 commit comments

Comments
 (0)