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

chore(agw): adapt gateway_subscriber_state format #13212

Merged

Conversation

alexzurbonsen
Copy link
Contributor

@alexzurbonsen alexzurbonsen commented Jul 6, 2022

Summary

To fix a deserializing error that occurred in an e2e test with agw integ tests, the outer array in the gateway_subscriber_state is removed.

Test Plan

Going back to the example from the original implementation, we basically change the following to make sure the entries for each IMSI exactly match the previous split subscriber_state:

{
    "subscribers": {
-        "IMSI001010000000002": [
+        "IMSI001010000000002": {
            {
                "magma.ipv4": [
                    {
                        "active_policy_rules": [
                            "{\"id\":\"allowlist_sid-IMSI001010000000002-magma.ipv4\",\"priority\":2,\"flowList\":[{\"match\":{}},{\"match\":{\"direction\":\"DOWNLINK\"}}],\"trackingType\":\"NO_TRACKING\"}"
                        ],
                        "active_duration_sec": 129,
                        "lifecycle_state": "SESSION_ACTIVE",
                        "session_start_time": 1654872554,
                        "apn": "magma.ipv4",
                        "ipv4": "192.168.128.13",
                        "msisdn": "",
                        "session_id": "IMSI001010000000002-320023"
                    }
                ]
            }
-        ],
-        "IMSI001010000000001": [
+        },
+        "IMSI001010000000001": {
            {
                "magma.ipv4": [
                    {
                        "active_policy_rules": [
                            "{\"id\":\"allowlist_sid-IMSI001010000000001-magma.ipv4\",\"priority\":2,\"flowList\":[{\"match\":{}},{\"match\":{\"direction\":\"DOWNLINK\"}}],\"trackingType\":\"NO_TRACKING\"}"
                        ],
                        "active_duration_sec": 129,
                        "lifecycle_state": "SESSION_ACTIVE",
                        "session_start_time": 1654872554,
                        "apn": "magma.ipv4",
                        "ipv4": "192.168.128.12",
                        "msisdn": "",
                        "session_id": "IMSI001010000000001-612699"
                    }
                ]
            }
-        ]
+        }
    }
}

Additional Information

  • Done in pairing with @sebathomas.

  • This change is backwards-breaking

@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines. label Jul 6, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jul 6, 2022

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

@github-actions github-actions bot added the component: agw Access gateway-related issue label Jul 6, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jul 6, 2022

feg-workflow

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

Results for commit 75721db.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 6, 2022

dp-workflow

18 tests   18 ✔️  5m 9s ⏱️
  2 suites    0 💤
  2 files      0

Results for commit 75721db.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 6, 2022

agw-workflow

580 tests   576 ✔️  4m 56s ⏱️
    2 suites      4 💤
    2 files        0

Results for commit 75721db.

♻️ This comment has been updated with latest results.

@pull-request-size pull-request-size bot added size/M Denotes a PR that changes 30-99 lines. and removed size/XS Denotes a PR that changes 0-9 lines. labels Jul 6, 2022
@sebathomas sebathomas requested a review from ajahl July 6, 2022 15:21
alexzurbonsen and others added 2 commits July 7, 2022 10:36
to fix deserializing error

Signed-off-by: Alexander zur Bonsen <alexander.zur.bonsen@tngtech.com>
Signed-off-by: Sebastian Thomas <sebastian.thomas@tngtech.com>
@sebathomas sebathomas force-pushed the sessiond/modify-gw-subscriber-state branch from cc600eb to 75721db Compare July 7, 2022 08:37
@sebathomas sebathomas marked this pull request as ready for review July 7, 2022 08:55
@sebathomas sebathomas requested review from a team and uri200 July 7, 2022 08:55
@sebathomas
Copy link
Contributor

@magma/approvers-agw-sessiond Could someone take a look at this small PR if you get the chance? Thank you.

@themarwhal
Copy link
Member

@magma/approvers-agw-sessiond Could someone take a look at this small PR if you get the chance? Thank you.

I'm not sure if we have any notifications set up for teams, so you might get a faster response if you tag directly in the future :)

@sebathomas
Copy link
Contributor

@magma/approvers-agw-sessiond Could someone take a look at this small PR if you get the chance? Thank you.

I'm not sure if we have any notifications set up for teams, so you might get a faster response if you tag directly in the future :)

Good to know, and thank you for the review!

@sebathomas sebathomas merged commit 228a655 into magma:master Jul 12, 2022
emakeev pushed a commit to emakeev/magma that referenced this pull request Aug 5, 2022
* chore(agw): remove outer array from gateway_subscriber_state to make
to fix deserializing error

Signed-off-by: Alexander zur Bonsen <alexander.zur.bonsen@tngtech.com>

* chore(agw): Adapt test case for get_operational_states

Signed-off-by: Sebastian Thomas <sebastian.thomas@tngtech.com>

Co-authored-by: Sebastian Thomas <sebastian.thomas@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 size/M Denotes a PR that changes 30-99 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants