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

[CloudSync] Adding the API to be exposed to thirdparty for Data Publish #446

Merged
merged 1 commit into from
Dec 27, 2021

Conversation

nitu-s-gupta
Copy link
Contributor

Signed-off-by: Nitu Gupta [email protected]

Description

provide mechanism to expose API that can be used by third party apps to publish data to MQTT broker running on AWS endpoint

Fixes # (#382 )

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

1.MQTT mosquitto broker is configured to be running in the AWS endpoint.
2. The edge orchestration is build and run using following command with option CLOUD_SYNC set to true

docker run -it -d --privileged --network="host" --name edge-orchestration -e CLOUD_SYNC=true -v /var/edge-orchestration/:/var/edge-orchestration/:rw -v /var/run/docker.sock:/var/run/docker.sock:rw -v /proc/:/process/:ro  lfedge/edge-home-orchestration-go:latest
  1. The logs are checked using the command
docker logs -f edge-orchestration
  1. From the another terminal/post make a curl command as follows to publish data using home edge to the broker running on AWS endpoint
curl --location --request POST 'http://192.168.1.107:56001/api/v1/orchestration/cloudsyncmgr' \
> --header 'Content-Type: text/plain' \
> --data-raw '{
>     "appid": "TV1",
>     "payload": "{Another data from TV1}",
>     "topic": "home/livingroom",
>     "url" : "ec2-54-144-110-26.compute-1.amazonaws.com"
> }'
  1. You should obtain the following message for successful publish
{"Message":"Data published successfully to Cloud","RemoteTargetInfo":"","ServiceName":""}

Test Configuration:

  • OS type & version: (Ubuntu 20.04)
  • Hardware: (x86-64)
  • Edge Orchestration Release: (v1.1.0)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@sonarcloud
Copy link

sonarcloud bot commented Dec 20, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

No Coverage information No Coverage information
12.2% 12.2% Duplication

Copy link
Contributor

@tiokim tiokim left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution.
Please take a look at my comments.

}
cloudsyncmgr.GetInstance().StartCloudSync(url)
for mqttmgr.GetClient() == nil {
//Wait till the client is set
Copy link
Contributor

Choose a reason for hiding this comment

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

How long have you took? How about calling sleep function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If using sleep what time value would be used. Its not always same in settting client

@@ -351,6 +359,101 @@ SEND_RESP:
h.helper.Response(w, respEncryptBytes, http.StatusOK)
}

// APIV1RequestCloudSyncmgrPost handles cloudsync publish request from service application
func (h *Handler) APIV1RequestCloudSyncmgrPost(w http.ResponseWriter, r *http.Request) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If I call this 10 times with the same host, will 10 mqtt client go routines be created?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

currently it is not storing the client data. Also one more thing when same host is sending the conenctn request the previous connection is closed and new is opened.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wanted to make the base code with single connection with the client. Going forward we think of scenarios where multiple service applications can call the send API to send data to different broker URL's. So in that case maintaining separate mqtt client instance for each service application would be better(and only one instance for each host). Use of db to maintain the same or so could be option. We plan to do this once the base code is merged as part of next steps.

Copy link
Contributor

@tdrozdovsky tdrozdovsky left a comment

Choose a reason for hiding this comment

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

LGTM

@tdrozdovsky tdrozdovsky self-requested a review December 23, 2021 13:27
Copy link
Contributor

@tiokim tiokim left a comment

Choose a reason for hiding this comment

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

LGTM Thanks!

Copy link
Contributor

@suresh-lc suresh-lc left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@MoonkiHong MoonkiHong left a comment

Choose a reason for hiding this comment

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

+1

@MoonkiHong MoonkiHong merged commit 30148eb into lf-edge:master Dec 27, 2021
@nitu-s-gupta nitu-s-gupta deleted the CloudAPI branch February 1, 2022 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants