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.

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

bash completions #563

Merged
merged 4 commits into from
Jan 23, 2025
Merged

bash completions #563

merged 4 commits into from
Jan 23, 2025

Conversation

martinetd
Copy link
Collaborator

Just clush for now; might be some bugs but seems to work.

perf-wise it calls cluset -L on every tab, we might need to cache the result somewhere if it is slow in some setups.
(But if user changes conf files it won't be reloaded so that's a bit annoying, it's better not to cache if it's reasonable e.g. just reading the conf files)

Prereq patch makes cluset -s foo -L not list foo groups with the @foo: prefix, like default source with -L -- this allows clush -s foo -w @bar to complete properly.

bash_completion.d/clush Outdated Show resolved Hide resolved
thiell added a commit to thiell/clustershell that referenced this pull request Sep 9, 2024
Let -s source behave as if it replaces the default group source, and
thus omit the source prefix for the groups in that source.

Part of cea-hpc#563.
github-merge-queue bot pushed a commit that referenced this pull request Sep 14, 2024
Let -s source behave as if it replaces the default group source, and
thus omit the source prefix for the groups in that source.

Part of #563.
@thiell
Copy link
Collaborator

thiell commented Jan 19, 2025

Hi @martinetd and thanks for this!

I would like to include clush bash completion is 1.9.3 if possible, so I've been looking at this today.
I confirm it works however there is one problem I noticed when testing on our cluster Sherlock: -L is way too costly. There, a sysadmin you know defined 32 group sources :-), including various external group sources. Calling cluset -L resolves all groups in all sources and takes up to several minutes to complete.

But I think we might be able to come up with something to only call cluset -l when the user presses TAB (and never -L). But I'm not 100% sure this is doable technically and need your help. Let me explain below.

The idea would be as follow:

  • clush -s source -w <TAB>
    Similar to what you propose but I just added -G so that the group source is not repeated (open for discussion):
options="$(cluset -s "$groupsource" -l -G 2>/dev/null)"  

This gives us this on our cluster:

# clush -s pdu -w <TAB>
# clush -s pdu -w @PX
@PX2-1901U-N1A6  @PX3-1901U-N1    @PX3-1901U-N1A6  @PX4-5851-E7V2 
  • Now, for -w when -s source is not specified, the idea would be to return groups in the default group sources + "@source:" prefixes for the matching group sources. Something like that:
# clush -w <TAB>
@base           @cpu            @hw:            @island:    ....

Here @base and @cpu are normal groups in the default group source, and @hw: and @island: are group sources.

I got this result from this:

    -w|-x)                                                                      
        if [[ -n "$groupsource" ]]; then                                        
            options="$(cluset -s "$groupsource" -l -G 2>/dev/null)"             
        else                                                                    
            options="$(cluset --groupsources | sed -e 's/ (default)//' -e 's/^/@/' -e 's/$/:/')"
            options+="$(cluset -l 2>/dev/null)"                                 
        fi                                                                      
        ;;  

However, I'm not sure how to trigger a group lookup when we do clush -w @hw:<TAB>. First, bash completion adds a space if I hit TAB after @hw: (that's the first problem) and second, we likely need to call cluset -s hw -l -G to list all groups in the hw source in that case. I'm not sure how to do that. If you have any ideas on how to resolve this... let me know. Should we define another completion function and use complete again?

@thiell thiell added this to the 1.9.3 milestone Jan 19, 2025
thiell added a commit to thiell/clustershell that referenced this pull request Jan 20, 2025
Special command for bash completion that lists group sources, groups in
current source and nodes from groups passed as argument:

cluset --completion [-s source] [groups]

Example:

cluset --completion $*
@martinetd martinetd force-pushed the completion branch 2 times, most recently from 276550c to 2f273dc Compare January 21, 2025 12:44
@thiell thiell self-requested a review January 21, 2025 18:53
bash_completion.d/cluset Outdated Show resolved Hide resolved
Copy link
Collaborator

@thiell thiell left a comment

Choose a reason for hiding this comment

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

Also investigating a unexpected blank result in some cases

bash_completion.d/clush Outdated Show resolved Hide resolved
thiell added a commit to martinetd/clustershell that referenced this pull request Jan 22, 2025
Special command for bash completion that lists group sources, groups in
current source and nodes from groups passed as argument:

cluset --completion [-s source] [groups]

Example:

cluset --completion $*

Part of cea-hpc#563.
thiell pushed a commit to martinetd/clustershell that referenced this pull request Jan 22, 2025
Provide bash completion scripts for clush and cluset/nodeset to
autocomplete group sources, groups and "all" nodes from the
default or selected source.

Part of cea-hpc#563.
thiell added a commit to martinetd/clustershell that referenced this pull request Jan 22, 2025
Special command for bash completion that lists group sources, groups in
current source and nodes from groups passed as argument:

cluset --completion [-s source] [groups]

Example:

cluset --completion @*

Part of cea-hpc#563.
thiell pushed a commit to martinetd/clustershell that referenced this pull request Jan 22, 2025
Provide bash completion scripts for clush and cluset/nodeset to
autocomplete group sources, groups and "all" nodes from the
default or selected source.

Part of cea-hpc#563.
thiell added a commit to martinetd/clustershell that referenced this pull request Jan 22, 2025
thiell added a commit to martinetd/clustershell that referenced this pull request Jan 22, 2025
Because it conflicts with xCAT's nodeset command.

Part of cea-hpc#563.
thiell added a commit to martinetd/clustershell that referenced this pull request Jan 22, 2025
thiell added a commit to martinetd/clustershell that referenced this pull request Jan 22, 2025
@thiell thiell requested review from thiell and removed request for thiell January 22, 2025 05:50
thiell added a commit to martinetd/clustershell that referenced this pull request Jan 22, 2025
thiell added a commit to martinetd/clustershell that referenced this pull request Jan 22, 2025
Because it conflicts with xCAT's nodeset command.

Part of cea-hpc#563.
thiell added a commit to martinetd/clustershell that referenced this pull request Jan 22, 2025
thiell added a commit to martinetd/clustershell that referenced this pull request Jan 22, 2025
Copy link
Collaborator

@degremont degremont left a comment

Choose a reason for hiding this comment

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

Overall, this is a very nice work! I'm just not convinced by the missing completion for nodeset

bash_completion.d/clush Show resolved Hide resolved
thiell and others added 3 commits January 22, 2025 21:55
Provide bash completion scripts for clush and cluset/nodeset to
autocomplete group sources, groups and "all" nodes from the
default or selected source.

Part of cea-hpc#563.
@thiell thiell marked this pull request as ready for review January 23, 2025 06:10
@thiell thiell added this pull request to the merge queue Jan 23, 2025
Merged via the queue into cea-hpc:master with commit 45a2357 Jan 23, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants