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

Bookworm migration for sonic-mgmt-framework and sonic-gnmi dockers #18548

Merged

Conversation

ranjinidn
Copy link
Contributor

@ranjinidn ranjinidn commented Apr 3, 2024

Why I did it

The base OS image has already migrated to bookwork. The current sonic-mgmt-framework is still on buster and sonic-gnmi on bullseye. The changes are made to migrate the sonic-mgmt-framework and sonic-gnmi dockers to bookworm build.

Work item tracking
  • Microsoft ADO (number only):

How I did it

Changed the docker manifest files to do the build for bookwork distro. The dependency packages are updated accordingly.

How to verify it

Built the mgmt-framework and gnmi dockers and executed the azure pipeline unit tests locally.

Note: The sonic-mgmt-framework build for bookworm has a dependency on the the below PR for the build to go through
sonic-net/sonic-mgmt-common#133
sonic-net/sonic-mgmt-framework#132

The sonic-gnmi build for bookworm has a dependency on the the below PR for the build to go through
sonic-net/sonic-gnmi#216

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

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

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

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

grpcio==1.58.0 \
grpcio-tools==1.20.0 \
grpcio-tools==1.58.0 \
certifi==2017.4.17 \
python-dateutil==2.6.0 \
six==1.11.0 \
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need these packages for docker-sonic-mgmt-framework?

connexion
python-dateutil
six
outdated version of certifi

Copy link
Contributor

Choose a reason for hiding this comment

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

Addressed. Removed unnecessary packages.

…ub.com:ranjinidn/sonic-buildimage into bookworm_migration_for_mgmt_framework_and_gnmi
@ranjinidn ranjinidn closed this Apr 18, 2024
@ranjinidn ranjinidn reopened this Apr 18, 2024
@ranjinidn ranjinidn closed this Apr 19, 2024
@ranjinidn ranjinidn reopened this Apr 19, 2024
…ub.com:ranjinidn/sonic-buildimage into bookworm_migration_for_mgmt_framework_and_gnmi
@ranjinidn ranjinidn force-pushed the bookworm_migration_for_mgmt_framework_and_gnmi branch from 4a79131 to bff4efa Compare April 23, 2024 23:34
@saiarcot895
Copy link
Contributor

Would you be able to handle the telemetry docker in this PR as well?

@ranjinidn
Copy link
Contributor Author

Would you be able to handle the telemetry docker in this PR as well?

Would you be able to handle the telemetry docker in this PR as well?

Yes we will be able to migrate both gnmi and mgmt-framework together as sonic-mgmt-common repo is common library used by both and the bookworm migration changes are already merged to master. We are waiting on the sonic-gnmi repo changes for bookworm migration to be merged. (sonic-net/sonic-gnmi#216)

Regarding the build failure in this PR we find a few open PRs have the same build failure reason. Hence we think the build failure is not related to the changes in this PR. How do we get past this so that this PR can be merged?

@saiarcot895
Copy link
Contributor

That build failure is now fixed. Looks like there's a possibly new failure for marvell-armhf:

python scripts/klish_preproc_cmdtree.py /sonic/src/sonic-mgmt-framework/build/target/command-tree /sonic/src/sonic-mgmt-framework/build/target/cli-macro 6 
make[4]: python: No such file or directory
make[4]: *** [Makefile:38: /sonic/src/sonic-mgmt-framework/build/target/command-tree/.done] Error 127
make[4]: Leaving directory '/sonic/src/sonic-mgmt-framework/CLI/clitree'
make[3]: *** [Makefile:39: all] Error 2
make[3]: Leaving directory '/sonic/src/sonic-mgmt-framework/CLI'
make[2]: *** [Makefile:50: cli] Error 2
make[2]: *** Waiting for unfinished jobs....

This might need a change in sonic-slave-bookworm to add the python-is-python3 for all architectures, instead of just amd64 and arm64.

@ranjinidn ranjinidn force-pushed the bookworm_migration_for_mgmt_framework_and_gnmi branch 2 times, most recently from 2a9caa8 to 4fbd5b7 Compare May 1, 2024 19:23
@ranjinidn
Copy link
Contributor Author

@saiarcot895 we are facing some build failures and this is blocking us to test further.
The following PRs are opened to resolve the build dependencies on mgmt-framework and gnmi.
sonic-net/sonic-mgmt-framework#141
sonic-net/sonic-gnmi#224
However for the gnmi PR apart from build failure some pipeline Test failures are also seen. This is in discussion with the appropriate teams to resolve. We are not sure the timeline for the above PRs to be resolved.

In the meanwhile if the submodule update for mgmt-common is reverted (PR below) we can resolve the build issue for sonic-buildimage.
#18825

Please can you do the needful so we can get this PR merged for May release.

Also we tried using the python-is-python3 both in dockers/docker-sonic-mgmt-framework/Dockerfile.j2 and sonic-slave-bookworm/Dockerfile.j2 But it was still failing for armhf with the same python error. Any pointers will be helpful.

@amrutasali
Copy link
Contributor

@saiarcot895 an update on above comment

sonic-net/sonic-gnmi#224 has been updated to take care of pipeline test cases too.

So instead of reverting the submodule update for mgmt-common (PR) the build issue for sonic-buildimage can be resolved by merging the following and making submodule update for these 2 repos :
sonic-net/sonic-mgmt-framework#141
sonic-net/sonic-gnmi#224

@saiarcot895
Copy link
Contributor

Got it, thanks @ranjinidn and @amrutasali!

Reverts of submodule updates are a bit difficult to do so given current automation, so it's better to make new commits in the submodules to fix the issue.

@saiarcot895
Copy link
Contributor

As for the armhf build error, it looks like the errors are in sonic-mgmt-framework and sonic-gnmi, both of which are related to go.mod and go.sum, so I'm thinking they would get handled by the above commits.

@ranjinidn ranjinidn force-pushed the bookworm_migration_for_mgmt_framework_and_gnmi branch from ded4126 to a689b08 Compare May 7, 2024 18:34
@ranjinidn ranjinidn force-pushed the bookworm_migration_for_mgmt_framework_and_gnmi branch from a689b08 to 99ec35e Compare May 8, 2024 15:16
@ranjinidn
Copy link
Contributor Author

@saiarcot895 The builds are passing but see some failures in the kvmtests. Not very clear what is the error. Will you please help retrigger this / understand these errors.

@ranjinidn
Copy link
Contributor Author

Thanks Sai for retriggering the tests, we still see a couple of kvmtest timeouts.
If you think this is not an issue and are random failures seen due to the setup environment, can you merge this PR?
The below 3 PRs for the individual repos are already merged for bookworm migration.
sonic-net/sonic-mgmt-common#133
sonic-net/sonic-mgmt-framework#132
sonic-net/sonic-gnmi#216

@saiarcot895
Copy link
Contributor

I can't approve/request a merge of this PR until all the checks are passing (unless there's a justifiable reason for the checks to fail).

It looks like the current timeouts are because it failed to lock a testbed in time, rather than any specific failure during the test execution. I've requested another rerun of those two test suites.

@ranjinidn
Copy link
Contributor Author

ranjinidn commented May 10, 2024

@saiarcot895 we have seen the kvm tests failing due to "Acquire testbed timeout" a few times and have been rebasing our branch to master to trigger a new build. Please can you let us know if there is a way in which we can retrigger the failing cases only. Looks like we don't have permission to retrigger only the failing test. Else each retrigger we will have to reach out to you.
It is important to push these changes for the May release as the mgmt-framwork and gnmi repos are already migrated for bookworm as mentioned earlier. Without this change the docker build for mgmt-framework and gnmi is broken currently.

Update: @saiarcot895 All builds and tests have passed. Now this can be merged so the build works for the above mentioned dockers.

# Install Bazel build system (amd64 and arm64 architectures are supported using this method)
# TODO(PINS): Remove once pre-build Bazel binaries are available for armhf (armv7l)
{%- if CONFIGURED_ARCH == "amd64" or CONFIGURED_ARCH == "arm64" %}
ARG bazelisk_url=https://github.com/bazelbuild/bazelisk/releases/latest/download/bazelisk-linux-{{ CONFIGURED_ARCH }}
RUN curl -fsSL -o /usr/local/bin/bazel ${bazelisk_url} && chmod 755 /usr/local/bin/bazel
# Bazel requires "python"
# TODO(PINS): remove when Bazel is okay with "python3" binary name
RUN apt install -y python-is-python3
#RUN apt install -y python-is-python3
Copy link
Collaborator

Choose a reason for hiding this comment

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

please remove it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Addressed

@ranjinidn
Copy link
Contributor Author

/azpw ms_conflict

@ranjinidn
Copy link
Contributor Author

/azpw run Azure.sonic-buildimage

@mssonicbld
Copy link
Collaborator

/AzurePipelines run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ranjinidn
Copy link
Contributor Author

@saiarcot895 we have a successful build for this PR. Please do merge this PR.

@ranjinidn
Copy link
Contributor Author

@lguohan please review and merge this PR.

@yxieca yxieca merged commit f4fc6ce into sonic-net:master May 16, 2024
19 checks passed
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.

8 participants