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

New API QOD Provisioning #299

Merged
merged 18 commits into from
Aug 7, 2024
Merged

Conversation

jlurien
Copy link
Collaborator

@jlurien jlurien commented Jun 5, 2024

What type of PR is this?

  • enhancement/feature

What this PR does / why we need it:

First draft of new QoD Provision API. It is a new API in the family of Quality On Demand. It implements the new mode approved to enhance the current scope:

  • Provision mode, indefinitely for each time the device connects to the same access network

Which issue(s) this PR fixes:

Fixes #268, #332

Special notes for reviewers:

  • Name proposal was originally "qod-provision" but it was agreed to use "qod-provisioning". Versions wip/vwip, to be changed to 0.1.0 (and its variants) for first RC

  • The PR just adds the yaml spec. Other files like README, documentation, etc may need to be updated once the API is reviewed and approved.

  • It follows closely the model for the sessions mode (renamed to quality-on-demand, final name TBD). Main differences are:

    • It does not consider duration and expiresAt, as provision is assumed to exist indefinitely, until it is revoked
    • It does not consider applicationServer, provisioning affects all device traffic
    • It assumes to reuses same qosProfiles than the session modes. We may review if it makes sense to have distinctions between the profiles applicable to each mode.
    • status replaces qosStatus in sessions, as it is the status of the configuration being applied, not the status of the QoS being in place at the moment. Configuration is active even if the device is temporarily not having a QoS benefit, for example because is disconnected or out of coverage.
    • A new endpoint to retrieve the provisions for a given device is modeled. This is equivalent to the new endpoint for session in PR List sessions endpoint for quality-on-demand #325
  • It already applies the latest alignment with Commonalities/ICM guidelines, and it is also aligned with Align quality-on-demand with Commonalties regarding optional device object and error messages #326

Changelog input

Initial version of QoD Provision mode API

First draft of QoD Provision API
Linting complaints
More linting fixes
More trailing spaces
@jlurien
Copy link
Collaborator Author

jlurien commented Jun 18, 2024

From the minutes of the last meeting:

@eric-murray: Does the API still allow to have multiple provisions per device? Or should it be restricted to one, and enhance the complete traffic of the device (would simplify the API a lot)
@jlurien : all traffic is only one possible use case (hence 'applicationServer' is proposed to be optional)
Open: Are there use cases to provision QoD for specific flows (defined by applicationServer)? - Jose will ask on product side

I took this AP to our Product team, and Eric's suggestion makes sense for this first version. So far, the initial use case is restricted to one provision per device, and affecting all of its traffic. Indeed, in case that it was required to include server side information for provision, we would likely need to use different identifiers for applicationServer, as server side IPs may change or they may be reused or shared by several applications (in a cloud server for example). Using AS domain names would make more sense. Even the original "session" mode may need to review these aspects in the future.

So I will simplify the original proposal accordingly. fyi @hdamker

* Removed support for serveral appliocation flows per device. In this version provision is per device and affects all its traffic. Proiperties `applicationServer`, `devicePorts` and `applicationPorts` are deleted.
* Also `messages` in the response is deleted as it had not a requirement
* Added support for async deletion process (202 response) in addition to the sync 204
Copy link
Collaborator

@RandyLevensalor RandyLevensalor left a comment

Choose a reason for hiding this comment

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

The more I read /provisions, it doesn't seem to capture the intent of this end point. Which if I'm understanding this endpoint correctly to intent is is to apply a persistent QoS profile to a device until it is removed.

Comment on lines 138 to 139
"404":
$ref: "#/components/responses/DeviceNotFound404"
Copy link
Collaborator

Choose a reason for hiding this comment

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

What about adding a second 404 response for when multiple devices are found?

        "404":
          oneOf:
            $ref: "#/components/responses/DeviceNotFound404"
            $ref: "#/components/responses/MultipleDevicesFound404"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Device Not Found is the only code considered in Commonalities for 404 related to devices. For cases when the device identifiers identify more than a single device, the response 422 DEVICE_IDENTIFIERS_MISMATCH applies

Copy link
Collaborator

Choose a reason for hiding this comment

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

@jlurien what is then the right code and message if the (single) provided identifier, e.g. Phone Number, belongs to multiple devices (e.g. "MultiCard"/"MultiSIM" features)? "DEVICE_IDENTIFIERS_MISMATCH" would be misleading here ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@hdamker I think that this case has not been considered yet in Commonalities. It is really a problem we have from the beginning when we identify a device by a MSISDN, which may not be exclusive for a single device, as it identifies a subscriber. IMO it would be a subcase for 422, as the entity is not processable for the service. The problem is also extensible to other APIs such as the sessions mode QoD or Device Location, when a single device is expected.

Copy link
Collaborator Author

@jlurien jlurien Jul 12, 2024

Choose a reason for hiding this comment

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

We still may use the generic 422 UNPROCESSABLE_ENTITY for this sub-case, or even DEVICE_NOT_SUPPORTED, but probably it is worthy to raise an issue to handle properly multi-sim scenarios

Copy link
Collaborator

Choose a reason for hiding this comment

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

@hdamker, @jlurien

If, by "multiSIM" you mean one MSISDN shared between multiple devices/SIMs (e.g. smartphone and smartwatch), this was discussed in Device Identifier, and the conclusion was that the phoneNumber should always be interpreted as the secondary MSISDN rather than primary.

The secondary MSISDN should be unique to a single device (SIM) - at least for the Vodafone "OneNumber" service it is, but maybe there are services where this is not true. For most devices, the secondary MSISDN will anyway be the same as the primary, and so this interpretation does not change things for such devices. The issue is that, if the secondary MSISDN is different, it is not so easy to find out what it is.

Of course, for Device Identifier this mattered, as the API is returning the IMEI of the UE, so we need to know which UE is intended. For QoS provisioning, maybe we just prioritise all devices that share the primary MSISDN.

- openId:
- "qod-provision:provisions:delete"

/provisions/retrieve:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would something like /provisions/active be a more intuitive to get the active provision on the current device.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I see that the description is wrong as it returns all provisions regardless the status. I'll have to fix that. And as it is a POST modeled to get information, the path is usually modeled as an action. A POST /resource/active would be seen as an endpoint to create a resource in active status.

"403":
$ref: "#/components/responses/Generic403"
"404":
$ref: "#/components/responses/RetrieveProvision404"
Copy link
Collaborator

Choose a reason for hiding this comment

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

What about a second response to 404 for device not found? Since this code implies that there isn't an active provision, which is technically true. There are no active provisions on a none existent device, but the implications are different.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The response (schema in components) already considers 2 codes: PROVISION_NOT_FOUND and DEVICE_NOT_FOUND. As I said above there is an error in the description and provision could be in any status.

- name: QoD Provision
description: Manage the provision of QoD
paths:
/provisions:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is provisions the best name for this endpoint? As I'm reviewing this, I keep thinking about getting supplies (provisions) for a camping trip.

What about /persistent or /device-qod? Something that denotes it's setting a QoS Provide for the device and / or that it's a type of QoD session that will never expire.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I trust a native speaker on this :) We are open to change the name to whatever makes the use case more understandable and remark the difference with respect to the other API (sessions). The concept is related to the provisioning of the QoD persistently.

For API name, maybe qod-provisioning is more clear? For the resource name it could be device-qos, and it would imply operations such as POST /qod-provisioning/v0.1/device-qos and GET /qod-provisioning/v0.1/device-qos/{device_qos_id}

Copy link
Collaborator

Choose a reason for hiding this comment

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

@jlurien /qod-provisioning makes sense to me.

- Replaced "provision" for "provisioning"
- Path for /provisions changed to /device-qos
- Fix that get operations do not only return active provision
- Trailing space
Copy link
Collaborator

@RandyLevensalor RandyLevensalor left a comment

Choose a reason for hiding this comment

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

@jlurien I'm still confused about one aspect of this end point / use case.

Since we are not specifying an application server, target port or any DCSP markings (which is a separate issue on the backlog), would this mean that all traffic would fall under this QoS profile or would the service provider just use this to update a default or predetermined flow/PDU session etc.?

If possible, could we add documentation as to how this impacts the traffic on the given device or just clarify that it's on per provider basis and may change based on type of service. I can't imagine that we'd want to change some of the standard QoS to just flatten the profiles on the device.

Co-authored-by: Randy Levensalor <randymartini@gmail.com>
@hdamker
Copy link
Collaborator

hdamker commented Jul 19, 2024

Since we are not specifying an application server

The PR description still mentions the optional ApplicationServer parameter but than was the API simplified after the discussion documented in #299 (comment).

I can't give a statement if this simplified API (but also the QoD Provisioning API as such) makes sense as product or not ... currently lacking feedback from product side.

@hdamker hdamker changed the title New API QOD Provision New API QOD Provisioning Jul 19, 2024
@jlurien
Copy link
Collaborator Author

jlurien commented Jul 25, 2024

@jlurien I'm still confused about one aspect of this end point / use case.

Since we are not specifying an application server, target port or any DCSP markings (which is a separate issue on the backlog), would this mean that all traffic would fall under this QoS profile or would the service provider just use this to update a default or predetermined flow/PDU session etc.?

If possible, could we add documentation as to how this impacts the traffic on the given device or just clarify that it's on per provider basis and may change based on type of service. I can't imagine that we'd want to change some of the standard QoS to just flatten the profiles on the device.

You're right, this means that the QoD profile will be applied to all the traffic sent to or received from the line under consideration. A relevant example would be the line for a point-of-sale terminal that will be configured with a better than best-effort service so that the credit card transactions will succeed even in crowded areas.

Revert required: true for request Body
@hdamker
Copy link
Collaborator

hdamker commented Aug 2, 2024

@hdamker please let me know if you miss some other task

@jlurien in addition you need to merge #330 (details can be changed within the release PR, but would be good if the file is already there)

- Updated implicit subscriptions model with Commonalities, issue camaraproject#332
- Error examples aligned with Commonalities (removed summary and some messages updated)
@jlurien jlurien requested a review from hdamker August 5, 2024 12:29
@jlurien
Copy link
Collaborator Author

jlurien commented Aug 5, 2024

Hi, @RandyLevensalor please review again, as you requested changes in a past review and now your explicit approval is needed to unblock the merge.

RandyLevensalor
RandyLevensalor previously approved these changes Aug 5, 2024
@RandyLevensalor RandyLevensalor self-requested a review August 5, 2024 14:40
code/API_definitions/qod-provisioning.yaml Outdated Show resolved Hide resolved
code/API_definitions/qod-provisioning.yaml Outdated Show resolved Hide resolved
Dedicated 400 response was in wrong place
@jlurien jlurien requested a review from hdamker August 6, 2024 12:41
Copy link
Collaborator

@hdamker hdamker left a comment

Choose a reason for hiding this comment

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

LGTM

@hdamker
Copy link
Collaborator

hdamker commented Aug 6, 2024

One point which I have seen in my final review: the reference to PROVISIONING_AVAILABLE_WITHOUT_DEVICE made me curios what is behind. Learned then that it is mainly "...WITHOUT_DEVICE_INFORMATION_IN_RESPONSE". Makes sense to have this example for OAS viewers/editors.

But it brought me to a point - feel free to create an issue or discussion out of it: is the provisioning really bound to a specific device? Or - in case of phoneNumber as the device identifier - is the provisioning actually bound to a subscription?

If the provisioning is bound to a device:

  • what is the expected behaviour if currently no device with the (valid) phoneNumber is registered with the network? Will the provisioning resource getting AVAILABLE and the improved QoS established when the/a device registers with the network?
  • in case the device was registered with the network when creating the provisioning ... what happens in case of a device swap with the provisioning? Should that be detected and the provisioning gets terminated by the network?

@jlurien
Copy link
Collaborator Author

jlurien commented Aug 6, 2024

One point which I have seen in my final review: the reference to PROVISIONING_AVAILABLE_WITHOUT_DEVICE made me curios what is behind. Learned then that it is mainly "...WITHOUT_DEVICE_INFORMATION_IN_RESPONSE". Makes sense to have this example for OAS viewers/editors.

I just added the example to illustrate that device can be empty in responses if not provided in the request (because it was implied from the 3-legged access token), but the other questions are relevant

But it brought me to a point - feel free to create an issue or discussion out of it: is the provisioning really bound to a specific device? Or - in case of phoneNumber as the device identifier - is the provisioning actually bound to a subscription?

If the provisioning is bound to a device:

  • what is the expected behaviour if currently no device with the (valid) phoneNumber is registered with the network? Will the provisioning resource getting AVAILABLE and the improved QoS established when the/a device registers with the network?
  • in case the device was registered with the network when creating the provisioning ... what happens in case of a device swap with the provisioning? Should that be detected and the provisioning gets terminated by the network?

I will share the questions with our Product team to fully discuss the corner cases, but the "happy case" designed so far, to my knowledge, is that device identifiers (explicitly provided or implied from token) are used to identify a valid subscription (typically associated to a SIM), and provisioning is applied to it. If may happen that SIM is not connected to the network at the time of the request if a phone number is provided, and my assumption is that provisioning should be allowed, but it may depend on the implementation. Allowing IP addresses enables the possibility to identify the device in cases where phone numbers are not known by the client, as with the session mode, but if an IP address is provided the device has to be connected.

If the SIM is swapped to a different terminal it shouldn't terminate the provisioning (IMO) but again it may be business decision to terminate it, if it is detected.

There may be some implications with multi-sim scenarios, but that kind of implications may affect as well the session mode, and it depends also on how multi-sim is implemented internally by operators.

For future releases we can open a dedicated issue to discuss all of these implications.

@hdamker
Copy link
Collaborator

hdamker commented Aug 6, 2024

@RandyLevensalor @eric-murray any last words on the PR? To complete the release PR I would need the API in main. For an initial version with 0.1.0 it is in quite good shape.

@hdamker
Copy link
Collaborator

hdamker commented Aug 7, 2024

@jlurien Eric had reviewed previously and the comment is addressed, @RandyLevensalor has approved before the last change which was only in the error examples. I think we are good here - would you merge the PR, so that I can finish the Release PR?

@hdamker hdamker mentioned this pull request Aug 7, 2024
5 tasks
@jlurien jlurien merged commit a434317 into camaraproject:main Aug 7, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provision mode for QoD
5 participants