[Feat][kubectl-plugin] Add instructions for static shell completion #2384
Conversation
|
@kevin85421 @andrewsykim PTAL |
|
cc @chiayi |
|
|
||
| ### Retrieve Ray Cluster Head Logs | ||
| 1. Install [Krew](https://krew.sigs.k8s.io/docs/user-guide/setup/install/). | ||
| 2. (TODO: Replace this step with the installation command). |
There was a problem hiding this comment.
Do we plan to add this instruction before merging this PR?
There was a problem hiding this comment.
I am considering distributing our kubectl plugin with krew, but this requires we to
- Package our plugin and then put it on GitHub release page
- Open a pull request to krew index github repo
So I think it's better to merge this PR so that we can implement dynamic completion of nouns.
See https://krew.sigs.k8s.io/docs/developer-guide/distributing-with-krew/ for details.
There was a problem hiding this comment.
I like the idea of using Krew, but until it's published we should still providing working solution here to install the plugin even if that involves manually compiling it. What do you think?
There was a problem hiding this comment.
There is an offline slack discussion here:
https://ray.slack.com/archives/C07M825T94Z/p1726476910442009
The message is:
I just found this repository, which can automatically release our plugin to Krew. However, it relies on git release tags. Currently KubeRay is a monorepo, with all Kubernetes-related items stored in it. So, I'm thinking that the tag format might need to be something like kubectl-ray/v0.1.0, and the versioning might need to be separate from the ray-operator version. I'm just not sure if this will look odd because I noticed that the current tags match the release tags, such as ray-operator/v1.2.1 and v1.2.1. Does anyone have any thoughts on this?
https://github.com/ray-project/kuberay/tags
There was a problem hiding this comment.
I moved the manually installation guide to Development section. Do you think its better to rename it to "Compile from source"?
There was a problem hiding this comment.
Also before the PR to Krew index repo is merged, we can tell the users to install from our manifest file like this:
kubectl krew install --manifest-url <url>
But we still need to package the binary and put it on GitHub release page.
There was a problem hiding this comment.
I moved the manually installation guide to Development section. Do you think its better to rename it to "Compile from source"?
I think it should still be at the top under "Installation" until the installation with krew is simpler.
Also, once we have a released binary of the we should also provide installation steps without krew:
curl -LO <binary-releases-url>
mv ./kubectl-ray $GOBIN
Or something like that
There was a problem hiding this comment.
Make senses to me. Updated.
Signed-off-by: Chi-Sheng Liu <chishengliu@chishengliu.com>
MortalHappiness
force-pushed
the
feature/kubectl-shell-completion
branch
from
596dc2e
to
85e5aff
Compare
| Aliases: | ||
| log, logs | ||
| 1. Install [Krew](https://krew.sigs.k8s.io/docs/user-guide/setup/install/). | ||
| 2. (TODO: Replace this step with the installation command). |
There was a problem hiding this comment.
To clarify, this TODO will be blocked until we release KubeRay v1.3 which includes the binray release of the plugin right?
There was a problem hiding this comment.
I mentioned a discussion above here
I don't know whether we want to use the same version for ray-operator and kubectl-plugin or not. If we want to use different versioning for them, then this will block untill to kubectl-plugin v0.1.0 release. If we want to use the same version, then yes, this will block until the release of KubeRay v1.3.0.
cc @kevin85421
There was a problem hiding this comment.
ah gotcha, that makes sense to me
There was a problem hiding this comment.
Personally I think we should keep the releases together but I'm also fine to do it separately. Will defer to you and @chiayi
There was a problem hiding this comment.
Personally I think we should keep the releases together but I'm also fine to do it separately.
I also prefer to keep the releases together to simplify the release process, unless there is a strong benefit to separating them.
Why are these changes needed?
This PR updates README.md to add instruction for static shell completion for kubectl ray plugin. Static means that we haven't implemented Dynamic completion of nouns yet.
Also the default
completioncommand incobrais disabled because the shell completion forkubectlplugin works differently from normal commands. Therefore this command is useless. I removed it to avoid confusion. See kubernetes/kubernetes#74178 and kubernetes/kubernetes#105867 for details.Screenshot (my shell is fish shell):
Related issue number
N/A
Checks