Skip to content

Update sonic-swss-common pipeline build for Bullseye#716

Merged
saiarcot895 merged 14 commits intosonic-net:masterfrom
saiarcot895:bullseye-docker-sonic-vs
Mar 28, 2023
Merged

Update sonic-swss-common pipeline build for Bullseye#716
saiarcot895 merged 14 commits intosonic-net:masterfrom
saiarcot895:bullseye-docker-sonic-vs

Conversation

@saiarcot895
Copy link
Contributor

@saiarcot895 saiarcot895 commented Nov 24, 2022

With docker-sonic-vs now upgraded to Bullseye (in sonic-net/sonic-buildimage#13294), make the necessary changes in the sonic-swss-common pipeline build to build for and use Bullseye.

Signed-off-by: Saikrishna Arcot [email protected]

This is in preparation for docker-sonic-vs being upgraded to Bullseye,
where Python 2 is no longer available.

Signed-off-by: Saikrishna Arcot <[email protected]>
@saiarcot895 saiarcot895 marked this pull request as draft November 24, 2022 00:22
@saiarcot895 saiarcot895 force-pushed the bullseye-docker-sonic-vs branch from c442f7f to 87f5077 Compare November 27, 2022 02:28
@saiarcot895 saiarcot895 changed the title Don't install python-swsscommon in docker-sonic-vs Update sonic-swss-common pipeline build for Bullseye Feb 28, 2023
@saiarcot895 saiarcot895 marked this pull request as ready for review March 20, 2023 16:14
Signed-off-by: Saikrishna Arcot <[email protected]>
Signed-off-by: Saikrishna Arcot <[email protected]>
Looks like the C++ code generated by SWIG 4.0 takes much longer to
compile than the C++ code generated by SWIG 3.0 for some reason. For
this reason, bump up the time limit for the sairedis build to 90
minutes.

There's some fixes that can be done on the sairedis side, but that needs
swss-common to be properly building.

Signed-off-by: Saikrishna Arcot <[email protected]>
In updating docker-sonic-vs, instead of removing the existing packages
first and then installing the current version of packages, just replace
the existing deb packages. This gets around a dependency issue due to
sonic-eventd being in the docker-config-engine-bullseye layer.

Signed-off-by: Saikrishna Arcot <[email protected]>
@liushilongbuaa
Copy link
Contributor

@saiarcot895 ,

Build-Depends: dh-exec (>=0.3), debhelper (>= 12), autotools-dev, libboost-dev | libboost1.71-dev

Why this line doesn't need change?

RUN dpkg -i /debs/syncd-vs_1.0.0_amd64.deb

RUN dpkg -i /debs/swss_1.0.0_amd64.deb
RUN dpkg -i /debs/libswsscommon_1.0.0_amd64.deb /debs/python3-swsscommon_1.0.0_amd64.deb /debs/sonic-db-cli_1.0.0_amd64.deb /debs/libsaimetadata_1.0.0_amd64.deb /debs/libsairedis_1.0.0_amd64.deb /debs/libsaivs_1.0.0_amd64.deb /debs/syncd-vs_1.0.0_amd64.deb /debs/swss_1.0.0_amd64.deb
Copy link
Contributor

Choose a reason for hiding this comment

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

libswsscommon depends on libboost1.71-dev. (in debian/control Line5)
But docker-sonic-vs uninstalled libboost1.71. This line should fail. @saiarcot895 can you rerun the pipeline? Maybe the former PR check used old docker-sonic-vs image.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The dependency string is specified as libboost-dev | libboost1.71-dev, which means either libboost-dev or libboost1.71-dev needs to be installed. Since libboost-dev is installed in Bullseye, the dependency is met.

In Buster, Boost 1.71 is installed through the libboost1.71-dev (and associated) packages. The Buster default is Boost 1.67, and I guess something explicitly wanted to use Boost 1.71, so all packages are built with that. Because libswsscommon might still get built for Buster (it's needed for the nat and sflow containers), that part of the dependency string cannot be removed just yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

/mnt$ docker run --rm -ti --entrypoint bash docker-sonic-vs:latest
root@9f79d2f3aae7:/# apt list| grep libboost

WARNING: apt does not have a stable CLI interface. Use with caution in scripts.

libboost-serialization1.74.0/now 1.74.0-9 amd64 [installed,local]

Copy link
Contributor

Choose a reason for hiding this comment

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

docker-sonic-vs didn't install libboost-dev or libboost1.71-dev

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The docker container shouldn't need the dev package. The dev package is only for building libswsscommon, and should only be present in the slave container.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Line 5 of debian/control lists the build-time dependencies. This does not necessarily correspond to runtime dependencies. The runtime dependency list is automatically generated, and gets filled into the shlibs:Depends variable for each package. For the libswsscommon package, this dependency list is libboost-serialization1.74.0 (>= 1.74.0), libc6 (>= 2.14), libgcc-s1 (>= 3.0), libhiredis0.14 (>= 0.14.0), libnl-3-200 (>= 3.2.21), libnl-nf-3-200 (>= 3.2.7), libnl-route-3-200 (>= 3.2.7), libstdc++6 (>= 9), libuuid1 (>= 2.16), libyang, libzmq5 (>= 4.0.1+dfsg) (for a package built on Bullseye).

@liushilongbuaa
Copy link
Contributor

I didn't see this PR. So I have a fix for platform-daemons. https://github.com/sonic-net/sonic-platform-daemons/pull/349/files
It is running in sonic-slave-bullseye, so it succeeded.
If it run in docker-sonic-vs, it fails. https://github.com/sonic-net/sonic-sairedis/pull/1227/files

@saiarcot895
Copy link
Contributor Author

/azp run Azure.sonic-swss-common

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@liushilongbuaa liushilongbuaa left a comment

Choose a reason for hiding this comment

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

pipeline succeeded.

@saiarcot895
Copy link
Contributor Author

@liushilongbuaa can you force merge this PR? Looks like the status check on Github is looking at a merge commit instead.

@liushilongbuaa
Copy link
Contributor

@liushilongbuaa can you force merge this PR? Looks like the status check on Github is looking at a merge commit instead.

@xumia, please help

@liushilongbuaa liushilongbuaa requested a review from xumia March 27, 2023 06:09
@xumia
Copy link
Collaborator

xumia commented Mar 27, 2023

@liushilongbuaa can you force merge this PR? Looks like the status check on Github is looking at a merge commit instead.

@xumia, please help

It is weird, the results in the GitHub check page are succeeded.
image

@xumia
Copy link
Collaborator

xumia commented Mar 27, 2023

@saiarcot895 , could you please try to run /easycla ? I can merge it if the easycla passed.
Or could you please send another PR?

@saiarcot895
Copy link
Contributor Author

saiarcot895 commented Mar 27, 2023

I can run EasyCLA, but the merge section of the PR is looking at some other commit. If this fails, I'll push an empty commit.

image

@saiarcot895
Copy link
Contributor Author

/easycla

@saiarcot895 saiarcot895 merged commit c3b2c71 into sonic-net:master Mar 28, 2023
@saiarcot895 saiarcot895 deleted the bullseye-docker-sonic-vs branch March 28, 2023 16:03
saiarcot895 added a commit to saiarcot895/sonic-swss-common that referenced this pull request Jul 28, 2023
PR sonic-net#716 removed a step where existing deb packages were removed before
installing the new deb packages. The problem is that due to Docker's
change detection code, where it only checks the file size and the
modification timestamp, even if the file gets modified when installing a
newer debian package, the file size may remain the same, and the
modification timestamp will remain the same (since this is based on the
debian/changelog timestamp). This caused changed binaries to not
actually replace the existing binaries.

Fix this by re-adding the line that removed existing packages first.

Signed-off-by: Saikrishna Arcot <[email protected]>
qiluo-msft pushed a commit that referenced this pull request Aug 1, 2023
…807)

PR #716 removed a step where existing deb packages were removed before installing the new deb packages. The problem is that due to Docker's change detection code, where it only checks the file size and the modification timestamp, even if the file gets modified when installing a newer debian package, the file size may remain the same, and the modification timestamp will remain the same (since this is based on the debian/changelog timestamp). This caused changed binaries to not actually replace the existing binaries.

Fix this by re-adding the line that removed existing packages first.
SviatoslavBoichuk pushed a commit to SviatoslavBoichuk/sonic-swss-common that referenced this pull request Sep 7, 2023
…onic-net#807)

PR sonic-net#716 removed a step where existing deb packages were removed before installing the new deb packages. The problem is that due to Docker's change detection code, where it only checks the file size and the modification timestamp, even if the file gets modified when installing a newer debian package, the file size may remain the same, and the modification timestamp will remain the same (since this is based on the debian/changelog timestamp). This caused changed binaries to not actually replace the existing binaries.

Fix this by re-adding the line that removed existing packages first.
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.

3 participants