-
Notifications
You must be signed in to change notification settings - Fork 10
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
[QG][KIM] Application that generates loads by creating or deleting Runtime CR #421
Conversation
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.
Please check my comments
hack/performance/cmd/cmd.go
Outdated
switch os.Args[1] { | ||
case "create": | ||
createCmd.Parse(os.Args[2:]) | ||
if *loadID == "" || *namePrefix == "" || *kubeconfig == "" || *rtNumber == 0 { |
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.
The flag templatePath is also required and should be also checked here
hack/performance/cmd/cmd.go
Outdated
kubeconfig := createCmd.String("kubeconfig", "", "the path to the kubeconfig file (required)") | ||
rtNumber := createCmd.Int("rt-number", 0, "the number of the runtimes to be created (required)") | ||
templatePath := createCmd.String("rt-template", "", "the path to the yaml file with the runtime template (required)") | ||
runOnCi := createCmd.String("run-on-ci", "false", "identifies if the load is running on CI") |
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.
I suggest to change type of this flag to Bool if possible - this will avoid problem in line 70
if a user provides anything different than "false"
return Unknown, nil, err | ||
} | ||
} | ||
|
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.
if a user provides anything different than "false" ex "FALSE" or "aaaaa" the code under if statement will not be executed.
I suggest:
- use flag.Bool type for this parameter runOnCi
- Change this code into:
if *runOnCi == false {
}
} | ||
|
||
func readFromSource(reader io.Reader) (imv1.Runtime, error) { | ||
data, err := io.ReadAll(reader) |
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.
The error value is not handled
Co-authored-by: Grzegorz Karaluch <grzegorz.karaluch@sap.com>
Co-authored-by: Grzegorz Karaluch <grzegorz.karaluch@sap.com>
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.
Almost ready just one small suggestion
Co-authored-by: Przemyslaw Golicz <przemyslaw.golicz@sap.com>
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.
LGTM
Description
The program is used to generate performance test loads by creating or deleting Runtime CR in a given Kubernetes cluster.
Changes proposed in this pull request:
Related issue(s)
#14