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

Add frr-reload.py to FRR container #2462

Closed
wants to merge 11 commits into from
Closed

Add frr-reload.py to FRR container #2462

wants to merge 11 commits into from

Conversation

mslocrian
Copy link
Contributor

- What I did
I have added frr-reload.py to the frr docker container.

For those who don't use it, it is a great tool for those who want to deploy their own frr configuration to the sonic host. You can do the following:

  1. push an updated frr.conf to /etc/sonic/frr/frr.conf
  2. ensure your frr.conf is functional - docker exec -ti bgp vtysh -f /etc/frr/frr.conf --dryrun
  3. run docker exec -ti bgp /usr/bin/frr-reload.py --reload /etc/frr/frr.conf

frr-reload.py makes a local copy of running config, and compares it to the provided config. It then only applies the diffs to vtysh. It's much nicer than pushing a config and having to reload a container (in the majority of cases)

Note, I haven't actually rebuilt the sonic image and tested this properly, but I shall!

- How I did it
Copied frr-reload.py from FRR repo into frr container and modified Dockerfile.j2

- How to verify it

- Description for the changelog
Added frr-reload.py to FRR container.

- A picture of a cute animal (not mandatory but encouraged)
Animal

Signed-off-by: Stegen Smith <stegen@owns.com>
@mslocrian
Copy link
Contributor Author

Erm. Accidentally have the daemons and daemons.conf changes in here too.

Signed-off-by: Stegen Smith <stegen@owns.com>
@mslocrian
Copy link
Contributor Author

Rebased! Should have branched the other PR of mine.

@nikos-github
Copy link
Collaborator

I believe this is part of the pythontools debian pkg that is built along with frr. Perhaps it is best for those who need it, to install the pkg rather than bring it into sonic-buildimage as it then becomes difficult to track and reflect any changes to it from its source location. Sonic team can comment here @lguohan

@mslocrian
Copy link
Contributor Author

I believe this is part of the pythontools debian pkg that is built along with frr. Perhaps it is best for those who need it, to install the pkg rather than bring it into sonic-buildimage as it then becomes difficult to track and reflect any changes to it from its source location. Sonic team can comment here @lguohan

Ah, you're right: https://github.com/FRRouting/frr/releases. frr-pythontools*.deb

Personally, if people are going to be using the features that you've added to allow to split frr configuration, then this would be a super helpful utility to just have on hand.

That being said, if someone is going to make the conscious decision to enable the split frr functionality, then I suppose that they can also just install it.

So, I suppose that there are two questions.

  1. Do the powers to be think it's a helpful enough utility to include?
  2. If so, would that be something to be added to docker_fpm_frr_debs (wherever that is)?

@mslocrian
Copy link
Contributor Author

@nikos-github, I've shuffled things around so I'm not COPY'ing the file directly, but installing the frr-pythonutils package which is built when the frr package is built.

This should keep things consistent for when one upgrades frr.

I suppose the discussion of whether or not it should be included in the base image should still be had, but it's a slick utility for those wanting to externally manage frr.conf.

@mslocrian
Copy link
Contributor Author

Verified change works in fresh image install.

root@sonic:/home/sonic# docker exec -t -i bgp ls /usr/lib/frr
babeld	eigrpd	       frrcommon.sh  ldpd    ospfd  ripd     watchfrr
bfdd	frr-reload     frrinit.sh    nhrpd   pbrd   ripngd   watchfrr.sh
bgpd	frr-reload.py  isisd	     ospf6d  pimd   staticd  zebra
root@sonic:/home/sonic# docker exec -t -i bgp dpkg --list | grep frr
ii  frr            6.0.2-201901 amd64        FRRouting suite of internet proto
ii  frr-pythontool 6.0.2-201901 all          FRRouting suite - Python tools
root@sonic:/home/sonic# show version
SONiC Software Version: SONiC.add_frr_reload.0-dirty-20190128.181135
Distribution: Debian 9.7
Kernel: 4.9.0-8-amd64
Build commit: 0777bc0
Build date: Tue Jan 29 03:57:43 UTC 2019

@batmancn
Copy link

batmancn commented Jan 30, 2019

So, does this PR is for save configure in sonic-frr? #2355 is another way for this. Maybe some conflict, I have to have a look.

@mslocrian
Copy link
Contributor Author

@batmancn,

This is for a different use case. @nikos-github has done some great work in splitting out the frr config from something that is managed within sonic, to being able to manage it outside.

The frr-pythontool package is for the latter case. What you can do is place a new frr config, parse it through the frr-reload.py and it only applies the diff of running config to your desired config. When you want to get a config into place without having to touch the bgpd process, then that can be pretty useful.

@batmancn
Copy link

batmancn commented Jan 30, 2019

@batmancn,

This is for a different use case. @nikos-github has done some great work in splitting out the frr config from something that is managed within sonic, to being able to manage it outside.

The frr-pythontool package is for the latter case. What you can do is place a new frr config, parse it through the frr-reload.py and it only applies the diff of running config to your desired config. When you want to get a config into place without having to touch the bgpd process, then that can be pretty useful.

Great, That's cool.

If there is some SPEC or PR about these work?

@mslocrian
Copy link
Contributor Author

The PR for the spit config can be found here:
#2286

The file/package that I am adding to the container is already part of the FRR software.
https://github.com/FRRouting/frr/releases

The package is already getting built when sonic goes through its own frr build in the sonic-frr repo. I'm just copying that package over for inclusion to the rest of the build process, and getting it added to the deb package list for installation only in the sonic-frr container.

@nikos-github
Copy link
Collaborator

I'll take a look. This approach is preferred.

@@ -36,7 +36,7 @@ fabricd=no
# Check /etc/pam.d/frr if you intend to use "vtysh"!
#
vtysh_enable=yes
zebra_options=" -A 127.0.0.1 -s 90000000 -M fpm"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are you removing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That actually should not have happened. I can't recall why/how that file would have changed, or perhaps I merged something in which I shouldn't have. My bad!

Thank you and good catch. It's been fixed.

@nikos-github
Copy link
Collaborator

retest this please

@mslocrian
Copy link
Contributor Author

@nikos-github , I've rebuilt and reinstalled the image. Things are looking good from that perspective.

root@sonic:/usr/lib/frr# grep zeb /etc/frr/daemons
# The watchfrr and zebra daemons are always started.
zebra_options="  -A 127.0.0.1 -s 90000000 -M fpm"
root@sonic:/usr/lib/frr# ps uaxww | grep zeb
root        69  0.0  0.0  80092  2732 ?        Ss   16:16   0:03 /usr/lib/frr/watchfrr -d -r /usr/lib/frr/watchfrr.sh restart %s -s /usr/lib/frr/watchfrr.sh start %s -k /usr/lib/frr/watchfrr.sh stop %s zebra bgpd staticd
frr         84  0.0  0.2 411976 37828 ?        Ssl  16:16   0:02 /usr/lib/frr/zebra -d -A 127.0.0.1 -s 90000000 -M fpm
root       589  0.0  0.0  11128   996 pts/1    S+   22:27   0:00 grep zeb
root@sonic:/usr/lib/frr# pwd 
/usr/lib/frr
root@sonic:/usr/lib/frr# ls
babeld	bfdd  bgpd  eigrpd  frr-reload	frr-reload.py  frrcommon.sh  frrinit.sh  isisd	ldpd  nhrpd  ospf6d  ospfd  pbrd  pimd	ripd  ripngd  staticd  watchfrr  watchfrr.sh  zebra
root@sonic:/usr/lib/frr# ls -l frr-reload
-rwxr-xr-x 1 root root 198 Feb 24 04:16 frr-reload
root@sonic:/usr/lib/frr# ls -l frr-reload.py 
-rwxr-xr-x 1 root root 52120 Feb 26 06:25 frr-reload.py
root@sonic:/usr/lib/frr# 

Any other testing that you are looking for?

@nikos-github
Copy link
Collaborator

nikos-github commented Feb 27, 2019

@mslocrian The retest this was not for you. It's a jenkins trigger to rerun the tests because they were failing when it first run.

@nikos-github
Copy link
Collaborator

@lguohan Can we merge this after you review?

@nikos-github
Copy link
Collaborator

nikos-github commented Apr 23, 2019

@lguohan This will be conflicting with my debian stretch frr changes. Are we getting it merged or should I include in my diffs?

@mslocrian
Copy link
Contributor Author

Would be excited to see this included, @lguohan! Thanks for pushing on it @nikos-github

@lguohan
Copy link
Collaborator

lguohan commented May 9, 2019

duplicated as #2819

@lguohan lguohan closed this May 9, 2019
dgsudharsan added a commit to dgsudharsan/sonic-buildimage that referenced this pull request Sep 21, 2022
Update sonic-swss submodule pointer to include the following:
* 8eea92e [202205][counters] Revert PR sonic-net#2432 for the buffer queue/pg counters improvement ([sonic-net#2462](sonic-net/sonic-swss#2462))
* 5d8636a [202205] Enhance orchagent and buffer manager in error handling (sonic-net#2414) ([sonic-net#2449](sonic-net/sonic-swss#2449))
* aa22237 [Everflow/ERSPAN] Set correct destination port and mac address when the nexthop is updated for ERSPAN mirror destination (sonic-net#2392) ([sonic-net#2455](sonic-net/sonic-swss#2455))
* 04ce7be check state_db for po before sending ARP/ND pkts (sonic-net#2444) ([sonic-net#2450](sonic-net/sonic-swss#2450))
* f0138a2 [portmgr] Fixed the orchagent crash due to late arrival of notif (sonic-net#2431) ([sonic-net#2451](sonic-net/sonic-swss#2451))
* 7cfde48 Change the log messages in addKernelNeigh/Route from ERROR to INFO ([sonic-net#2437](sonic-net/sonic-swss#2437))
* 2c5116e [202205][counters] Improve performance by polling only configured ports buffer queue/pg counters ([sonic-net#2432](sonic-net/sonic-swss#2432))

Signed-off-by: dgsudharsan <sudharsand@nvidia.com>
prsunny pushed a commit that referenced this pull request Sep 21, 2022
Update sonic-swss submodule pointer to include the following:
* 8eea92e [202205][counters] Revert PR #2432 for the buffer queue/pg counters improvement ([#2462](sonic-net/sonic-swss#2462))
* 5d8636a [202205] Enhance orchagent and buffer manager in error handling (#2414) ([#2449](sonic-net/sonic-swss#2449))
* aa22237 [Everflow/ERSPAN] Set correct destination port and mac address when the nexthop is updated for ERSPAN mirror destination (#2392) ([#2455](sonic-net/sonic-swss#2455))
* 04ce7be check state_db for po before sending ARP/ND pkts (#2444) ([#2450](sonic-net/sonic-swss#2450))
* f0138a2 [portmgr] Fixed the orchagent crash due to late arrival of notif (#2431) ([#2451](sonic-net/sonic-swss#2451))
* 7cfde48 Change the log messages in addKernelNeigh/Route from ERROR to INFO ([#2437](sonic-net/sonic-swss#2437))
* 2c5116e [202205][counters] Improve performance by polling only configured ports buffer queue/pg counters ([#2432](sonic-net/sonic-swss#2432))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants