Skip to content
This repository has been archived by the owner on Nov 15, 2022. It is now read-only.

Fallback to oc command if kubectl is not found #537

Merged
merged 1 commit into from
Jan 3, 2018

Conversation

cdrage
Copy link
Collaborator

@cdrage cdrage commented Dec 18, 2017

This checks to see if the kubectl and oc commands are available and
fallback to the oc command appropriatley if deploying a container
without kubectl installed on the host system.

Closes #458

@cdrage
Copy link
Collaborator Author

cdrage commented Dec 18, 2017

Not too sure how to make integrations tests for this, but 🤷‍♂️

@cdrage
Copy link
Collaborator Author

cdrage commented Dec 20, 2017

@kadel @surajnarwade @containscafeine review?

Copy link
Member

@kadel kadel left a comment

Choose a reason for hiding this comment

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

I've tested this on Linux, Windows, and MacOS and it correctly worked everywhere.

executable := "kubectl"
if useOC {
executable = "oc"
}

// Use oc if kubectl is unavailable
if executable == "kubectl" {
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't be better to move this block to previous if statement as else?

if useOC {
   executable = "oc"
} else {
if _, err := exec.LookPath("kubectl"); err != nil {
....

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup. I'll change this.


// If oc is used, error out if it's not available
if executable == "oc" {
if _, err := exec.LookPath("oc"); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

This could be also in first if statement.

If you think this is better than let's leave as it is. This is a just small suggestion that might make code shorter and maybe more readable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@kadel tried, but combining both doesn't work especially when assigning new variables 😒

This checks to see if the kubectl and oc commands are available and
fallback to the oc command appropriatley if deploying a container
without kubectl installed on the host system.

Closes kedgeproject#458
@cdrage
Copy link
Collaborator Author

cdrage commented Dec 21, 2017

Thanks @kadel I've made the changes and pushed it again 💯

@pradeepto
Copy link
Member

@containscafeine Can you have a look at this? @kadel is on PTO from tomorrow until Jan 2. I would like to see this in master and in the next release because other teams have asked for this. Thanks.

@surajnarwade
Copy link
Collaborator

Worked for me, LGTM

@kadel kadel merged commit 03f40d5 into kedgeproject:master Jan 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants