-
Notifications
You must be signed in to change notification settings - Fork 489
Add support for windows shells and bash #1749
Conversation
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'd like to see this being able to explicitly set it when the shell is known. For example, in the case of an ephemeral debugging image, we know what shell will be available.
I'm going to think about how and what about this we might want to be testing. |
Current plan is to reduce the space in half by finding out if it is a windows container based on the presents of a runtimeClass, tolerations, or a nodeSelector in the pod spec (checking volume mounts for a path seems redundant). There doesn't seem to be a standard way to tell the default shell in a container otherwise. In other words, octant still has to guess if a given linux container uses sh, bash, something else, or none of them. |
765afb6
to
f4cde01
Compare
Rebased and updated to check for windows labels before attempting any of the windows shells. Per https://kubernetes.io/docs/setup/production-environment/windows/intro-windows-in-kubernetes/ , |
Signed-off-by: GuessWhoSamFoo <foos@vmware.com>
f4cde01
to
4dbb210
Compare
To Whom It May Concern: Consider upgrading to Octant v0.19.0 if you're using Octant v0.17.0 and v0.18.0. Why: Feature Change default shell to bash #1533's initial implementation, Add support for windows shells and bash #1749 released in v0.17.0 and v0.18.0, has a bug of:
with operating systems without A bug fix increase timeout for remote cmd stream setup #2258 released in v0.19.0, fixed a similar bug No option to choose the interpreter on the terminal #2164. After the bug fix, operating systems without Thank you. |
What this PR does / why we need it:
Iterate through common shells to give terminal instances a better chance of running.
Which issue(s) this PR fixes
Signed-off-by: GuessWhoSamFoo foos@vmware.com