chore: magma deb build is bazelified #13749

nstng · 2022-08-26T11:03:35Z

Summary

This change introduces a first throw for a magma debian build with bazel.

this approach tries to be as near as possible to the make build - but there are differences
- especially, the services are not packaged hermetically, i.e., common dependencies are only packaged once
far less dependencies are needed because bazel builds as hermetic as possible

known issues

the resulting package is very large (~900mb) and with --config=production even larger
there is some debug logging (basically warnings) during a build because files are overridden during packaging - these should only be generated proto files that are used by different services

Test Plan

build magma debian package on VM [edit] ~~or bazelBase/devcontainer~~ (the bazelBase/devcontainer sessiond services is currently not compatible with magma_deb)
- bazel build //lte/gateway/release:magma_deb_pkg
- add --config=production to be closer to the make debian build
- ~~note: in rebuild the vscode container setup because of recent needed changes (fix: bazel base uses same folly version as magma vm #13743)~~
copy the artifact to an accessible place[edit]: ~/magma$ cp bazel-bin/lte/gateway/release/magma_1.8.0_amd64.deb .
in lte/gateway/deploy/roles/magma_deploy/tasks/main.yml modify "Installing magma."
- # name: "{{ packages }}"
- add deb: "/home/vagrant/magma/magma_1.8.0_amd64.deb"
run ~/magma/lte/gateway$ vagrant up magma_deb
check magma_deb for issues, services running, logging, ...
integration tests can be run vs magma_deb

Additional Information

This change is backwards-breaking

github-actions · 2022-08-26T11:03:55Z

Thanks for opening a PR! 💯

A couple initial guidelines

All commits must be signed off. This is enforced by DCO check.
All PR titles must follow the semantic commits format. This is enforced by Semantic PR.

Howto

Reviews. The "Reviewers" listed for this PR are the Magma maintainers who will shepherd it.
Checks. All required CI checks must pass before merge.
Merge. Once approved and passing CI checks, use the ready2merge label to indicate the maintainers can merge your PR.

More info

Please take a moment to read through the Magma project's

Contributing Conventions for norms around contributed code

If this is your first Magma PR, also consider reading

Developer Onboarding for onboarding as a new Magma developer
Development Workflow for guidance on your first pull request
CI Checks for points of contact for failing or flaky CI checks
GitHub-to-Slack mappings for Magma maintainers for guidance on how to contact maintainers on Slack

lte/gateway/python/BUILD.bazel

github-actions · 2022-08-26T11:12:24Z

feg-workflow

    2 files 203 suites 40s ⏱️
374 tests 374 ✔️ 0 💤 0 ❌
388 runs 388 ✔️ 0 💤 0 ❌

Results for commit d500c9d.

♻️ This comment has been updated with latest results.

github-actions · 2022-08-26T11:27:55Z

dp-workflow

14 tests 14 ✔️ 3m 7s ⏱️
  1 suites   0 💤
  1 files   0 ❌

Results for commit d500c9d.

♻️ This comment has been updated with latest results.

github-actions · 2022-08-26T11:30:07Z

cloud-workflow

1 129 tests 1 129 ✔️ 2m 12s ⏱️
  368 suites       0 💤
      7 files       0 ❌

Results for commit d500c9d.

♻️ This comment has been updated with latest results.

github-actions · 2022-08-26T11:53:51Z

agw-workflow

615 tests 611 ✔️ 4m 30s ⏱️
    2 suites     4 💤
    2 files     0 ❌

Results for commit d500c9d.

♻️ This comment has been updated with latest results.

sebathomas · 2022-08-29T08:52:54Z

I just tried a build on my VM and I can't see the sctpd package described in the test plan.

vagrant@magma-dev:~/magma/bazel-bin/lte/gateway/release$ ls
magma_1.8.0_amd64.changes  magma_1.8.0_amd64.deb  magma_1.8.0_amd64.tar  magma_content.manifest  magma_deb_pkg.deb

nstng · 2022-08-29T09:23:44Z

I just tried a build on my VM and I can't see the sctpd package described in the test plan.
vagrant@magma-dev:~/magma/bazel-bin/lte/gateway/release$ ls
magma_1.8.0_amd64.changes  magma_1.8.0_amd64.deb  magma_1.8.0_amd64.tar  magma_content.manifest  magma_deb_pkg.deb

woops, c&p issue ... corrected in test plan.

lte/gateway/release/magma-postinst-bazel

sebathomas · 2022-08-29T14:51:24Z

"far less dependencies are needed because bazel builds as hermetic as possible" - I noticed that the list is a lot smaller than in the latest release from focal-ci. So the reason is that pydep adds many more dependencies than needed?

nstng · 2022-08-29T14:59:05Z

"far less dependencies are needed because bazel builds as hermetic as possible" - I noticed that the list is a lot smaller than in the latest release from focal-ci. So the reason is that pydep adds many more dependencies than needed?

Almost all python3-foo dependencies are not needed anymore. But iirc a make built magma.deb has all python3-foo dependencies twice (i.e., the list feels a lot longer). I do not know if pydep adds unnecessary dependencies.

sebathomas · 2022-08-29T15:02:54Z

From looking at the built magma.deb, the main difference in file size is because of the C++ services (mme, sessiond...). Is this because Bazel compiles all dependencies into the file or is there something else going on?

sebathomas

Still reviewing...

sebathomas · 2022-08-29T15:12:08Z

bazel/runfiles.bzl

+    "../com_google_protobuf",
+    # external grpc is only needed during compile time
+    "../com_github_grpc_grpc",
+    "_solib_k8/libexternal_Scom_Ugithub_Ugrpc_Ugrpc_Slibgrpc.so",


Maybe add a comment here too (whatever that thing is :)).

I will. The long description for the record: this is how bazel compiles dynamic libraries - separators like / and . are encoded -> libexternal/com.github.grpc.grpc/libgrpc.so. But it's still a library that is only needed during compile time.

sebathomas · 2022-08-29T15:25:37Z

lte/gateway/python/magma/kernsnoopd/BUILD.bazel

-        "ebpf/byte_count.bpf.c",
-        "ebpf/common.bpf.h",
-    ],
+    data = ["//lte/gateway/python/magma/kernsnoopd/ebpf:ebpf_data_files"],


These might be duplicated somehow. I see them both in

└── var ├── opt │ └── magma │ ├── ebpf │ │ ├── ebpf_dl_handler.c │ │ ├── ebpf_manager.py │ │ ├── ebpf_ul_handler.c │ │ └── kernsnoopd │ │ ├── byte_count.bpf.c │ │ └── common.bpf.h │ └── envoy.yaml

and usr/local/lib/python3.8

│ │ │ ├── kernsnoopd │ │ │ │ ├── ebpf │ │ │ │ │ ├── byte_count.bpf.c │ │ │ │ │ └── common.bpf.h │ │ │ │ ├── handlers.py │ │ │ │ ├── kernsnoopd │ │ │ │ ├── main.py │ │ │ │ ├── metrics.py │ │ │ │ └── snooper.py

nice catch - seems like this is inconsistent to the make magma.deb file. I will have a look ...

nils@nilstng:~/Downloads$ dpkg -c magma_1.8.0-1661850050-5221f975_amd64.deb | grep "var/opt/magma/ebpf" drwxr-xr-x 0/0 0 2022-08-30 11:05 ./var/opt/magma/ebpf/ -rw-r--r-- 0/0 3235 2022-08-30 09:09 ./var/opt/magma/ebpf/ebpf_ul_handler.c drwxr-xr-x 0/0 0 2022-08-30 11:05 ./var/opt/magma/ebpf/kernsnoopd/ -rw-r--r-- 0/0 2336 2022-08-30 09:09 ./var/opt/magma/ebpf/kernsnoopd/byte_count.bpf.c -rw-r--r-- 0/0 1042 2022-08-30 09:09 ./var/opt/magma/ebpf/kernsnoopd/common.bpf.h -rw-r--r-- 0/0 2730 2022-08-30 09:09 ./var/opt/magma/ebpf/ebpf_dl_handler.c -rw-r--r-- 0/0 991 2022-08-30 09:09 ./var/opt/magma/ebpf/BUILD.bazel -rw-r--r-- 0/0 12416 2022-08-30 09:09 ./var/opt/magma/ebpf/ebpf_manager.py nils@nilstng:~/Downloads$ dpkg -c magma_1.8.0-1661850050-5221f975_amd64.deb | grep "usr/local/lib/python3.8/dist-packages/magma/kernsnoopd" drwxrwxr-x 0/0 0 2022-08-30 11:05 ./usr/local/lib/python3.8/dist-packages/magma/kernsnoopd/ -rw-r--r-- 0/0 1465 2022-08-30 09:09 ./usr/local/lib/python3.8/dist-packages/magma/kernsnoopd/main.py -rw-r--r-- 0/0 853 2022-08-30 09:09 ./usr/local/lib/python3.8/dist-packages/magma/kernsnoopd/metrics.py -rw-r--r-- 0/0 7578 2022-08-30 09:09 ./usr/local/lib/python3.8/dist-packages/magma/kernsnoopd/handlers.py -rw-r--r-- 0/0 476 2022-08-30 09:09 ./usr/local/lib/python3.8/dist-packages/magma/kernsnoopd/__init__.py -rw-r--r-- 0/0 4271 2022-08-30 09:09 ./usr/local/lib/python3.8/dist-packages/magma/kernsnoopd/snooper.py

sebathomas · 2022-08-30T12:05:38Z

bazel/runfiles.bzl

+            # lte/protos/target_name/lte/protos/foo_pb2.py -> lte/protos/foo_pb2.py
+            return path.replace(prefix, "", 1).replace(file.owner.name + "/", "", 1)
+
+    print("Unhandle path: " + path)  # buildifier: disable=print


Nitpick:

Suggested change

print("Unhandle path: " + path) # buildifier: disable=print

print("Unhandled path: " + path) # buildifier: disable=print

sebathomas · 2022-08-30T12:10:19Z

bazel/runfiles.bzl

+# Contributed to https://github.com/aspect-build/rules_container under Apache-2.0
+
+"""
+This file represents a workaround for the current state of https://github.com/bazelbuild/rules_pkg.


Will it not be necessary any more in future versions?

I would hope so ... there are a couple of open upstream issues where this topic is discussed (iirc some are open for several years).

I think one argument is that python files should be pre-package, e.g., into a zip and then package the zip files. Problem here is that we have redundancy in our services. E.g., proto files (small) but also pip dependencies (and these sum up). This approach results in very large magma.debs (>2gb) ans also has other disadvantages during runtime (the zips are unpacked in a tmp folder - each time the service restarts).

LKreutzer

For me the tests

//lte/gateway/python/integ_tests/s1aptests:test_attach_service_with_multi_pdns_and_bearers_mt_data
//lte/gateway/python/integ_tests/s1aptests:test_attach_dl_udp_data

keep failing with an assertion error, can you reproduce this?

Unfortunately the standard query for deps, e.g. bazel query 'filter("(^[^@])", filter("^(?!.*external)", deps(//lte/gateway/release:magma_deb_pkg)))' --output graph | dot -Tsvg > magma_deb.svg does not work here. Is there a way to query the bazel structure of the package?

lte/gateway/release/magma-postinst-bazel

bazel/runfiles.bzl

lte/gateway/release/magma-postinst-bazel

lte/gateway/release/BUILD.bazel

nstng · 2022-09-05T13:47:33Z

Unfortunately the standard query for deps, e.g. bazel query 'filter("(^[^@])", filter("^(?!.*external)", deps(//lte/gateway/release:magma_deb_pkg)))' --output graph | dot -Tsvg > magma_deb.svg does not work here. Is there a way to query the bazel structure of the package?

@LKreutzer it works for me (it's just very very large and the resulting graph might not be a good visual aid) - what issue do you encounter?

nstng · 2022-09-05T19:31:57Z

For me the tests
* //lte/gateway/python/integ_tests/s1aptests:test_attach_service_with_multi_pdns_and_bearers_mt_data
* //lte/gateway/python/integ_tests/s1aptests:test_attach_dl_udp_data
keep failing with an assertion error, can you reproduce this?

@LKreutzer they were really unstable for me too - after restarting the trf server several times they became stable ¯\_(ツ)_/¯

SUMMARY: 2/2 tests were successful.
  //lte/gateway/python/integ_tests/s1aptests:test_attach_service_with_multi_pdns_and_bearers_mt_data: PASSED
  //lte/gateway/python/integ_tests/s1aptests:test_attach_dl_udp_data: PASSED

nstng · 2022-09-07T13:34:23Z

From looking at the built magma.deb, the main difference in file size is because of the C++ services (mme, sessiond...). Is this because Bazel compiles all dependencies into the file or is there something else going on?

@sebathomas by the last commit the size of the debian package was decreased from 930mb to 390mb. This was possible because before we added debug symbols not only to the magma binaries, but also to the external dependencies. we might want to do further analysis if the size can be optimized, but I suggest to do this as follow-up tasks.

.bazelrc

nstng · 2022-09-07T14:46:58Z

precommit tests on latest artifact:

vagrant@magma-test:~/magma$ ./bazel/scripts/run_integ_tests.sh --precommit
SUMMARY: 115/117 tests were successful.
  //lte/gateway/python/integ_tests/s1aptests:test_attach_ul_udp_data_with_mobilityd_restart: FAILED
  //lte/gateway/python/integ_tests/s1aptests:test_attach_detach_attach_dl_tcp_data: FAILED

vagrant@magma-test:~/magma$ ./bazel/scripts/run_integ_tests.sh --rerun-previously-failed
SUMMARY: 1/2 tests were successful.
  //lte/gateway/python/integ_tests/s1aptests:test_attach_ul_udp_data_with_mobilityd_restart: FAILED

vagrant@magma-test:~/magma$ ./bazel/scripts/run_integ_tests.sh --rerun-previously-failed
SUMMARY: 1/1 tests were successful.

LKreutzer

Lgtm

Bazel rule tests are working too

jheidbrink · 2022-09-08T11:19:58Z

lte/gateway/configs/BUILD.bazel

+pkg_mklink(
+    name = "pipelined_symlink",
+    link_name = "/etc/magma/pipelined.yml",
+    target = "/etc/magma/pipelined.yml_prod",


Could we just store pipelined.yml_prod at /etc/magma/pipelined.yml instead of symlinking?

Would be possible, yes - for now this is the behavior of the make created .deb.
I can only speculate: I assume the reason is/was, to have a convenient way to switch between prod and test setups.

.bazelrc

sebathomas · 2022-09-08T13:33:12Z

From looking at the built magma.deb, the main difference in file size is because of the C++ services (mme, sessiond...). Is this because Bazel compiles all dependencies into the file or is there something else going on?

@sebathomas by the last commit the size of the debian package was decreased from 930mb to 390mb. This was possible because before we added debug symbols not only to the magma binaries, but also to the external dependencies. we might want to do further analysis if the size can be optimized, but I suggest to do this as follow-up tasks.

Wow, that makes huge difference. Good catch!

Signed-off-by: Nils Semmelrock <nils.semmelrock@tngtech.com>

…ifacts is decreased) Signed-off-by: Nils Semmelrock <nils.semmelrock@tngtech.com>

nstng requested a review from a team August 26, 2022 11:03

nstng requested a review from a team as a code owner August 26, 2022 11:03

nstng requested review from LKreutzer and ajahl August 26, 2022 11:03

pull-request-size bot added the size/XXL Denotes a Pull Request that changes 1000+ lines. label Aug 26, 2022

nstng changed the title ~~Bazelify magma deb build~~ chore: magma deb build is bazelified Aug 26, 2022

github-actions bot added component: agw Access gateway-related issue component: ci All updates on CI (Jenkins/CircleCi/Github Action) component: feg FEG-gateway related issues component: orc8r Orchestrator-related issue labels Aug 26, 2022

github-actions bot reviewed Aug 26, 2022

View reviewed changes

lte/gateway/python/BUILD.bazel Show resolved Hide resolved

nstng linked an issue Aug 26, 2022 that may be closed by this pull request

magma.deb is bazelified #13416

Closed

3 tasks

nstng force-pushed the bazelify_magma_deb_build branch from 516e96a to a64156e Compare August 29, 2022 06:56

sebathomas reviewed Aug 29, 2022

View reviewed changes

lte/gateway/release/magma-postinst-bazel Show resolved Hide resolved

sebathomas reviewed Aug 29, 2022

View reviewed changes

sebathomas reviewed Aug 30, 2022

View reviewed changes

LKreutzer requested changes Sep 2, 2022

View reviewed changes

lte/gateway/release/magma-postinst-bazel Show resolved Hide resolved

bazel/runfiles.bzl Show resolved Hide resolved

lte/gateway/release/magma-postinst-bazel Outdated Show resolved Hide resolved

lte/gateway/release/BUILD.bazel Show resolved Hide resolved

nstng force-pushed the bazelify_magma_deb_build branch from a64156e to aeb3258 Compare September 5, 2022 13:35

github-actions bot removed the component: orc8r Orchestrator-related issue label Sep 7, 2022

LKreutzer requested changes Sep 7, 2022

View reviewed changes

.bazelrc Show resolved Hide resolved

nstng force-pushed the bazelify_magma_deb_build branch from 9c2458a to ca5e4d2 Compare September 7, 2022 14:08

nstng requested a review from LKreutzer September 7, 2022 14:36

LKreutzer approved these changes Sep 8, 2022

View reviewed changes

jheidbrink reviewed Sep 8, 2022

View reviewed changes

.bazelrc Show resolved Hide resolved

sebathomas approved these changes Sep 8, 2022

View reviewed changes

jheidbrink approved these changes Sep 8, 2022

View reviewed changes

nstng force-pushed the bazelify_magma_deb_build branch from ca5e4d2 to 4bf94bd Compare September 14, 2022 08:29

nstng added 14 commits September 14, 2022 16:41

chore: sctpd bazel build is cleaned-up

4d64061

Signed-off-by: Nils Semmelrock <nils.semmelrock@tngtech.com>

chore: skeleton for bazel magma deb build is added

d0760c4

Signed-off-by: Nils Semmelrock <nils.semmelrock@tngtech.com>

chore: service definitions are added

2a5fad3

Signed-off-by: Nils Semmelrock <nils.semmelrock@tngtech.com>

chore: ebpf files are added

6e69a03

Signed-off-by: Nils Semmelrock <nils.semmelrock@tngtech.com>

chore: c binaries are added

558f921

Signed-off-by: Nils Semmelrock <nils.semmelrock@tngtech.com>

chore: configuration files are added

03d36ed

Signed-off-by: Nils Semmelrock <nils.semmelrock@tngtech.com>

chore: misc configuration files are added

3c40054

Signed-off-by: Nils Semmelrock <nils.semmelrock@tngtech.com>

chore: python runfiles can be expanded for packaging

672726c

Signed-off-by: Nils Semmelrock <nils.semmelrock@tngtech.com>

chore: python scripts and dependencies are added

f7b19b1

Signed-off-by: Nils Semmelrock <nils.semmelrock@tngtech.com>

chore: go binary (envoy_controller) is added

7785f0e

Signed-off-by: Nils Semmelrock <nils.semmelrock@tngtech.com>

chore: python services are added

3162ccd

Signed-off-by: Nils Semmelrock <nils.semmelrock@tngtech.com>

chore: magma system dependencies are added

820dc32

Signed-off-by: Nils Semmelrock <nils.semmelrock@tngtech.com>

chore: post install script is added

1ca20ba

Signed-off-by: Nils Semmelrock <nils.semmelrock@tngtech.com>

chore: cpp debug symbols are only created for magma code (size of art…

d500c9d

…ifacts is decreased) Signed-off-by: Nils Semmelrock <nils.semmelrock@tngtech.com>

nstng force-pushed the bazelify_magma_deb_build branch from 4bf94bd to d500c9d Compare September 14, 2022 14:41

nstng merged commit 935783d into magma:master Sep 14, 2022

nstng mentioned this pull request Dec 27, 2022

Towards a Bazel-built AGW #12519

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: magma deb build is bazelified #13749

chore: magma deb build is bazelified #13749

nstng commented Aug 26, 2022 •

edited

github-actions bot commented Aug 26, 2022

github-actions bot commented Aug 26, 2022 •

edited

github-actions bot commented Aug 26, 2022 •

edited

github-actions bot commented Aug 26, 2022 •

edited

github-actions bot commented Aug 26, 2022 •

edited

sebathomas commented Aug 29, 2022

nstng commented Aug 29, 2022

sebathomas commented Aug 29, 2022

nstng commented Aug 29, 2022

sebathomas commented Aug 29, 2022

sebathomas left a comment

sebathomas Aug 29, 2022

nstng Sep 1, 2022

nstng Sep 5, 2022

sebathomas Aug 29, 2022

nstng Sep 1, 2022

nstng Sep 5, 2022

sebathomas Aug 30, 2022

nstng Sep 5, 2022

sebathomas Aug 30, 2022

nstng Sep 1, 2022

nstng Sep 1, 2022 •

edited

LKreutzer left a comment

nstng commented Sep 5, 2022

nstng commented Sep 5, 2022 •

edited

nstng commented Sep 7, 2022

nstng commented Sep 7, 2022

LKreutzer left a comment

jheidbrink Sep 8, 2022 •

edited

nstng Sep 8, 2022

sebathomas commented Sep 8, 2022

	print("Unhandle path: " + path) # buildifier: disable=print
	print("Unhandled path: " + path) # buildifier: disable=print

chore: magma deb build is bazelified #13749

chore: magma deb build is bazelified #13749

Conversation

nstng commented Aug 26, 2022 • edited

Summary

known issues

Test Plan

Additional Information

github-actions bot commented Aug 26, 2022

Howto

More info

github-actions bot commented Aug 26, 2022 • edited

feg-workflow

github-actions bot commented Aug 26, 2022 • edited

dp-workflow

github-actions bot commented Aug 26, 2022 • edited

cloud-workflow

github-actions bot commented Aug 26, 2022 • edited

agw-workflow

sebathomas commented Aug 29, 2022

nstng commented Aug 29, 2022

sebathomas commented Aug 29, 2022

nstng commented Aug 29, 2022

sebathomas commented Aug 29, 2022

sebathomas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nstng Sep 1, 2022 • edited

Choose a reason for hiding this comment

LKreutzer left a comment

Choose a reason for hiding this comment

nstng commented Sep 5, 2022

nstng commented Sep 5, 2022 • edited

nstng commented Sep 7, 2022

nstng commented Sep 7, 2022

LKreutzer left a comment

Choose a reason for hiding this comment

jheidbrink Sep 8, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebathomas commented Sep 8, 2022

nstng commented Aug 26, 2022 •

edited

github-actions bot commented Aug 26, 2022 •

edited

github-actions bot commented Aug 26, 2022 •

edited

github-actions bot commented Aug 26, 2022 •

edited

github-actions bot commented Aug 26, 2022 •

edited

nstng Sep 1, 2022 •

edited

nstng commented Sep 5, 2022 •

edited

jheidbrink Sep 8, 2022 •

edited