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.

Sign up for GitHub

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 --context support #1

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add --context support #1

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions bin/kubectl-forward
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ describe() {
help() {
echo -e "
Usage:
forward [-n namespace] host [sport:]dport
forward [--context context] [-n namespace] host [sport:]dport
Copy link
Owner

Choose a reason for hiding this comment

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

Let's keep this code consistent and use short options. I would suggest -c context here.

Copy link
Author

@levrik levrik Feb 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Just wanted to be in line with other built-in kubectl commands here. There's just --context available.

Copy link
Owner

Choose a reason for hiding this comment

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

I understand, nevertheless using short options is cleaner here imho. We already do it for the namespace which is also supported natively by the kubectl. So, Iwould either:

  • change the context to just c to stay consistent with the current style,
  • or remove it completely in order to allow users to pass any kubectl built-in options, but without creating an explicits support for them here. In this scenario I would leave a note in readme to let users know they could find these options with the kubectl options.

Copy link
Author

@levrik levrik Feb 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I see. But it's a bit different with --namespace as kubectl also supports -n as alias for it while -c isn't an alias for --context but is used to specify a config file.
The latter probably would be the approach here but I don't have time to work on this right now.

Copy link
Owner

Choose a reason for hiding this comment

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

Sure, no worries, there is no rush, feel free to come back to it when you have more time.

Copy link
Author

Choose a reason for hiding this comment

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

I'll keep using my fork for now.


Examples:
# Forward TCP traffic to the RDS host from your local 5432 port.
Expand All @@ -27,15 +27,25 @@ Examples:
exit 1
}

context=$(kubectl config current-context)
namespace="default"
while getopts "hn:" opt; do
while getopts "hn:-:" opt; do
case "$opt" in
h)
describe
help
;;
n) namespace="$OPTARG"
;;
-)
Copy link
Owner

Choose a reason for hiding this comment

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

Same as above, let's keep it consistent and simplify by using short option.

case "${OPTARG}" in
context) context="${!OPTIND}"
OPTIND=$((OPTIND+1))
;;
*) OPTIND=$((OPTIND+1))
;;
esac
;;
esac
done
shift $((OPTIND-1))
Expand All @@ -57,13 +67,13 @@ fi
suffix=$(openssl rand -hex 3)

cleanup() {
kubectl delete pod -n $namespace forwarder-$suffix 2>&1 >/dev/null
kubectl wait --for=delete pod/forwarder-$suffix 2>&1 >/dev/null
kubectl delete pod --context $context -n $namespace forwarder-$suffix 2>&1 >/dev/null
kubectl wait --context $context -n $namespace --for=delete pod/forwarder-$suffix 2>&1 >/dev/null
}

trap cleanup SIGINT

cat <<HERE | kubectl -n $namespace create -f 2>&1 >/dev/null -
cat <<HERE | kubectl --context $context -n $namespace create -f 2>&1 >/dev/null -
apiVersion: v1
kind: Pod
metadata:
Expand All @@ -80,5 +90,5 @@ spec:
- TCP:${host}:${dport}
HERE

kubectl wait -n $namespace --for=condition=Ready pod/forwarder-$suffix 2>&1 >/dev/null
kubectl port-forward -n $namespace pod/forwarder-$suffix $sport:$dport
kubectl wait --context $context -n $namespace --for=condition=Ready pod/forwarder-$suffix 2>&1 >/dev/null
kubectl port-forward --context $context -n $namespace pod/forwarder-$suffix $sport:$dport