atlanticwave-sdx / sdx-controller

Central Controller for AtlanticWave SDX.
https://www.atlanticwave-sdx.net
MIT License
1 stars 3 forks source link

Need an API to update existing connection (and check the request spec 2.0) #259

Closed congwang09 closed 3 weeks ago

congwang09 commented 5 months ago

Per our discussions here, we should change the POST /connection API to PUT /connection API. That way user (MEICAN) can send SDX controller a connection, if the connection exists, it will be a update operation; otherwise, it will be a add connection operation.

@YufengXin @sajith Would updating POST to PUT make sense here, or if we should add a separate API for updating existing connection?

YufengXin commented 5 months ago

Ideally we should have both PUT (existing connection) and POST (new connection).

'id' is the problem: ie, (1) whenever 'post' a connection request, even the A point and Z point are the same, a new connection resource should be created, with a new 'id', by the server. (2) the 'put' would just update the object of specific 'id'.

if the object/resource is only indexed by a 'connection_id' provided by the client, then 'post' is not needed anymore, I guess?

So, the server side code in the controller' needs to be considered for this? esp. how is the 'id' used in the database when you retrieve the particular object? ie, the {id} used in the 'put' endpoint should be the same 'id' key in the database.

YufengXin commented 5 months ago

the endpoint for post is normally POST https://{end point}/, the put is PUT https://{end point}/{id}

congwang09 commented 5 months ago

the endpoint for post is normally POST https://{end point}/, the put is PUT https://{end point}/{id}

The connection_id is included in the request, this {id} field seems redundant? But yeah, this {id} should make the PUT API idempotent.

YufengXin commented 3 months ago

https://docs.google.com/document/d/1YKIvNgOaoSA30coT0hvhjVGb8KSu5bNxz3fbfdOBdUY/edit?usp=sharing

Repurpose this issue to also check/change to the recent request spec 2.

Looks like PATCH /connection/{id} would be needed, though it's not very clear what action needs to be.

May need to split into smaller issues.

sajith commented 3 months ago

For reference, the request format will be of this form:

{
  "name": "new-connection",
  "endpoints": [
    {
      "port_id": "urn:sdx:port:ampath.net:Ampath3:50",
      "vlan": "777"
    },
    {
      "port_id": "urn:sdx:port:sax.net:Sax01:41",
      "vlan": "55:90"
    },
    {
      "port_id": "urn:sdx:port:ampath.net:Ampath3:50",
      "vlan": "untagged"
    }
  ],
  "description": "a test circuit",
  "scheduling": {
    "start_time": "2024-06-24T01:00:00.000Z",
    "end_time": "2024-06-26T01:00:00.000Z"
  },
  "qos_metrics": {
    "min_bw": {
      "value": 12,
      "strict": true
    },
    "max_delay": {
      "value": 4,
      "strict": false
    },
    "max_number_oxps": {
      "value": 7,
      "strict": true
    }
  },
  "notifications": [
    {
      "email": "john.doe@example.net"
    }
  ]
}

Also see test-l2vpn-p2p-v2.json.

YufengXin commented 3 weeks ago

resolved in PR: https://github.com/atlanticwave-sdx/sdx-controller/pull/324