chore: Fix scripts for Bazel integration tests #13978

vktng · 2022-09-21T09:50:31Z

Summary

Some integration tests are not properly working with Bazel because of not reachable scripts. these are

test_enable_ipv6_iface
test_ipv6_paging_with_dedicated_bearer
test_ipv4v6_paging_with_dedicated_bearer
This PR fixes these issues.

Test Plan

CI.

Additional Information

This change is backwards-breaking

github-actions · 2022-09-21T09:50:45Z

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

github-actions · 2022-09-21T09:52:52Z

Oops! Looks like you failed the Python Format Check.

Howto

Instructions on running the formatter and linter locally are provided in the format AGW doc
Guide to the different CI checks and resolution guidelines

♻️ Updated: ✅ The check is passing the Python Format Check after the last commit.

jheidbrink · 2022-09-21T09:56:57Z

bazel/scripts/check_py_bazel.sh

@@ -58,29 +58,30 @@ DENY_LIST_NOT_YET_BAZELIFIED=(
  "./lte/gateway/python/integ_tests/cloud_tests"
  "./lte/gateway/python/integ_tests/federated_tests"
  "./lte/gateway/python/integ_tests/s1aptests/workflow"
+  "./lte/gateway/python/integ_tests/s1aptests/test_enable_ipv6_iface.py"


FYI: This just changes the order of the files, no files added or removed.

mpfirrmann · 2022-09-21T10:00:02Z

Was this just a Bazel issue or did it affect the tests on the make-build AGW as well?

github-actions · 2022-09-21T10:00:10Z

feg-workflow

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

Results for commit f1ec46b.

♻️ This comment has been updated with latest results.

github-actions · 2022-09-21T10:21:37Z

dp-workflow

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

Results for commit f1ec46b.

♻️ This comment has been updated with latest results.

github-actions · 2022-09-21T10:51:55Z

agw-workflow

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

Results for commit f1ec46b.

♻️ This comment has been updated with latest results.

nstng · 2022-09-21T10:58:35Z

lte/gateway/python/integ_tests/s1aptests/s1ap_utils.py

@@ -60,7 +60,8 @@
 from orc8r.protos.directoryd_pb2_grpc import GatewayDirectoryServiceStub

 DEFAULT_GRPC_TIMEOUT = 10
-
+MAGTIVATE_CMD = "source /home/vagrant/build/python/bin/activate"


This is just a refactoring as these values were used in various places.

nstng · 2022-09-21T11:02:03Z

lte/gateway/python/scripts/config_iface_for_ipv6.py

@@ -21,74 +21,34 @@



Reasons for the refactorings:

the script was using sys.exit for communication that values were already changed - especially the return code 1 did basically "override" the return code 1 if the script was not found

in general sys.exit > 0 should only be used for errors

it does not seem relevant for the usage if the config values already had the desired values

-> do not check the current values and always override

nstng

lgtm % py formatting fixes

respective tests (in make based workflow) did run successfully with this change - see https://github.com/nstng/magma/actions/runs/3091802252

nstng · 2022-09-21T12:35:55Z

Was this just a Bazel issue or did it affect the tests on the make-build AGW as well?

@mpfirrmann it was just an issue with bazel - the solution for icmpv6.py (affects test_ipv6_paging_with_dedicated_bearer.py and test_ipv4v6_paging_with_dedicated_bearer.py) did still work for the make workflow on fork runs.

Signed-off-by: Krisztián Varga <krisztian.varga@tngtech.com>

…ap_utils.py) Signed-off-by: Krisztián Varga <krisztian.varga@tngtech.com>

Signed-off-by: Krisztián Varga <krisztian.varga@tngtech.com>

sebathomas

Looking good to me, just a few questions.

sebathomas · 2022-09-21T13:44:20Z

lte/gateway/python/integ_tests/s1aptests/s1ap_utils.py

        else:
-            print("Failed to configure")
+            raise Exception("Failed to configure the IPV6 interface!")


This is to make the test fail early?

to make it fail at all - the test was not failing if the script was not executed successfully

See also #13982 why it would be good for this step to fail if the script has problems - it was only visible by info logging that the script was not executed.

sebathomas · 2022-09-21T13:44:53Z

lte/gateway/python/integ_tests/s1aptests/s1ap_utils.py

        execute_icmpv6_cmd = (
-            "sudo /home/vagrant/build/python/bin/python3 "
-            + "/home/vagrant/magma/lte/gateway/python/scripts/icmpv6.py "
+            MAGTIVATE_CMD


It looks like this didn't activate the venv before, but I assume that's no problem?

It was not done before and in the test runs it was not necessary. Assumption is that it is not needed for this script.

sebathomas · 2022-09-21T13:45:33Z

lte/gateway/python/scripts/BUILD.bazel

+    imports = [ORC8R_ROOT],
+    legacy_create_init = False,
+    deps = ["//orc8r/gateway/python/magma/configuration:service_configs"],
+)


This is the main part of the fix?

Yes, now the script is build with bazel and can be used by the integration tests.

VinashakAnkitAman

LGTM

vktng added component: ci All updates on CI (Jenkins/CircleCi/Github Action) bazel changes for the Bazelification effort labels Sep 21, 2022

vktng requested review from jheidbrink and nstng September 21, 2022 09:50

vktng self-assigned this Sep 21, 2022

vktng requested review from a team September 21, 2022 09:50

vktng requested a review from a team as a code owner September 21, 2022 09:50

vktng requested review from VinashakAnkitAman and ardzoht September 21, 2022 09:50

pull-request-size bot added the size/L Denotes a Pull Request that changes 100-499 lines. label Sep 21, 2022

github-actions bot added component: agw Access gateway-related issue and removed component: ci All updates on CI (Jenkins/CircleCi/Github Action) labels Sep 21, 2022

jheidbrink reviewed Sep 21, 2022

View reviewed changes

jheidbrink approved these changes Sep 21, 2022

View reviewed changes

This was referenced Sep 21, 2022

chore: fix unstable integration tests #13955

Closed

Track LTE Integ Flakiness #13954

Open

Stabilize bazel lte integ tests #13963

Closed

nstng reviewed Sep 21, 2022

View reviewed changes

nstng approved these changes Sep 21, 2022

View reviewed changes

vktng added 2 commits September 21, 2022 15:26

chore: Clean up check_py_bazel.sh

40fa5d9

Signed-off-by: Krisztián Varga <krisztian.varga@tngtech.com>

chore: Refactor ipv6 interface config script (including failure in s1…

a528d3a

…ap_utils.py) Signed-off-by: Krisztián Varga <krisztian.varga@tngtech.com>

vktng added 2 commits September 21, 2022 15:26

chore: Bazelify the config_iface_for_ipv6.py script

c3d7e54

Signed-off-by: Krisztián Varga <krisztian.varga@tngtech.com>

chore: Fix path for icmpv6.py (use /usr/local/bin)

f1ec46b

Signed-off-by: Krisztián Varga <krisztian.varga@tngtech.com>

vktng force-pushed the fix_scripts_for_bazel_integ_tests branch from 1341a0e to f1ec46b Compare September 21, 2022 13:26

nstng mentioned this pull request Sep 21, 2022

fix: magma_deb integ tests can use config_iface_for_ipv6.py script #13982

Merged

1 task

sebathomas reviewed Sep 21, 2022

View reviewed changes

sebathomas approved these changes Sep 21, 2022

View reviewed changes

VinashakAnkitAman approved these changes Sep 23, 2022

View reviewed changes

nstng merged commit f951954 into magma:master Sep 23, 2022

wolfseb mentioned this pull request Oct 5, 2022

fix(agw): fix agw ipv6 paging integ tests #14084

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Fix scripts for Bazel integration tests #13978

chore: Fix scripts for Bazel integration tests #13978

vktng commented Sep 21, 2022

github-actions bot commented Sep 21, 2022

github-actions bot commented Sep 21, 2022 •

edited

jheidbrink Sep 21, 2022 •

edited

mpfirrmann commented Sep 21, 2022

github-actions bot commented Sep 21, 2022 •

edited

github-actions bot commented Sep 21, 2022 •

edited

github-actions bot commented Sep 21, 2022 •

edited

nstng Sep 21, 2022

nstng Sep 21, 2022 •

edited

nstng left a comment •

edited

nstng commented Sep 21, 2022

sebathomas left a comment

sebathomas Sep 21, 2022

nstng Sep 21, 2022

nstng Sep 21, 2022

sebathomas Sep 21, 2022

nstng Sep 21, 2022

sebathomas Sep 21, 2022

nstng Sep 21, 2022

VinashakAnkitAman left a comment

chore: Fix scripts for Bazel integration tests #13978

chore: Fix scripts for Bazel integration tests #13978

Conversation

vktng commented Sep 21, 2022

Summary

Test Plan

Additional Information

github-actions bot commented Sep 21, 2022

Howto

More info

github-actions bot commented Sep 21, 2022 • edited

Howto

jheidbrink Sep 21, 2022 • edited

Choose a reason for hiding this comment

mpfirrmann commented Sep 21, 2022

github-actions bot commented Sep 21, 2022 • edited

feg-workflow

github-actions bot commented Sep 21, 2022 • edited

dp-workflow

github-actions bot commented Sep 21, 2022 • edited

agw-workflow

Choose a reason for hiding this comment

nstng Sep 21, 2022 • edited

Choose a reason for hiding this comment

nstng left a comment • edited

Choose a reason for hiding this comment

nstng commented Sep 21, 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

VinashakAnkitAman left a comment

Choose a reason for hiding this comment

github-actions bot commented Sep 21, 2022 •

edited

jheidbrink Sep 21, 2022 •

edited

github-actions bot commented Sep 21, 2022 •

edited

github-actions bot commented Sep 21, 2022 •

edited

github-actions bot commented Sep 21, 2022 •

edited

nstng Sep 21, 2022 •

edited

nstng left a comment •

edited