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

feat(agw): Run integ tests vs containerized AGW #13659

Merged
merged 1 commit into from
Aug 29, 2022

Conversation

voisey
Copy link
Contributor

@voisey voisey commented Aug 18, 2022

Summary

Works towards getting all integration tests running versus the containerized AGW, starting with test_attach_detach. Modifies system restarts to account for Docker. Also modifies the readme to explain the present situation and how to run the test, as well as adding sctpd to the services that need to be stopped before starting the Docker containers.

This aims to be a non-breaking PR which can be built upon in the future to get further tests running.

Test Plan

At present, test_attach_detach runs successfully if MAGMA_DEV_MODE is set to 1 in the .env file.

Notes:

@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines. label Aug 18, 2022
@voisey voisey force-pushed the run-integ-tests-vs-dockerised-agw branch from 6b623a2 to fed0dcf Compare August 18, 2022 16:28
@github-actions github-actions bot added component: agw Access gateway-related issue component: ci All updates on CI (Jenkins/CircleCi/Github Action) labels Aug 18, 2022
@github-actions
Copy link
Contributor

Thanks for opening a PR! 💯

A couple initial guidelines

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

If this is your first Magma PR, also consider reading

@voisey voisey linked an issue Aug 18, 2022 that may be closed by this pull request
5 tasks
@github-actions
Copy link
Contributor

github-actions bot commented Aug 18, 2022

feg-workflow

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

Results for commit 2242d2b.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 18, 2022

dp-workflow

13 tests   13 ✔️  2m 6s ⏱️
  1 suites    0 💤
  1 files      0

Results for commit 2242d2b.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 18, 2022

agw-workflow

615 tests   611 ✔️  3m 45s ⏱️
    2 suites      4 💤
    2 files        0

Results for commit 2242d2b.

♻️ This comment has been updated with latest results.

@voisey voisey marked this pull request as ready for review August 22, 2022 08:38
@voisey voisey requested a review from a team August 22, 2022 08:38
@voisey voisey requested a review from a team as a code owner August 22, 2022 08:38
@voisey voisey requested review from a team, nstng, rsarwad and jheidbrink August 22, 2022 08:38
@voisey voisey force-pushed the run-integ-tests-vs-dockerised-agw branch from fed0dcf to ae7bb8d Compare August 22, 2022 09:15
lte/gateway/docker/README.md Outdated Show resolved Hide resolved
lte/gateway/python/integ_tests/s1aptests/s1ap_utils.py Outdated Show resolved Hide resolved
lte/gateway/python/integ_tests/s1aptests/s1ap_utils.py Outdated Show resolved Hide resolved
lte/gateway/python/integ_tests/s1aptests/s1ap_utils.py Outdated Show resolved Hide resolved
lte/gateway/docker/.env Outdated Show resolved Hide resolved
@voisey voisey force-pushed the run-integ-tests-vs-dockerised-agw branch 3 times, most recently from deb8d6e to 7cf688e Compare August 22, 2022 14:57
@sebathomas sebathomas mentioned this pull request Aug 22, 2022
10 tasks
Copy link
Contributor

@jheidbrink jheidbrink left a comment

Choose a reason for hiding this comment

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

I confirmed that I can run test_attach_detach on this branch and it succeeds.

@voisey voisey force-pushed the run-integ-tests-vs-dockerised-agw branch 2 times, most recently from f7da167 to cf32c0b Compare August 24, 2022 12:50
@@ -25,6 +25,7 @@ x-generic-service: &service
logging: *logging_anchor
restart: always
network_mode: host
env_file: .env
Copy link
Contributor

Choose a reason for hiding this comment

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

might this line be redundant? is .env used by default?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The line was required to set the added environment variable in the containers, however the paths in .env were already being used in docker-compose.yaml as default. That said, this modification wasn't ideal so it has been replaced by the use of an override file, docker-compose.dev.yaml.

Copy link
Contributor

@nstng nstng left a comment

Choose a reason for hiding this comment

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

lgtm (modulo minor comment)

@voisey voisey force-pushed the run-integ-tests-vs-dockerised-agw branch from cf32c0b to 89f0189 Compare August 24, 2022 15:48
@pull-request-size pull-request-size bot added size/L Denotes a Pull Request that changes 100-499 lines. and removed size/M Denotes a PR that changes 30-99 lines. labels Aug 24, 2022
@voisey voisey force-pushed the run-integ-tests-vs-dockerised-agw branch 2 times, most recently from 8cbc576 to 030cff2 Compare August 24, 2022 15:51
@sebathomas
Copy link
Contributor

Could someone from @magma/approvers-agw-integtests have a look at this PR? It's the first step for integration tests of the dockerized AGW.

)
if self._init_system == InitMode.SYSTEMD:
self.exec_command(
"sudo service magma@* stop ; sudo service magma@magmad start",
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @sebathomas here it looks good as all services have restarted. Do we need similar handling for test case where an individual service have started. Say for test case, test_sctp_shutdown_while_mme_is_stopped.py mme, mobilityd, pipelined services have stopped and started mme service.
Please find the test case here, https://github.com/magma/magma/blob/master/lte/gateway/python/integ_tests/s1aptests/test_sctp_shutdown_while_mme_is_stopped.py

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for looking into this. Yes, I expect more changes will be necessary to run all tests, this will be handled in the follow-up ticket #13684.

Copy link
Contributor

@rsarwad rsarwad left a comment

Choose a reason for hiding this comment

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

S1ap tester changes looks good to me

@sebathomas
Copy link
Contributor

S1ap tester changes looks good to me

Can you approve the PR then? :)

Copy link
Contributor

@rsarwad rsarwad left a comment

Choose a reason for hiding this comment

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

s1ap changes LGTM

@sebathomas
Copy link
Contributor

Thank you! I think the PR needs a rebase now because of changes to the semantic-pr CI, but apart from that it's ready to merge.

@rsarwad
Copy link
Contributor

rsarwad commented Aug 26, 2022

Thank you! I think the PR needs a rebase now because of changes to the semantic-pr CI, but apart from that it's ready to merge.
Yes

Signed-off-by: Cameron Voisey <cameron.voisey@tngtech.com>
@voisey voisey force-pushed the run-integ-tests-vs-dockerised-agw branch from 030cff2 to 2242d2b Compare August 26, 2022 12:04
@voisey voisey merged commit 7cdd7e9 into magma:master Aug 29, 2022
rsarwad pushed a commit to rsarwad/magma that referenced this pull request Sep 4, 2022
Signed-off-by: Cameron Voisey <cameron.voisey@tngtech.com>

Signed-off-by: Cameron Voisey <cameron.voisey@tngtech.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: agw Access gateway-related issue component: ci All updates on CI (Jenkins/CircleCi/Github Action) size/L Denotes a Pull Request that changes 100-499 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Run integration test against containerized AGW
6 participants