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

Send OVN logs to events API and Loki #12200

Merged
merged 13 commits into from
Sep 20, 2023

Conversation

monstermunchkin
Copy link
Contributor

@monstermunchkin monstermunchkin commented Aug 31, 2023

This adds an OVN log monitor which listens for log messages on a unix socket. If messages are received, they are sent to the events API which will then forward them to Loki (if configured). The log monitor can be enabled by setting core.syslog_socket to true. The logs will only be sent to Loki, if loki.types contains ovn.

In this case, the ovn events can also be monitored using lxc monitor --type=ovn.

Setting up OVN

On Ubuntu, OVN uses the /etc/default/ovn-host environment file. To enable the OVN controller to send their logs via unix socket, one needs to add the following:

OVN_CTL_OPTS="--ovn-controller-log='-vsyslog:info --syslog-method=unix:/var/lib/lxd/syslog.socket'"

There are also --ovn-sb-log and --ovn-nb-log which take the same arguments. These need to be set in /etc/default/ovn-central though.

Specification https://discourse.ubuntu.com/t/send-ovn-logs-to-the-events-api/38445

@github-actions github-actions bot added the API Changes to the REST API label Aug 31, 2023
Copy link
Member

@simondeziel simondeziel left a comment

Choose a reason for hiding this comment

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

LGTM but I'm curious to hear your opinion on the seek approach.

lxd/ovn.go Outdated Show resolved Hide resolved
@simondeziel simondeziel self-requested a review September 1, 2023 12:23
simondeziel
simondeziel previously approved these changes Sep 1, 2023
lxd/loki/loki.go Outdated Show resolved Hide resolved
lxd/ovn.go Outdated Show resolved Hide resolved
lxd/ovn.go Outdated Show resolved Hide resolved
lxd/ovn.go Outdated Show resolved Hide resolved
lxd/daemon.go Outdated Show resolved Hide resolved
lxd/daemon.go Outdated Show resolved Hide resolved
Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

We should not re-read the whole log file(s) each time they change.

@monstermunchkin monstermunchkin force-pushed the features/ovn-loki branch 2 times, most recently from 7b7cb64 to dded3dc Compare September 5, 2023 08:16
@tomponline
Copy link
Member

@monstermunchkin when you're ready for another review please can you click the refresh review button on my name top right so that I get a notification and it'll appear on my todo list on https://github.com/pulls/review-requested

Thanks

@github-actions github-actions bot added the Documentation Documentation needs updating label Sep 5, 2023
@monstermunchkin monstermunchkin force-pushed the features/ovn-loki branch 2 times, most recently from 84ef3bc to f308765 Compare September 5, 2023 13:54
@simondeziel
Copy link
Member

@tomponline @monstermunchkin ovn supports streaming logs to a Unix socket using syslog protocol. Would it be simpler to stream those through LXD then to Loki? That would avoid writing them to disk to read them back.

@tomponline
Copy link
Member

@tomponline @monstermunchkin ovn supports streaming logs to a Unix socket using syslog protocol. Would it be simpler to stream those through LXD then to Loki? That would avoid writing them to disk to read them back.

That would be preferable by the sounds of it, do you have more details on how this works in a cluster?
Does each host send its own local logs to the local unix socket?

@simondeziel
Copy link
Member

That's a good question re cluster, one I cannot answer. All I saw is https://www.man7.org/linux/man-pages/man8/ovn-controller.8.html which also mentions log levels can changed dynamically using ovs-appctl and that also includes the off level.

@monstermunchkin
Copy link
Contributor Author

I've been playing around with the unix socket thing but didn't manage to get it working. I wasn't receiving any log messages for some reason.

However, what I did get working is OVN sending logs via UDP using --syslog-method=udp:localhost:<port>.

@tomponline
Copy link
Member

Sounds promising. Although this approach would require more setup in ovn, it would avoid needing special microovn handling, and we could get microcloud to make the necessary configuration changes in both microovn and lxd during setup.

@tomponline
Copy link
Member

cc @masnax @markylaing

@monstermunchkin monstermunchkin force-pushed the features/ovn-loki branch 2 times, most recently from 24ea749 to d99762a Compare September 12, 2023 08:36
Copy link
Member

@tomponline tomponline left a comment

Choose a reason for hiding this comment

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

This is also missing the tests we discussed adding in our 1:1 wrt to using nc to send messages to the syslog listener and checking they appear in the lxc monitor.

@tomponline
Copy link
Member

@monstermunchkin please let me know when tests pass and ill re-review. Thanks

Copy link
Contributor

@ru-fu ru-fu left a comment

Choose a reason for hiding this comment

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

Some fixes.

doc/api-extensions.md Outdated Show resolved Hide resolved
lxd/node/config.go Outdated Show resolved Hide resolved
doc/howto/network_ovn_setup.md Outdated Show resolved Hide resolved
doc/howto/network_ovn_setup.md Outdated Show resolved Hide resolved
doc/howto/network_ovn_setup.md Outdated Show resolved Hide resolved
lxd/node/config.go Outdated Show resolved Hide resolved
lxd/syslog.go Outdated Show resolved Hide resolved
shared/version/api.go Show resolved Hide resolved
test/suites/syslog.sh Outdated Show resolved Hide resolved
Signed-off-by: Thomas Hipp <thomas.hipp@canonical.com>
Signed-off-by: Thomas Hipp <thomas.hipp@canonical.com>
Signed-off-by: Thomas Hipp <thomas.hipp@canonical.com>
Signed-off-by: Thomas Hipp <thomas.hipp@canonical.com>
Signed-off-by: Thomas Hipp <thomas.hipp@canonical.com>
Signed-off-by: Thomas Hipp <thomas.hipp@canonical.com>
Signed-off-by: Thomas Hipp <thomas.hipp@canonical.com>
Signed-off-by: Thomas Hipp <thomas.hipp@canonical.com>
@monstermunchkin monstermunchkin force-pushed the features/ovn-loki branch 2 times, most recently from 6586560 to 2dd5489 Compare September 19, 2023 18:36
This adds the following words to the wordlist

    * lxc
    * syslog
    * unixgram

Signed-off-by: Thomas Hipp <thomas.hipp@canonical.com>
Signed-off-by: Thomas Hipp <thomas.hipp@canonical.com>
Signed-off-by: Thomas Hipp <thomas.hipp@canonical.com>
Signed-off-by: Thomas Hipp <thomas.hipp@canonical.com>
Signed-off-by: Thomas Hipp <thomas.hipp@canonical.com>
@monstermunchkin
Copy link
Contributor Author

@tomponline all tests have passed.

@tomponline
Copy link
Member

LGTM thanks

@tomponline tomponline merged commit ec53be6 into canonical:main Sep 20, 2023
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Changes to the REST API Documentation Documentation needs updating
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants