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

Update P4RT docker to Bookworm #18886

Merged
merged 1 commit into from
May 30, 2024
Merged

Conversation

mkeda
Copy link
Collaborator

@mkeda mkeda commented May 6, 2024

Why I did it

Enable the P4RT docker container to be able to build using Bookworm.

How I did it

Confirm that the P4RT docker is able to build in a local workspace.

NOBULLSEYE=1 PLATFORM=<our platform> make target/docker-sonic-p4rt.gz
kbuilder@mikeattig-playground-1:~/sonic_net_sbi/sonic-buildimage/target$ ls -alh
total 307M
drwxrwxr-x  9 kbuilder kbuilder 4.0K Mar 25 21:56 .
drwxrwxr-x 20 kbuilder kbuilder 4.0K Mar 25 21:57 ..
drwxrwxrwx  2 kbuilder kbuilder 4.0K Mar 25 10:56 cache
drwxr-xr-x  7 kbuilder kbuilder 4.0K Mar 25 10:56 debs
-rw-r--r--  1 kbuilder kbuilder  83M Mar 25 21:37 docker-base-bookworm.gz
-rw-r--r--  1 kbuilder kbuilder  468 Mar 25 21:37 docker-base-bookworm.gz-load.log
-rw-r--r--  1 kbuilder kbuilder  91K Mar 25 21:37 docker-base-bookworm.gz.log
-rw-r--r--  1 kbuilder kbuilder 100M Mar 25 21:44 docker-config-engine-bookworm.gz
-rw-r--r--  1 kbuilder kbuilder  532 Mar 25 21:52 docker-config-engine-bookworm.gz-load.log
-rw-r--r--  1 kbuilder kbuilder  64K Mar 25 21:44 docker-config-engine-bookworm.gz.log
-rw-r--r--  1 kbuilder kbuilder 125M Mar 25 21:57 docker-sonic-p4rt.gz
-rw-r--r--  1 kbuilder kbuilder  22K Mar 25 21:57 docker-sonic-p4rt.gz.log
drwxr-xr-x  7 kbuilder kbuilder 4.0K Mar 25 10:56 files
drwxr-xr-x  3 kbuilder kbuilder 4.0K Mar 25 10:56 python-debs
drwxr-xr-x  3 kbuilder kbuilder 4.0K Mar 25 10:56 python-wheels
drwxrwxr-x  7 kbuilder kbuilder 4.0K Mar 25 21:52 vcache
drwxrwxr-x  6 kbuilder kbuilder 4.0K Mar 25 10:56 versions

How to verify it

See above.

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

n/a, this is a feature.

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

Tested branch (Please provide the tested image version)

Tested with latest branch.

Description for the changelog

Update P4RT docker container to build under Bookworm

Link to config_db schema for YANG module changes

n/a

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

@mkeda mkeda requested review from qiluo-msft, xumia and lguohan as code owners May 6, 2024 18:41
Copy link

linux-foundation-easycla bot commented May 6, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: mkeda / name: Michael Attig (ae56b12)

@mkeda mkeda force-pushed the update_p4rt_to_bookworm branch from d053c56 to 06782a6 Compare May 6, 2024 18:51
@mkeda
Copy link
Collaborator Author

mkeda commented May 7, 2024

Is there a way to re-run the Test kvmtest-t0 by Elastictest test? It doesn't look like I have permission to do so. It seems there was a timeout in what appears to be an unrelated telemetry test.

telemetry/test_events.py::test_events[vlab-01-False] 
-------------------------------- live log call ---------------------------------
20:45:08 __init__.pytest_runtest_call             L0040 ERROR  | Traceback (most recent call last):
  File "/usr/local/lib/python3.8/dist-packages/_pytest/python.py", line 1788, in runtest
    self.ihook.pytest_pyfunc_call(pyfuncitem=self)
...
  File "/var/src/sonic-mgmt/tests/telemetry/telemetry_utils.py", line 115, in listen_for_events
    assert results[0] != "", "No output from PTF docker, thread timed out after {} seconds".format(thread_timeout)
AssertionError: No output from PTF docker, thread timed out after 30 seconds

rules/config Outdated
@@ -153,7 +153,7 @@ INCLUDE_DHCP_RELAY = y
INCLUDE_DHCP_SERVER ?= n

# INCLUDE_P4RT - build docker-p4rt for P4RT support
INCLUDE_P4RT = n
INCLUDE_P4RT = y
Copy link
Contributor

@saiarcot895 saiarcot895 May 8, 2024

Choose a reason for hiding this comment

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

Is this change intentional, to build the P4RT docker by default? Was it brought up/discussed somewhere?

FYI @qiluo-msft for awareness.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It looked like it had been turned off, since a build failure was encountered at some point. I wasn't able to reproduce the build failure, so I thought it could be turned back on.

Copy link
Contributor

Choose a reason for hiding this comment

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

@dgsudharsan I see that #16343 disabled the P4RT build by default due to some build issues. Are you still seeing these issues?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We have not enabled it since disabling. I would recommend fixing UT issues I raised before reenabling

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the comments. I went ahead and reverted this particular change, since it's unrelated to upgrading to Bookworm.

Signed-off-by: mkeda <mikeattig@google.com>
@mkeda mkeda force-pushed the update_p4rt_to_bookworm branch from 06782a6 to ae56b12 Compare May 17, 2024 17:26
@mkeda
Copy link
Collaborator Author

mkeda commented May 24, 2024

@saiarcot895 - Are there still concerns on this PR?

@yxieca yxieca merged commit 0e3fbd9 into sonic-net:master May 30, 2024
19 checks passed
arun1355492 pushed a commit to arun1355492/sonic-buildimage that referenced this pull request Jul 26, 2024
Signed-off-by: mkeda <mikeattig@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants