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

[build][reproducible]Fix version file access for dbg container build #18485

Merged
merged 1 commit into from
Apr 12, 2024

Conversation

baxia-lan
Copy link
Contributor

@baxia-lan baxia-lan commented Mar 27, 2024

Why I did it

Reproducible build fails when building dbg containers make INSTALL_DEBUG_TOOLS=y target/docker-orchagent-dbg.gz

Work item tracking

How I did it

How to verify it

export SONIC_VERSION_CONTROL_COMPONENTS=all
make configure PLATFORM=broadcom
make INSTALL_DEBUG_TOOLS=y target/docker-orchagent-dbg.gz

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

@baxia-lan
Copy link
Contributor Author

baxia-lan commented Mar 27, 2024 via email

@k-v1
Copy link
Contributor

k-v1 commented Mar 27, 2024

@baxia-lan

I removed my comment because I thought you mean SONIC_VERSION_CACHE.
This feature is not tested enough and I think it's broken.

SONIC_VERSION_CONTROL and SONIC_DPKG_CACHE should work, of course.
But I'm not sure about debug containers. Maybe I'll check them later too.

@lguohan
Copy link
Collaborator

lguohan commented Mar 30, 2024

@liushilongbuaa , can you take a look?

@k-v1
Copy link
Contributor

k-v1 commented Mar 30, 2024

@baxia-lan

I didn't get any errors for dbg docker container when reproducible build is enabled.
Can you provide any logs?

@liushilongbuaa
Copy link
Contributor

We already get these dbg packages in
This Line
all these packages: gdb gdbserver vim openssh-client sshpass strace

When we copy version files in this line, the version files in buildinfo is the same as normal docker.
docker-orchagent and docker-orchagent-dbg shares the same root folder. So they use the same version file.

@baxia-lan
Copy link
Contributor Author

Take docker-syncd-brcm-dbg.gz as an example, the dbg packages are saved to the dfiles/build/versions/dockers/docker-syncd-brcm/versions-deb-bullseye file, but these version files are not correct loaded when building. The version files copied to docker container are empty when inspecting it.

Another evidence in docker-syncd-brcm-dbg.gz.log:

�[91mWarning: the version of the package gdb is not specified.
Warning: the version of the package gdbserver is not specified.
Warning: the version of the package vim is not specified.
Warning: the version of the package openssh-client is not specified.
Warning: the version of the package sshpass is not specified.
Warning: the version of the package strace is not specified.

The warning is printed out at this line. You can still build the dbg docker because the check_apt_version only give warnings never errors.

Speaking of that, I think the reproducible framework should throw errors instead of warnings but continue with "latest" versions, this silent warning makes the build not reproducible... And pin ALL dependency packages during build, not only the incremental ones to avoid conflicts resolving dependencies versions. I would love to provide the possible solutions for debian packages if you agreed.

docker-syncd-brcm-dbg.gz.log
docker-syncd-brcm.gz.log

@liushilongbuaa
Copy link
Contributor

I checked the log. It didn't work for dbg docker image.

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.

LGTM

@k-v1
Copy link
Contributor

k-v1 commented Apr 2, 2024

The warning is printed out at this line. You can still build the dbg docker because the check_apt_version only give warnings never errors.

As I understand, we are using MIRROR_SNAPSHOT for reproducible builds by default since last year.
It means that all deb packages for reproducible builds are installed from snapshots and check_apt_version can be ignored at all.

Get:16 http://packages.trafficmanager.net/snapshot/debian/20240123T000238Z bullseye/main amd64 gdb amd64 10.1-1.7 [3395 kB]
Get:17 http://packages.trafficmanager.net/snapshot/debian/20240123T000238Z bullseye/main amd64 gdbserver amd64 10.1-1.7 [421 kB]
Get:18 http://packages.trafficmanager.net/snapshot/debian/20240123T000238Z bullseye/main amd64 libgpm2 amd64 1.20.7-8 [35.6 kB]
Get:19 http://packages.trafficmanager.net/snapshot/debian/20240123T000238Z bullseye/main amd64 libunwind8 amd64 1.3.2-2 [54.5 kB]
Get:20 http://packages.trafficmanager.net/snapshot/debian/20240123T000238Z bullseye/main amd64 sshpass amd64 1.09-1+b1 [13.0 kB]
Get:21 http://packages.trafficmanager.net/snapshot/debian/20240123T000238Z bullseye/main amd64 strace amd64 5.10-1 [1084 kB]

@k-v1
Copy link
Contributor

k-v1 commented Apr 2, 2024

@liushilongbuaa

20240123

BTW, why last version update was in January?

@baxia-lan
Copy link
Contributor Author

It means that all deb packages for reproducible builds are installed from snapshots and check_apt_version can be ignored at all.

We don't use this public mirror to build as we host our own mirrors to have better control. But I assume the mirror repositories host multiple versions of packages as the version files might be different between different release branches and can be upgraded? The version files should be still needed to tell which versions of packages to install. Anyway, the behavior of the reproducible build should be consistent between non-dbg and dbg containers build. We should merge this PR and continue discussion for separate issues if needed.

@k-v1
Copy link
Contributor

k-v1 commented Apr 3, 2024

Anyway, the behavior of the reproducible build should be consistent between non-dbg and dbg containers build.

I agree. It's just FYI.
Also keep in mind warnings also exist for build_debian.sh:

https://sonic-build.azurewebsites.net/api/sonic/artifacts?branchName=master&platform=broadcom&buildId=514080&target=target%2Fsonic-broadcom.bin.log

Warning: the version of the package libcairo2-dev is not specified.
Warning: the version of the package libdbus-1-dev is not specified.
Warning: the version of the package libgirepository1.0-dev is not specified.
Warning: the version of the package libsystemd-dev is not specified.
Warning: the version of the package pkg-config is not specified.

And if you use some non-standard package installation like apt-get install <package>/bookworm-backports instead of apt install -t bookworm-backports <package> then version of this package probably also won't saved.
That's why I prefer to use snapshots based on local mirror. Mirror has multiple versions but snapshot has only one for each package. And if we are talk about repositories like https://download.docker.com/ then you can pin version of the package like here

@baxia-lan
Copy link
Contributor Author

Anyway, the behavior of the reproducible build should be consistent between non-dbg and dbg containers build.

I agree. It's just FYI. Also keep in mind warnings also exist for build_debian.sh:

https://sonic-build.azurewebsites.net/api/sonic/artifacts?branchName=master&platform=broadcom&buildId=514080&target=target%2Fsonic-broadcom.bin.log

Warning: the version of the package libcairo2-dev is not specified.
Warning: the version of the package libdbus-1-dev is not specified.
Warning: the version of the package libgirepository1.0-dev is not specified.
Warning: the version of the package libsystemd-dev is not specified.
Warning: the version of the package pkg-config is not specified.

And if you use some non-standard package installation like apt-get install <package>/bookworm-backports instead of apt install -t bookworm-backports <package> then version of this package probably also won't saved. That's why I prefer to use snapshots based on local mirror. Mirror has multiple versions but snapshot has only one for each package. And if we are talk about repositories like https://download.docker.com/ then you can pin version of the package like here

Opened a thread to discuss the strategy. Please move discussions there. I also add my proposal in the issue description.

If all agreed, can this PR be approved and merged now?

@baxia-lan
Copy link
Contributor Author

@lguohan @qiluo-msft @xumia

Kindly ping for review.

Copy link
Collaborator

@xumia xumia left a comment

Choose a reason for hiding this comment

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

LGTM

@xumia xumia merged commit 19e3bbf into master Apr 12, 2024
19 checks passed
@baxia-lan baxia-lan deleted the docker-dbg branch May 18, 2024 18:19
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.

5 participants