-
Notifications
You must be signed in to change notification settings - Fork 14
will not exit when no kubeconfig found #581
Conversation
/cherrypick release-2019.6.x |
@yashbhutwala: once the present PR merges, I will cherry-pick it on top of release-2019.6.x in a new PR and assign it to you. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
pkg/synopsysctl/cmd_root.go
Outdated
@@ -61,8 +61,7 @@ var rootCmd = &cobra.Command{ | |||
} | |||
// Sets kubeconfig and initializes resource client libraries | |||
if err := setResourceClients(); err != nil { | |||
log.Error(err) | |||
os.Exit(1) | |||
log.Warnf("resource clients were not set properly, %v", 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.
I don't think we want to log anything in the case of the native command. Printing even a warning would seem confusing imo. May need to take most of PreRunE and place in a function that only non-native commands would use unless there is a better way to have the native sub-commands not use this PreRunE (which I'm not sure there is)
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.
There is a hack where I can have a PersistentRunE
for a subcommand that will override the root command, but it's being fixed in cobra right now. See issue 252
@yashbhutwala @rrati I think we want to exit by default if there is no kube config file... just not when using the native command. |
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
/cherrypick release-2019.6.x
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jdartigalongue, rrati, yashbhutwala The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/cherrypick release-2019.6.x |
@rrati: once the present PR merges, I will cherry-pick it on top of release-2019.6.x in a new PR and assign it to you. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@yashbhutwala: new pull request created: #591 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/closes #547
also related to issue #258 and PR #425
@jdartigalongue @rrati