-
Notifications
You must be signed in to change notification settings - Fork 3
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
Connection request should allow user to specify which VLAN to be used on ingress and egress #232
Comments
Yes. The pce core function handles these different VLAN combinations. Some code might be needed on the front. The 'requesthandler' class was meant to properly handle the different request combinations. We'll create a meta issue with a list of these individual scenario. As I mentioned in an earlier message: we need a template/spec for the connection response that works for Meican. The 'connection' object itself has an optional 'path' field that actually could show all the internal VLANs-:) |
Hi @YufengXin can you please provide an update if the VLAN ID is already supported by SDX-Controller connection creation request? As per the swagger API spec, it does not seems to support: |
@italovalcy The port is an object here, which could contain the user specified VLANs. |
atlanticwave-sdx/pce#177 ("Update getting port when breakdown") is related, right? The PR was made when I was away, so I'm not quite up to speed on that. I guess we should get it merged first? |
Also here's sdx-controller/sdx_controller/swagger/swagger.yaml Lines 710 to 743 in b931068
And here's an example connection request: {
"id": "test-connection-request",
"name": "Test connection request",
"start_time": "2000-01-23T04:56:07.000Z",
"end_time": "2000-01-23T04:56:07.000Z",
"bandwidth_required": 10,
"latency_required": 300,
"egress_port": {
"id": "urn:sdx:port:amlight.net:A1:1",
"name": "Novi100:1",
"node": "urn:sdx:node:amlight.net:A1",
"status": "up"
},
"ingress_port": {
"id": "urn:ogf:network:sdx:port:zaoxi:A1:2",
"name": "Novi100:2",
"node": "urn:ogf:network:sdx:node:zaoxi:A1",
"status": "up"
}
} What would a request that carries a VLAN ID look like? I don't believe that Topology Data Model Specification specifies a format/schema for connection requests. |
@sajith I think we reused the 'label' and 'label_range' fields in the port object for the connection request, ie, 'label' for the type, vlan in this case, and 'vlan_range' for the value. See this test json in data model. That's why I remembered this was tested already. @italovalcy we'll use your test request to test. But there is no specification doc for request yet. I started with the json schema like others and tried to reuse as much as possible. We can use your schema/convention. |
(related datamodel #102, pce issue#179): on user specified vlans on ports. The current parser and provisioning in datamodel and pce have implemented functions to process both port.vlan_range and port.services.l2vpn-ptp.vlan_range, aka user specified vlan function. The idea is to use port.vlan_range for the user specified vlans in the connection request, and port.services.l2vpn-ptp.vlan_range for the available vlan ranges in the OXP's topology.json. But we need to finalize/test the whole process. To kick it off, I started a draft on the request connection Specification draft in the project Google drive: https://docs.google.com/document/d/1vMKn5Mjf404BPJIilBcs2VpikdzOxS1jTep4kdJ1_0I/edit The workflow: spec doc-> json schema->parser->unittest->swagger.yaml->sdx-doc. (Note: for this version, we should just get the basic done, then we can iterate and add new requirements in the request) |
Hi @YufengXin and @sajith I tried to use the label and vlan_range as in the data model request example above, but it didnt seems to have any effect on the sdx-controller:
|
@italovalcy Thanks for testing this. could you share the OXP topology json you used? Specifically, what are the vlan_range specified in these two ports' services component? |
Hi @YufengXin sure thing! Here is is:
|
@italovalcy As I can see, the vlan_range is [1-100] and you were asking vlan 107 in your request. 107 is outside of the OXP available vlan range, in which case, the current logic would just pick an available one to assign to your request. A while ago, we agreed to retire the 'sdx-continuous-development' repo. Now you made it work, we could look at it again. (2) For the timebeing, we only need to run unittest for debugging, would you be able to save these three OXP topology.json and check them in to the tests/data in 'datamodel' repo? Thanks. |
@italovalcy BTW, in your testing environment, are you using the main branch for sdx-controller and sdx-lc? |
Hi @YufengXin actually, if you see the requests I created two tests: 1) first using VLAN 107, which is outside of the range allowed from the OXP; 2) I created a second test using VLAN 49, inside the VLAN range and still got assigned VLAN 100.... For situation 1, the SDX-Controller has to reply saying that the requested VLAN is not allowed, instead of creating a VLAN of the choice of the SDX-COntroller. For situation 2, the SDX-Controller just ignored the request from the user and assigned a VLAN of its choice again..
In this case you just need to run the same steps as I did before and you should be able to get the JSONs! ;) |
I'm using main branch. |
Thanks, @italovalcy. I'll leave the testing environment to Mert. (I see both submodules were one month old.) @sajith Could you please take a look at this issue? I think our unittest json tried to cover both in- and out- vlan range cases, please double check and fix. Thx. |
Hi Yufeng,
Yes, good point! Indeed, on the procedure above I just ignored the submodules and cloned directly from upstream. See the specific part here:
|
It would be even better if I see that using those docker images is an item in atlanticwave-sdx/sdx-continuous-development#106. Is the plan to resurrect that repo? How quick/easy would it be to use it for local testing today? |
Since submodules are not used anyway and the Kytos NAPP now works, let Mert and Cong try to duplicate the setup first. Then move to issue #247 on using these images. This issue is on itself, let's get an unittest case work in duplicating Italo's test. |
Hi team, any updates on this issue? |
Not yet. I will get to this, but not sure when. |
@italovalcy I think you relayed Jeronimo's opinion to hold on this issue, because it depends on the request specification and examples. Anyway, I made the move, making changes in 'datamdoel' and 'sdx-lc' first , that include cleaning up and streamline the schema and the code, after turning on the function to parse the 'vlan_range' attribute moved from the port object to the port.services. And as I explained to you, any changes in the datamodel will trigger the chain reaction: datamodel<->sdx-lc<->pce and finally here in sdx-controller. I have two PRs in datamodel and sdx-lc that have completed the unittest. The next step is to conduct the system tests with sdx-lc and sdx-controller before merging. |
Hi Yufeng,
Thanks for looking into this. Actually, what I mentioned was regarding the openapi spec on Kytos SDX topology, that task was put on hold to wait for the final connection spec from Jeronimo. However, I think we have all we need from the current spec: https://github.com/atlanticwave-sdx/datamodel/blob/main/src/sdx_datamodel/data/requests/test_request_p2p.json#L13 We even discussed about this in two meetings ago (on the meeting 2024-05-01) and we talked about using that data model spec the way it was and then later on we adapt to comply with the final connection model spec. |
Hi,
It is interesting that the connection request should allow the user to define which VLAN ID he/she wants to use when requesting a circuit (ingress and egress, which may be different by the way).
Choosing the user VLAN does not relate to VLAN IDs used for inter-domain connections. The latter will be automatically chosen from the SDX Controller / PCE.
In the meantime, it would be nice if the GET /connection displays which VLAN was chose for ingress and egress from the user request (again, only on user ports, not inter-OXP ports)
The text was updated successfully, but these errors were encountered: