-
Notifications
You must be signed in to change notification settings - Fork 13
Add diagnostic tool selection with optional DTOOL/DTOOLARGS flags in … #306
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
base: master
Are you sure you want to change the base?
Conversation
…diagnostic script
@@ -0,0 +1,6 @@ | |||
--- | |||
title: Add diagnostic tool selection with DTOOL and DTOOLARGS support |
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.
title: Add diagnostic tool selection with DTOOL and DTOOLARGS support | |
title: Add support to select diagnostic tool with DTOOL and DTOOLARGS |
ocExists=$? | ||
|
||
command -v kubectl >/dev/null 2>&1 | ||
kcExists=$? |
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.
kcExists=$? | |
kubectlExists=$? |
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.
nit: I think it's better to be explicit about what exactly we are talking about. wdyt?
kcExists=$? | ||
|
||
# Let the user override via environment variable | ||
dToolUser=${DTOOL:-""} |
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.
kubectl
and oc
are not really the diagnostic tools right? they are just tools to interact with the api server. So I would recommend naming this to something else. Maybe clientTool
, kubeClientTool
or something like that.
@@ -49,7 +89,7 @@ usage() { | |||
contains() { | |||
local e match=$1 | |||
shift | |||
for e; do [[ "${e}" == "${match}" ]] && return 0; done | |||
for e; do [[ "$e" == "$match" ]] && return 0; done |
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.
is this change necessary? this might upset the shellcheck. or the changed version is better suited for shellcheck? 😆
|
||
# Let the user override via environment variable | ||
dToolUser=${DTOOL:-""} | ||
dToolUserArgs=${DTOOLARGS:-""} |
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.
what exactly is the motivation to introduce this? do we want users to be able to configure the kubeconfig?
Summary
This change updates the diagnostic script to support selecting a tool (oc or kubectl) using the DTOOL environment variable and passing additional flags using DTOOLARGS. The script detects which tools are installed and chooses the preferred tool if specified.
Proof of Work
The script was tested in an environment where both oc and kubectl were installed. It was verified that when no preference was set, the script correctly defaulted to oc. It was also tested with DTOOL set to either oc or kubectl, and in both cases the script correctly used the specified tool.
Checklist
skip-changelog
label if not needed