-
Notifications
You must be signed in to change notification settings - Fork 1
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.
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
dry-run added #127
base: master
Are you sure you want to change the base?
dry-run added #127
Conversation
pkg/genericcli/cmds.go
Outdated
@@ -182,6 +190,9 @@ func NewCmds[C any, U any, R any](c *CmdsConfig[C, U, R], additionalCmds ...*cob | |||
}, | |||
} | |||
|
|||
//Adding dry-run flas to the command | |||
cmd.Flags().String("dry-run", "", "Ser dry-run mode. Values: client, server") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be a bool flag, there is no need for us to distinguish between client / server.
pkg/genericcli/cmds.go
Outdated
if viper.GetString("dry-run") == "client" { | ||
fmt.Println("Create request as client:", rq) | ||
return nil | ||
} else if viper.GetString("dry-run") == "server" { | ||
fmt.Println("Create request as server:", rq) | ||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably error out when this flag is used in combination with the -f
flag as otherwise we run a request against the API and the --dry-run
flag is ignored, which is kind of dangerous.
Also fmt.Println
cannot be used as this does not produce serialized output, see:
❯ metalctl partition create --name test --description "i am a partition" --imageurl https://foo-image --kernelurl https://kernel-image --cmdline "1 2 3 4 5" --dry-run=client
Create request as client: &{0xc00026e450 i am a partition 0xc000233690 map[] test 0}
The expected result should look something like this:
❯ metalctl partition create --id test --name test --description "i am a partition" --imageurl https://foo-image --kernelurl https://kernel-image --cmdline "1 2 3 4 5" --dry-run=client
---
bootconfig:
commandline: "1 2 3 4 5"
imageurl: https://foo-image
kernelurl: https://kernel-image
changed: "2024-02-12T15:26:25.664Z"
created: "2020-03-11T14:29:12.446Z"
description: i am a partition
mgmtserviceaddress: fra-equ01.bmc-proxy.metal-stack.dev:3333
name: test
id: test
Ideally, you use the DescribePrinter
to achieve this kind of output (in this case a user could also do -o json
to print the manifest in JSON and not YAML).
In general, the output should be able to be used as input as well, like: metalctl partition create --dry-run --id test [...] | metalctl partition create -f -
. So no additional text should be outputted, just valid YAML or JSON.
One of the main problems here is that |
No description provided.