-
Notifications
You must be signed in to change notification settings - Fork 40
Fallback to oc command if kubectl is not found #537
Conversation
Not too sure how to make integrations tests for this, but 🤷♂️ |
@kadel @surajnarwade @containscafeine review? |
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've tested this on Linux, Windows, and MacOS and it correctly worked everywhere.
pkg/cmd/commands.go
Outdated
executable := "kubectl" | ||
if useOC { | ||
executable = "oc" | ||
} | ||
|
||
// Use oc if kubectl is unavailable | ||
if executable == "kubectl" { |
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.
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 {
....
?
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.
Yup. I'll change this.
pkg/cmd/commands.go
Outdated
|
||
// If oc is used, error out if it's not available | ||
if executable == "oc" { | ||
if _, err := exec.LookPath("oc"); err != nil { |
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 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.
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.
@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
956c64a
to
1377657
Compare
Thanks @kadel I've made the changes and pushed it again 💯 |
@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. |
Worked for me, LGTM |
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