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

[Metadata] No dependency check for support-notifications on startup #3768

Closed
tonyespy opened this issue Oct 21, 2021 · 1 comment · Fixed by #3789
Closed

[Metadata] No dependency check for support-notifications on startup #3768

tonyespy opened this issue Oct 21, 2021 · 1 comment · Fixed by #3789
Labels
2-medium priority denoting issues with cross-cutting project impact bug Something isn't working help wanted Extra attention is needed

Comments

@tonyespy
Copy link
Member

Core Metadata has a notifications feature used to trigger notifications when devices are added. This feature is enabled by default and configured via the [Notifications] configuration stanza:

[Notifications]
PostDeviceChanges = true
Content = "Meatadata notice: "
Sender = "core-metadata"
Description = "Metadata change notice"
Label = "metadata"

Since the edgexfoundry snap disables support-notifications by default (as by there are no subscriptions configured)**, when devices are added, the follow error message is logged:

level=ERROR ts=2021-10-21T19:26:14.081557524Z app=core-metadata source=notify.go:53 msg="fail to invoke device service callback for adding device test-device-3, err: request failed, status code: 500, err: {\"apiVersion\":\"v2\",\"message\":\"driver.AddDevice callback failed for test-device-3 -\\u003e error adding device: ONVIF client could not be initialized: test-device-3\",\"statusCode\":500}"

Note, if the device being added defines secrets, then these need to be added before the device is added, as the call to notify support-notifications only happens if enabled and the callback to the device service succeeds.

If this Notifications.Enabled=true, then Core Metadata should check (via go-mod-bootstrap) that Support Notifications is available on startup.

Steps to reproduce

Note, these steps could be simplified by using a different device-service snap which doesn't require secrets, but this is how I managed to reproduce this today.

add-camera-secrets.sh:

#!/bin/sh

curl -w %{http_code} -X POST \
-d '{
  "apiVersion": "2",
  "path" : "credentials001",
  "secretData" : [
    {
      "key" : "username",
      "value" : "admin"
    },
	{
      "key" : "password",
      "value" : "ADMIN"
    }
  ]
}' http://localhost:59985/api/v2/secret

On an Ubuntu system:

$ sudo snap install edgexfoundry --channel=2.0
$ sudo snap install edgex-device-camera --channel=2.0
$ sudo snap start --enable edgex-device-camera.device-camera
$ ./add-camera-secrets.sh
$ sudo journalctl -f -o cat

The last command will display system logs continuously until you exit via Ctrl-C.

Use the UI to a device:

$ sudo snap install edgex-ui --edge
Now open the UI in a browser using the URL: http://localhost:4000, and add a device to the device-camera service using the following values:

Name = "Camera001"
ProfileName = "camera"
Description = "tony camera"
Location = "brickbottom"
  [DeviceList.Protocols]
    [DeviceList.Protocols.HTTP]
    Address = "192.168.0.13:8080"
    AuthMethod = "usernamepassword"
    CredentialsPath = "credentials001"

** We also should consider disabling this notifications by default, as it makes no sense to send a notification to Support Notifications unless someone has explicitly created at least a single subscription for this event.

Version: 2.0.1-dev.68
Deployment: snap

@tonyespy tonyespy added the bug Something isn't working label Oct 21, 2021
@cloudxxx8
Copy link
Member

We discussed a little in the Core WG meeting on OCT-21-2021.
It might be better to set the default to false.
However, the dependency check would block the core-metadata starting up. @siggiskulason suggested we can replace the Error messages with Warning messages.
Thus the changes are proposed for this issue are:

  1. Set the default value to disable sending out notifications
  2. Modify the log level to Warning instead of Error when the Notifications sending fails
    Note: Not adding dependency check

@tonyespy is it ok for you?

If so, @jinlinGuan we would need to modify TAF test, too

@cloudxxx8 cloudxxx8 self-assigned this Oct 22, 2021
@cloudxxx8 cloudxxx8 added the 2-medium priority denoting issues with cross-cutting project impact label Oct 22, 2021
@cloudxxx8 cloudxxx8 removed their assignment Oct 25, 2021
@cloudxxx8 cloudxxx8 added the help wanted Extra attention is needed label Oct 25, 2021
cloudxxx8 added a commit to cloudxxx8/edgex-go that referenced this issue Oct 31, 2021
Set Notifications.PostDeviceChanges to false by default in core-metadata, because support-notifications service is not always up.
Modify the logging level Warn when sending notifications failed.
Close edgexfoundry#3768

Signed-off-by: Cloud Tsai <[email protected]>
lenny-goodell pushed a commit that referenced this issue Nov 1, 2021
Set Notifications.PostDeviceChanges to false by default in core-metadata, because support-notifications service is not always up.
Modify the logging level Warn when sending notifications failed.
Close #3768

Signed-off-by: Cloud Tsai <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2-medium priority denoting issues with cross-cutting project impact bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants