-
Notifications
You must be signed in to change notification settings - Fork 592
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
fix(agw): restart mme as a single service for one single containerize… #14946
Conversation
Thanks for opening a PR! 💯
Howto
More infoPlease take a moment to read through the Magma project's
If this is your first Magma PR, also consider reading
|
9bf60d3
to
77b7e52
Compare
Oops! Looks like you failed the Howto
♻️ Updated: ✅ The check is passing the AGW Build & Format Python after the last commit. |
77b7e52
to
071a2bd
Compare
self._s1ap_wrapper.magmad_util.restart_services( | ||
|
||
# MME is explicitly restarted as a single service here, see comment | ||
# in the function. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not ideal to refer to a comment in another function at this line. If the comment in restart_single_service
ever goes away or changes, the comment in this file will be out of date. May be say instead e.g.
# MME is explicitly restarted as a single service here to avoid a race condition between the container restarts and the T3485 timer
# Since we hard code these intedependent container restarts for testing | ||
# only, one test is executed with only a MME restart to keep it green; | ||
# Test3485TimerForDefaultBearerWithMmeRestart. | ||
# |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we move this comment to the document string above? That way it is more easily visible, e.g. if a developer uses the quick documentation lookup feature in an IDE?
I successfully ran the tests locally and the changes LGTM. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm % comments from @MoritzThomasHuebner
…d AGW integ test Signed-off-by: Marco Pfirrmann <marco.pfirrmann@tngtech.com>
071a2bd
to
a7aeddf
Compare
…d AGW integ test (magma#14946) Signed-off-by: Marco Pfirrmann <marco.pfirrmann@tngtech.com>
…d AGW integ test
Signed-off-by: Marco Pfirrmann marco.pfirrmann@tngtech.com
Summary
Analyzing the failing S1AP integration tests based on a containerized AGW,
s1aptests/test_3485_timer_for_default_bearer_with_mme_restart.py
fails consistently. We suspect a race condition between the container restarts and the T3485 timer. For this test only, we only restart the MME container to mitigate the race condition.Background information:
In the systemd-base AGW, magma services do have a cascading restart mechanism: if one of them restarts, several other restart as well. This was mirrored in the S1AP test framework for the containerized AGW case. In production, every container is on its own. We now restart the mme container on its own for this particular test.
Test Plan
Additional Information