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
Add GET operations for DeviceGroups and Network Slices #81
Add GET operations for DeviceGroups and Network Slices #81
Conversation
Previously, device groups and slices were stored only in memory and used to detect devices added or removed. This was susceptible to cause issue whenever the webconsole service restarted, losing some context. This change will also allow the addition of GET operations for DeviceGroups and NetworkSlices.
Can one of the admins verify this patch? |
1 similar comment
Can one of the admins verify this patch? |
add to whiltelist |
ok to test |
@thakurajayL my org has now signed the CLA, this should be ready for review, sorry it took that long. |
add to whitelist |
ok to test |
@@ -0,0 +1,21 @@ | |||
# SPDX-License-Identifier: Apache-2.0 | |||
# Copyright 2022 Intel Corporation |
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.
May be add ONF copyright
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.
I added the copyright.
Dockerfile
Outdated
@@ -3,7 +3,7 @@ | |||
# SPDX-License-Identifier: Apache-2.0 | |||
# | |||
|
|||
FROM golang:1.16.0-stretch AS builder | |||
FROM golang:1.16.0-buster AS builder |
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.
How is buster different than stretch ? You can think of updating golang as well.
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.
Stretch is debian 9, which is an archived release under Extended LTS support. Buster is Debian 10, and it is under regular LTS support. The current stable release would be Bookworm, or Debian 12. I will test for further updates with golang updates that do not require extensive changes.
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.
I have updated to latest go version on latest debian version.
configapi/api_default.go
Outdated
logger.WebUILog.Infoln("Get Device Group by name") | ||
|
||
var deviceGroup configmodels.DeviceGroups | ||
filter := bson.M{"DeviceGroupName": c.Param("group-name")} |
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.
Do you think it makes sense to query the presense of the param? and if present then only go to database.
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.
group-name
is a path parameter and is defined in the router. There is no way to get in this function if the param is not there.
logger.WebUILog.Infoln("Get Network Slice by name") | ||
|
||
var networkSlice configmodels.Slice | ||
filter := bson.M{"SliceName": c.Param("slice-name")} |
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.
Please add check for query param presence.
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 is also a path parameter defined in the router.
if networkSlice.SliceName == "" { | ||
c.JSON(http.StatusNotFound, nil) | ||
} else { | ||
c.JSON(http.StatusOK, networkSlice) |
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.
Can you share curl command output how it looks like both success & failure case
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.
Sure, in the case that the slice does not exist, we get:
curl -v ${WEBUI_IP}:5000/config/v1/network-slice/default
* Trying 10.1.213.205:5000...
* Connected to 10.1.213.205 (10.1.213.205) port 5000 (#0)
> GET /config/v1/network-slice/default HTTP/1.1
> Host: 10.1.213.205:5000
> User-Agent: curl/7.88.1
> Accept: */*
>
< HTTP/1.1 404 Not Found
< Access-Control-Allow-Credentials: true
< Access-Control-Allow-Headers: Content-Type, Content-Length, Accept-Encoding, X-CSRF-Token, Authorization, accept, origin, Cache-Control, X-Requested-With
< Access-Control-Allow-Methods: POST, OPTIONS, GET, PUT, PATCH, DELETE
< Access-Control-Allow-Origin: *
< Content-Type: application/json; charset=utf-8
< Date: Mon, 11 Sep 2023 20:57:59 GMT
< Content-Length: 4
<
* Connection #0 to host 10.1.213.205 left intact
null
In the case where it exists, we get this:
curl -v ${WEBUI_IP}:5000/config/v1/network-slice/default
* Trying 10.1.213.205:5000...
* Connected to 10.1.213.205 (10.1.213.205) port 5000 (#0)
> GET /config/v1/network-slice/default HTTP/1.1
> Host: 10.1.213.205:5000
> User-Agent: curl/7.88.1
> Accept: */*
>
< HTTP/1.1 200 OK
< Access-Control-Allow-Credentials: true
< Access-Control-Allow-Headers: Content-Type, Content-Length, Accept-Encoding, X-CSRF-Token, Authorization, accept, origin, Cache-Control, X-Requested-With
< Access-Control-Allow-Methods: POST, OPTIONS, GET, PUT, PATCH, DELETE
< Access-Control-Allow-Origin: *
< Content-Type: application/json; charset=utf-8
< Date: Mon, 11 Sep 2023 20:59:34 GMT
< Content-Length: 238
<
* Connection #0 to host 10.1.213.205 left intact
{"SliceName":"default","slice-id":{"sst":"1","sd":"010203"},"site-device-group":["cows"],"site-info":{"site-name":"demo","plmn":{"mcc":"208","mnc":"93"},"gNodeBs":[{"name":"demo-gnb1","tac":1}],"upf":{"upf-name":"upf","upf-port":"8805"}}}
This reverts commit 786990a.
add to whitelist |
ok to test |
retest this please |
test this please |
1 similar comment
test this please |
This PR goal is to introduce GET operations to the API for DeviceGroups and Network Slices. To do so, the following changes were needed:
The larger goal is to be able to create a simple web application to configure SD-Core without requiring a full Aether installation for simple use cases.