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

Auto-completion, help (?), cmd navigation (up arrow) not working in vtysh on host system. #1124

Merged
merged 18 commits into from
Nov 13, 2017

Conversation

nikos-github
Copy link
Collaborator

@nikos-github nikos-github commented Nov 7, 2017

Auto-completion, help (?), cmd navigation (up/down arrows) not working in vtysh on host system.
This fix complements sonic-net/sonic-quagga#19

Nikos Triantafillis and others added 17 commits August 22, 2017 13:09
RB=
G=lnos-reviewers
R=ntrianta,rjonnadu,rmolina,sfardeen,zxu
A=
This reverts commit c9a2c92.
Add config support for nhopself, keepalive and holdtime timers.
Add route-map to prefer global nexthops for ebgp learned prefixes.
…ling to a more relevant place for this cmd.

Add config support for nhopself, keepalive and holdtime timers.
Add route-map to prefer global nexthops for ebgp learned prefixes.
…ling to a more relevant place for this cmd.

Add config support for nhopself, keepalive and holdtime timers.
Add route-map to prefer global nexthops for ebgp learned prefixes.
…tysh on host system.

RB=
G=lnos-reviewers
R=ntrianta,rjonnadu,rmolina,sfardeen,zxu
A=
@lguohan
Copy link
Collaborator

lguohan commented Nov 7, 2017

why are there so many commits in your PR? are you synced with the master to generate the PR?

@nikos-github
Copy link
Collaborator Author

nikos-github commented Nov 7, 2017

This is the commit and sync history of my private branch. I don't think it affect Azure/sonic-buildimage. My private branch is synced to latest master the way github does it. I then generate the PR on the delta.

@lguohan
Copy link
Collaborator

lguohan commented Nov 7, 2017

it causes issue in the merge messages, the commit message will by default includes all previous commit messages, we have to manually edited it.

@nikos-github
Copy link
Collaborator Author

I'm just following regular github process. If you have a suggestion that omits that without squashing the history of my private branch or deleting it, let me know.

@lguohan
Copy link
Collaborator

lguohan commented Nov 7, 2017

https://help.github.com/articles/creating-a-pull-request-from-a-fork/

We recommend that you make changes in a topic branch, so that you can push followup commits if you receive feedback on your pull request.

first, you make yout topic branch exactly the same as the master branch, then cherry-pick your commits and finally generate PR.

@nikos-github
Copy link
Collaborator Author

My topic branch/fork is long lived. I sync to the latest master each time, commit to it and I generate the PR as the above link suggests. That's how it picks up the delta of the diffs and the comparison is across the fork and the master.

@lguohan
Copy link
Collaborator

lguohan commented Nov 7, 2017

as time goes, you will have more and more commits in your PR. We cannot really know which commits is the one you want to merge.

@nikos-github
Copy link
Collaborator Author

Of course you do - you have the diffs of what I want to be merged. The PR generates always the delta. And on my side I want the pull requests directly on my forks not on topic branches. This is a valid way to work on github.

@lguohan
Copy link
Collaborator

lguohan commented Nov 7, 2017

https://gist.github.com/Chaser324/ce0505fbed06b947d962

I think we can discuss the workflow offline.

back to the PR, I tried this, vtysh will give me (END), I need to manually type 'q' to make the (END) go away. Am I missing anything here?

@nikos-github
Copy link
Collaborator Author

Cmds work fine for me. What exactly are you trying out?

@lguohan
Copy link
Collaborator

lguohan commented Nov 7, 2017

@nikos-github
Copy link
Collaborator Author

nikos-github commented Nov 7, 2017

In files/image_config/environment/environment the vtysh pager is set to more. Sounds like the quagga docker needs more fixing in addition to this one. On our side with frr, the pager is set properly to more and all commands work fine including the sonic show cli as well as everything inside vtysh.

admin@lnos-x1-a-asw01:~$ set|grep PAGER
VTYSH_PAGER=more

What's the VTYSH_PAGER env variable set to in your case?

@lguohan
Copy link
Collaborator

lguohan commented Nov 7, 2017

I think in this case, the vtysh is looking for the env inside the docker. If frr is working for you, then, can you limit this PR to only FRR? With -t, the vtysh is not working for us.

@rodnymolina
Copy link
Contributor

Quagga's issue can be easily fixed by editing host's vtysh:

docker exec -it bgp bash -c "export VTYSH_PAGER=more; vtysh "$@""

FRR deals with this issue by explicitly adding VTYSH_PAGER as env-var in /etc/environment:

<-- frr/debian/frr.postinst

ENVIRONMENTFILE=/etc/environment
if ! grep --quiet VTYSH_PAGER=/bin/cat ${ENVIRONMENTFILE}; then
echo "VTYSH_PAGER=/bin/cat" >> ${ENVIRONMENTFILE}
fi

@rodnymolina
Copy link
Contributor

Above fix needs more work as it breaks click cli.

@nikos-github
Copy link
Collaborator Author

nikos-github commented Nov 7, 2017

Which click cli is it breaking for you?

There is no reason why quagga shouldn't match frr on the environment variables but I will test the setting for quagga and will update the diffs.

@rodnymolina
Copy link
Contributor

Any "show" command (e.g. show bgp sum). Btw, i'm referring to my Quagga fix, not FRR one (yours).

@nikos-github
Copy link
Collaborator Author

I will test in quagga and see and fix accordingly if possible.

@nikos-github
Copy link
Collaborator Author

@lguohan Ok I fixed this in quagga by pointing the docker's pager to cat instead of less. I tested it extensively and it works. Are you ok with that change? Want me to post the diffs?

@nikos-github
Copy link
Collaborator Author

The changes below will complete the fix for quagga/vtysh:

diff --git a/debian/quagga.postinst b/debian/quagga.postinst
index bed2fd3..20c6d76 100644
--- a/debian/quagga.postinst
+++ b/debian/quagga.postinst
@@ -1,5 +1,8 @@
#!/bin/bash -e

+rm /etc/alternatives/pager
+ln -s /bin/cat /etc/alternatives/pager
+
if [ -n "$DEBIAN_SCRIPT_DEBUG" ]; then set -v -x; DEBIAN_SCRIPT_TRACE=1; fi
${DEBIAN_SCRIPT_TRACE:+ echo "#42#DEBUG# RUNNING $0 $*"}

Are you ok with those changes?

@nikos-github
Copy link
Collaborator Author

I have submitted the PR for quagga as well. Please approve.

@lguohan
Copy link
Collaborator

lguohan commented Nov 10, 2017

can you update the sonic-quagga submodule head in this PR?

@lguohan lguohan merged commit f18ed0d into sonic-net:master Nov 13, 2017
Mrak-IW pushed a commit to Mrak-IW/sonic-buildimage that referenced this pull request Dec 11, 2017
lguohan pushed a commit that referenced this pull request Dec 11, 2017
abdosi added a commit that referenced this pull request Mar 4, 2020
 Enable m_isCombinedMirrorV6Table for BFN platform (#1212)
[vnet]: Update VNET route table size to 40K for BITMAP implementation (#1132)
 Default action for Egress ACL Table not poulated. (#1208)
Add/Del lag_name_map item according to lag adding and removing (#1124)
Increase ip2me CIR/CBR for faster in-band file transfers (#1000)
pphuchar pushed a commit to SONIC-DEV/sonic-buildimage that referenced this pull request Mar 9, 2020
 Enable m_isCombinedMirrorV6Table for BFN platform (sonic-net#1212)
[vnet]: Update VNET route table size to 40K for BITMAP implementation (sonic-net#1132)
 Default action for Egress ACL Table not poulated. (sonic-net#1208)
Add/Del lag_name_map item according to lag adding and removing (sonic-net#1124)
Increase ip2me CIR/CBR for faster in-band file transfers (sonic-net#1000)
tiantianlv pushed a commit to SONIC-DEV/sonic-buildimage that referenced this pull request Apr 24, 2020
 Enable m_isCombinedMirrorV6Table for BFN platform (sonic-net#1212)
[vnet]: Update VNET route table size to 40K for BITMAP implementation (sonic-net#1132)
 Default action for Egress ACL Table not poulated. (sonic-net#1208)
Add/Del lag_name_map item according to lag adding and removing (sonic-net#1124)
Increase ip2me CIR/CBR for faster in-band file transfers (sonic-net#1000)
yxieca added a commit to yxieca/sonic-buildimage that referenced this pull request Jul 9, 2020
[aclorch] Use IPv6 Next Header internally for protocol number on MLNX platform (sonic-net#1343)
Add/Del lag_name_map item according to lag adding and removing (sonic-net#1124)

Signed-off-by: Ying Xie <ying.xie@microsoft.com>
yxieca added a commit that referenced this pull request Jul 9, 2020
[aclorch] Use IPv6 Next Header internally for protocol number on MLNX platform (#1343)
Add/Del lag_name_map item according to lag adding and removing (#1124)

Signed-off-by: Ying Xie <ying.xie@microsoft.com>
abdosi added a commit that referenced this pull request Sep 28, 2020
 Fix load minigraph on 201911 branch. (#1124)
 Fixed config load_minigrpah not working for Multi-asic platfroms.
     (#1123)

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-buildimage that referenced this pull request Jan 14, 2021
…-net#1124)

The code is to add/del items in LAG_NAME_MAP_TABLE in COUNTERS_DB if a lag is added or removed.
The code need the below pull request:
sonic-net#51 in Azure/sonic-py-swsssdk
read portchannel name from LAG_NAME_MAP_TABLE in COUNTERS_DB sonic-net#51

For use of LAG_NAME_MAP_TABLE in COUNTERS_DB just like fdbshow.
I have create another pull request in Azure/sonic-utilities:
Show mac learned on lag interface sonic-net#730
theasianpianist pushed a commit to theasianpianist/sonic-buildimage that referenced this pull request Feb 5, 2022
…-net#1124)

The code is to add/del items in LAG_NAME_MAP_TABLE in COUNTERS_DB if a lag is added or removed.
The code need the below pull request:
sonic-net#51 in Azure/sonic-py-swsssdk
read portchannel name from LAG_NAME_MAP_TABLE in COUNTERS_DB sonic-net#51

For use of LAG_NAME_MAP_TABLE in COUNTERS_DB just like fdbshow.
I have create another pull request in Azure/sonic-utilities:
Show mac learned on lag interface sonic-net#730
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.

4 participants