-
Notifications
You must be signed in to change notification settings - Fork 345
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
Skip requirement of kubeconfig for e2e & gen #438
Conversation
Earlier `kubeconfig` was needed to use the sub-commands `e2e` and `gen` which is unnecessary since using these sub-commands does not require communication to Kubernetes server. This commit removes that necessity. Signed-off-by: Suraj Deshmukh <[email protected]>
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.
Just one change then it LGTM
@@ -75,12 +75,10 @@ func e2es(cmd *cobra.Command, args []string) { | |||
os.Exit(1) | |||
} | |||
defer gzr.Close() | |||
restConfig, err := e2eflags.kubecfg.Get() |
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.
This might be a slightly deeper change than I originally expected. Since e2e does many things (oops) it will need the config later on. Would you be able to move this down below if !e2eflags.rerun {
this line?
There is a flag, which needs rethinking, --rerun
which allows users to rerun the failed tests if an error is detected.
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.
done, rather than moving it down I have added a check.
Moving down was causing the initiation of client twice, so added a check rather.
The special case/flag of `e2e` `--rerun-failed` needs kubeconfig, this commit adds a check to see if this is enabled then it creates the sonobuoy client with config, otherwise without it. Signed-off-by: Suraj Deshmukh <[email protected]>
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.
Awesome, this lgtm
* Skip requirement of kubeconfig for e2e & gen Earlier `kubeconfig` was needed to use the sub-commands `e2e` and `gen` which is unnecessary since using these sub-commands does not require communication to Kubernetes server. This commit removes that necessity. Signed-off-by: Suraj Deshmukh <[email protected]> * With `e2e` subcommand, kubeconfig needed The special case/flag of `e2e` `--rerun-failed` needs kubeconfig, this commit adds a check to see if this is enabled then it creates the sonobuoy client with config, otherwise without it. Signed-off-by: Suraj Deshmukh <[email protected]> Signed-off-by: Jesse Hamilton [email protected]
* Skip requirement of kubeconfig for e2e & gen Earlier `kubeconfig` was needed to use the sub-commands `e2e` and `gen` which is unnecessary since using these sub-commands does not require communication to Kubernetes server. This commit removes that necessity. Signed-off-by: Suraj Deshmukh <[email protected]> * With `e2e` subcommand, kubeconfig needed The special case/flag of `e2e` `--rerun-failed` needs kubeconfig, this commit adds a check to see if this is enabled then it creates the sonobuoy client with config, otherwise without it. Signed-off-by: Suraj Deshmukh <[email protected]> Signed-off-by: Jesse Hamilton [email protected] Signed-off-by: Jesse Hamilton [email protected]
What this PR does / why we need it
Earlier
kubeconfig
was needed to use the sub-commandse2e
andgen
which is unnecessary since using these sub-commands does not require
communication to Kubernetes server.
This commit removes that necessity.
Which issue(s) this PR fixes
Signed-off-by: Suraj Deshmukh [email protected]