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.

Sign up for GitHub

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

Jump to bottom

Convert docker-fpm-frr to buster #4920

Merged
merged 2 commits into from
Jul 29, 2020
Merged

Convert docker-fpm-frr to buster #4920

merged 2 commits into from
Jul 29, 2020

Conversation

joyas-joseph
Copy link
Contributor

Signed-off-by: Joyas Joseph joyas_joseph@dell.com

- Why I did it
This is an enhancement.

- How I did it
Code change

- How to verify it
Build target/docker-fpm-frr.gz
Verify that /etc/apt/sources.list points to buster using docker exec bgp cat /etc/apt/sources.list

BGP neighborship is established.

root@sonic:~# show ip bgp summary 

IPv4 Unicast Summary:
BGP router identifier 10.1.0.1, local AS number 65100 vrf-id 0
BGP table version 1
RIB entries 1, using 184 bytes of memory
Peers 1, using 20 KiB of memory

Neighbor        V         AS MsgRcvd MsgSent   TblVer  InQ OutQ  Up/Down State/PfxRcd
6.1.1.1         4        100      96      96        0    0    0 01:32:04            0

Total number of neighbors 1
root@sonic:~#

- Description for the changelog
Convert docker-fpm-frr to buster

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

@pavel-shirshov
Copy link
Contributor

pavel-shirshov commented Jul 8, 2020 •
edited
Loading

I believe we can avoid building our own swig and libyang for buster.
https://github.com/FRRouting/frr/blob/7bc33bcc1996b554328792d80a8e5557ccaba6f5/doc/developer/building-libyang.rst
Buster includes both dependency. So we could stop building libyang and swig

@joyas-joseph
Copy link
Contributor Author

https://github.com/FRRouting/frr/blob/7bc33bcc1996b554328792d80a8e5557ccaba6f5/doc/developer/building-libyang.rst

patched libyang is required for mgmt-framework.

@pavel-shirshov
Copy link
Contributor

pavel-shirshov commented Jul 9, 2020 •
edited
Loading

@joyas-joseph what about swig? Can we use swig from the repo, and don't build it by ourselves?

@joyas-joseph
Copy link
Contributor Author

@joyas-joseph what about swig? Can we use swig from the repo, and don't build it by ourselves?

Let me take a look.

@joyas-joseph
Copy link
Contributor Author

retest broadcom please

@joyas-joseph
Copy link
Contributor Author

retest vs please

@joyas-joseph
Copy link
Contributor Author

retest broadcom please

@joyas-joseph
Copy link
Contributor Author

retest vs please

pavel-shirshov
pavel-shirshov reviewed Jul 14, 2020
View reviewed changes
build_image.sh Outdated Show resolved Hide resolved
@joyas-joseph
Copy link
Contributor Author

retest broadcom please

pavel-shirshov
pavel-shirshov previously approved these changes Jul 14, 2020
View reviewed changes
@lguohan
Copy link
Collaborator

lguohan commented Jul 15, 2020

@joyas-joseph , can you rebase this pr to the latest master.

lguohan
lguohan previously approved these changes Jul 15, 2020
View reviewed changes
pavel-shirshov
pavel-shirshov previously approved these changes Jul 15, 2020
View reviewed changes
qiluo-msft
qiluo-msft requested changes Jul 15, 2020
View reviewed changes
Copy link
Collaborator

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

As comments

@joyas-joseph joyas-joseph dismissed stale reviews from pavel-shirshov and lguohan via fe01bee July 15, 2020 17:54
lguohan
lguohan previously approved these changes Jul 15, 2020
View reviewed changes
qiluo-msft
qiluo-msft previously approved these changes Jul 15, 2020
View reviewed changes
@lguohan
Copy link
Collaborator

lguohan commented Jul 16, 2020

retest vs please

pavel-shirshov
pavel-shirshov previously approved these changes Jul 16, 2020
View reviewed changes
@lguohan
Copy link
Collaborator

lguohan commented Jul 21, 2020

retest mellanox please

lguohan
lguohan previously approved these changes Jul 21, 2020
View reviewed changes
@pavel-shirshov
Copy link
Contributor

pavel-shirshov commented Jul 21, 2020 •
edited
Loading

https://packages.debian.org/buster/swig
buster has required swig version 3.12. How we get 3.10 version in the logs?
3.10 it is previous version
https://packages.debian.org/stretch/swig
Are we sure we move docker to buster?

@joyas-joseph
Copy link
Contributor Author

https://packages.debian.org/buster/swig
buster has required swig version 3.12. How we get 3.10 version in the logs?
3.10 it is previous version
https://packages.debian.org/stretch/swig
Are we sure we move docker to buster?

libyang was being compiled as a dependency for docker-sonic-vs which was using stretch environment and hence the use of 3.0.10 version of swig.
swig in buster environment is version 3.0.12.

@joyas-joseph
Copy link
Contributor Author

retest mellanox please

@pavel-shirshov
Copy link
Contributor

Are we going to use binary which is build under stretch with the binary which is building under buster? Is it safe?

@joyas-joseph
Copy link
Contributor Author

Are we going to use binary which is build under stretch with the binary which is building under buster? Is it safe?

The way things are setup is that if we have a stretch docker that requires libyang, then it will be built in a stretch environment and that package will be used in the docker. At the same time, if we have a buster docker that requires libyang, then another libyang is built in buster environment and that package would be used.

After the change to use swig from the distribution, we saw a build failure for 'vs' platform only. Build succeeded for other platforms. This was because 'vs' platform had 'docker-sonic-vs' which was a stretch docker and so libyang was being built for stretch. libyang requires at least 3.0.12 version but stretch had 3.0.10 version. Hence the failure.

Now we have docker-sonic-vs converted to buster and so dont see a case of libyang being built for stretch. Hence the change to add back using swig from the distribution.

@lguohan
Copy link
Collaborator

lguohan commented Jul 22, 2020

retest mellanox please

@pavel-shirshov
Copy link
Contributor

Thank you for detailed explanations @joyas-joseph !

pavel-shirshov
pavel-shirshov previously approved these changes Jul 22, 2020
View reviewed changes
@lguohan
Copy link
Collaborator

lguohan commented Jul 23, 2020

it looks like mellanox build keeps failing at the same place.

https://sonic-jenkins.westus2.cloudapp.azure.com/job/mellanox/job/buildimage-mlnx-all-pr/4142/console

@joyas-joseph
Copy link
Contributor Author

it looks like mellanox build keeps failing at the same place.

https://sonic-jenkins.westus2.cloudapp.azure.com/job/mellanox/job/buildimage-mlnx-all-pr/4142/console

make all for mellanox works.

ENABLE_SYNCD_RPC=y make target/docker-syncd-mlnx-rpc.gz target/docker-ptf-mlnx.gz fails with the below message.

[ FAIL LOG START ] [ target/debs/stretch/libsaivs-dev_1.0.0_amd64.deb-uninstall ]
target/debs/stretch/libsaivs-dev_1.0.0_amd64.deb does not exist

Not sure why uninstall is called on something that doesn't exist.

@joyas-joseph joyas-joseph dismissed stale reviews from pavel-shirshov and lguohan via bf3cdb5 July 23, 2020 18:01
@joyas-joseph
Copy link
Contributor Author

Probably rules for LIBSAIVS and LIBSAIVS_DEV needs to be moved to platform/vs/rules.mk

@pavel-shirshov
Copy link
Contributor

Check .dep files.

@joyas-joseph
Copy link
Contributor Author

retest baseimage please

@lguohan
Copy link
Collaborator

lguohan commented Jul 25, 2020

#5039 should solve the mellanox build failure.

@joyas-joseph
Convert docker-fpm-frr to buster
ad62499 
Signed-off-by: Joyas Joseph <joyas_joseph@dell.com>
@joyas-joseph
Use 'swig' from the distro
eb25e9b 
Signed-off-by: Joyas Joseph <joyas_joseph@dell.com>
@joyas-joseph
Copy link
Contributor Author

retest vsimage please

@joyas-joseph
Copy link
Contributor Author

retest broadcom please

1 similar comment
@lguohan
Copy link
Collaborator

lguohan commented Jul 29, 2020

retest broadcom please

@lguohan
Copy link
Collaborator

lguohan commented Jul 29, 2020

retest vsimage please

@lguohan
Copy link
Collaborator

lguohan commented Jul 29, 2020

retest mellanox please

@lguohan lguohan merged commit f0dfe36 into sonic-net:master Jul 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xumia xumia xumia left review comments

@pavel-shirshov pavel-shirshov pavel-shirshov approved these changes

@qiluo-msft qiluo-msft qiluo-msft left review comments

@lguohan lguohan lguohan left review comments

Assignees
No one assigned
Labels
bgp Enhancement ➕ FRR 🚥
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@joyas-joseph @pavel-shirshov @lguohan @qiluo-msft @xumia @jleveque