8000
Skip to content

OVN tests for main test suite#14651

Merged
tomponline merged 7 commits intocanonical:mainfrom
markylaing:ovn-tests
Dec 17, 2024
Merged

OVN tests for main test suite#14651
tomponline merged 7 commits intocanonical:mainfrom
markylaing:ovn-tests

Conversation

@markylaing
Copy link
Copy Markdown
Contributor
@markylaing markylaing commented Dec 13, 2024

This PR adds OVN networking tests to the main suite. This is primarily to give faster feedback when driver changes are made. This initial PR just validates that the configuration is applied correctly to the ovn northbound database. The LXD_OVN_NB_CONNECTION environment variable must be set to run the tests. The tests will fail if there is any existing data in the northbound database. The test expects that ovn-nbctl is available on the PATH.

Comment thread test/suites/network_ovn.sh Outdated
Copy link
Copy Markdown
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.

Couldn't see it in action due to minor issues flagged by CodeQL but LGTM so far, thanks!

Comment thread test/suites/network_ovn.sh Outdated
Comment thread test/suites/network_ovn.sh Outdated
Comment thread lxd/main_sql.go Outdated
Comment thread test/suites/network_ovn.sh
Comment thread test/suites/network_ovn.sh Outdated
Comment thread test/suites/network_ovn.sh
Comment thread test/suites/network_ovn.sh
Comment thread test/suites/network_ovn.sh Outdated
@markylaing markylaing force-pushed the ovn-tests branch 3 times, most recently from 862c055 to 59ca184 Compare December 14, 2024 17:46
@markylaing
Copy link
Copy Markdown
Contributor Author

@simondeziel just setting the northbound connection isn't enough. I've also set the certificates (based on a previous failed run). But now it fails with the following error:

Error: Failed to run: ovn-nbctl --timeout=10 --db ssl://127.0.0.1:6641 -c /proc/
8000
self/fd/3 -p /proc/self/fd/4 -C /proc/self/fd/5 --wait=sb ha-chassis-group-add lxd-net36: exit status 1 (2024-12-14T18:17:57Z|00002|stream_ssl|ERR|ssl://127.0.0.1:6641: connect: Address family not supported by protocol

Not sure on next steps.

Copy link
Copy Markdown
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.

Love it but couldn't see it in action due to my protocol mistake (ssl:// vs ssl:).

Comment thread test/suites/network_ovn.sh Outdated
Comment thread test/suites/network_ovn.sh
Comment thread test/suites/network_ovn.sh
Comment thread .github/workflows/tests.yml Outdated
@markylaing
Copy link
Copy Markdown
Contributor Author

Love it but couldn't see it in action due to my protocol mistake (ssl:// vs ssl:).

Ahhh of course. I'll update the commit. Then I think I'll split up this PR because the formatting of lxd sql can but submitted separately.

Comment thread test/suites/network_ovn.sh Outdated
tomponline added a commit that referenced this pull request Dec 16, 2024
This allows specifying the output of `lxd sql` as `sql`, `csv`, `table`,
`compact`, `json`, or `yaml`. The new `sql` format is added so that the
column names are not auto-formatted (upper cased, replacing underscores
with spaces).

This is added to ease scripting in tests and is required for #14651.
Signed-off-by: Simon Deziel <simon.deziel@canonical.com>
Signed-off-by: Simon Deziel <simon.deziel@canonical.com>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
Signed-off-by: Simon Deziel <simon.deziel@canonical.com>
Comment thread test/suites/network_ovn.sh Outdated
Comment thread test/suites/network_ovn.sh
@markylaing
Copy link
Copy Markdown
Contributor Author

@simondeziel strange failure now: https://github.com/canonical/lxd/actions/runs/12356855388/job/34484491302?pr=14651#step:13:45737

It seems the IPv6 address is already up in the instance when running in CI, but the test fails locally if this line is not present.

Comment thread test/suites/network_ovn.sh
Signed-off-by: Mark Laing <mark.laing@canonical.com>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
Signed-off-by: Mark Laing <mark.laing@canonical.com>
@markylaing markylaing marked this pull request as ready for review December 17, 2024 09:10
Copy link
Copy Markdown
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.

LGTM thanks!

@tomponline tomponline merged commit 7931651 into canonical:main Dec 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

0