Skip to content
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

Add kubectl Check for ko delete, apply, and create #120

Merged
merged 1 commit into from
Jan 16, 2020

Conversation

danielhelfand
Copy link
Contributor

@danielhelfand danielhelfand commented Jan 16, 2020

This pull request adds an error message in case kubectl is not installed. I'm assuming most users of ko will have kubectl installed, but it's nice to have a clear message in the edge case when it's not available.

@@ -46,6 +52,15 @@ func passthru(command string) runCmd {
}
}

// check if kubectl is installed
func isKubectlAvailable() bool {
cmd := exec.Command("kubectl")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use exec.LookPath for this, and we probably want to do this for apply, create, and run as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonjohnsonjr Awesome! I can definitely use LookPath instead. Thanks for the better suggestion. Would you recommend a good place to define isKubectlAvailable() so I can use it across all commands?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anywhere is fine! I'd just leave it here, probably.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put in commands.go for now, but more than willing to move it back to any of the commands that use it.

Copy link
Collaborator

@jonjohnsonjr jonjohnsonjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@jonjohnsonjr jonjohnsonjr merged commit 2e28671 into ko-build:master Jan 16, 2020
@danielhelfand danielhelfand changed the title Add kubectl Check for ko delete Add kubectl Check for ko delete, apply, and create Jan 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants