microovn/ovn: support OVS ovn-encap-ip configuration from external initConfig

[gabrielmougard]

This is needed by: canonical/microcloud#245

[gabrielmougard]

@fnordahl can you give a feedback on this when you have time please? Thanks.

[mkalcok]

Hi @gabrielmougard. I left some comments at your specification proposal which I think should be incorporated.
Point 1 and 2 should be, I think, easily incorporated.
Point 3, tests, could get tricky. Feel free to reach out if you have some questions.

[gabrielmougard]

@mkalcok thanks a lot for the feedback! Working on it.

[gabrielmougard]

@mkalcok I've been thinking about a potential issue: is there some kind of API extension system within MicroOVN that can be queryable by MicroCloud ? I'm just asking this because I wouldn't like to send a key-value config to a version of a deployed MicroOVN that does not support custom encapsulation IP.. I'm obviously new to the MicroOVN codebase so it is maybe a dumb question. Sorry for that in advance.

[tomponline]

@masnax maybe be able to offer some advice here too.

[mkalcok]

Thanks for looping in Max, @tomponline . I was just about to say that MicroOVN does have an API that can be extended, but this "feature/version checking" should be probably handled uniformly across all microcloud components.

[gabrielmougard]

@mkalcok Microcloud requires LXD, MicroCeph and MicroOVN. LXD has its own API to get API extensions (GET /1.0 returns the list of extensions among many other metadata) and MicroCeph has https://github.com/canonical/microceph/blob/main/microceph/api/types/configs.go (GET /1.0/configs that returns the value of a specified key). Could we then have something similar to this KV MicroCeph API here ? Plus, you already have a compatible DB schema for supporting this:

package database

type ConfigItem struct {
	ID    int
	Key   string `db:"primary=yes"`
	Value string
}

It is said to be used for tracking OVN configuration but could it be also used to keep track of currently used MicroOVN features ?

[gabrielmougard]

@mkalcok I don't understand why this gh runner fails for me.. Is there something I need to write in my commits for them to be accepted?

[mkalcok]

It's not your fault @gabrielmougard , PRs coming users that don't have write permissions on the repository need to be manually authorised.

/canonical/self-hosted-runners/run-workflows 4a0e6f1

[mkalcok]

@gabrielmougard regarding errors in the CI, I see similar issues popping up intermittently in my local environment as well . It does not look like a code issue to me, I'll look into it

[mkalcok]

/canonical/self-hosted-runners/run-workflows d87cc34

[mkalcok]

Overall this looks fine,thank you for this feature. I have two small nit picks that I mentioned in the comments.
Other than that, could you please add a separate test suite that would initialize the cluster with custom IP encapsulation and then test that it was properly set?

[gabrielmougard]

@mkalcok is it possible to re-run the CI tests?

[mkalcok]

/canonical/self-hosted-runners/run-workflows fbdd661

[mkalcok]

Thanks for the update @gabrielmougard. I think we need just few more touch-ups and we are done with this one. Aside from linting errors in tests, there's a way to cut down on repetition in tests themselves.

BATS doesn't really have a "native" way to load/import test suits, but we did come up with a way to do that in order to not copy/paste tests between different test suites. Example of how to do this would be scaleup_cluster.bats tests. There we execute bats from within the tests to "include" tests from other test suits.
What you'll need:

• custom setup/teardown (which you already have)
• way to "load" cluster.bats test suite as ipv4 or ipv6 variant
  • we don't have this yet but since the cluster.bats is already written in a way that handles ipv6, you can just create link in the test_helper/bats/cluster_ipv6.bats -> test_helper/bats/cluster.bats
• your test implementation (tests/init_cluster_custom_encap.bats) that will:
  • use your custom setup/teardown
  • tests that encapsulation is properly set in the OVN
  • loads test_helper/bats/cluster.bats (or cluster_ipv6.bats) as seen in the example above.
  • link tests/init_cluster_custom_encap_ipv6.bats -> test/init_cluster_custom_encap.bats to get ipv6 variant of the test suite.

[masnax]

@gabrielmougard I haven't been keeping up with this too much, but I don't think the features endpoint should be in MicroOVN directly. Is there anything specific about this endpoint that wouldn't apply to all microcluster projects?

Unless I'm missing something, I think all microcluster-backend projects will have API extensions and that grow over time and will need to be tracked like this, so I don't think it makes sense to re-implement this per project. In fact microcluster itself might eventually need to keep track of feature compatibility, and projects like MicroOVN would then need to manage two different implementations of this.

I think this should be an endpoint internal to microcluster, and similar to how MicroOVN passes schema extensions to microcluster, it should pass API extensions as well, which are checked when joining a cluster or upgrading an existing one. I think this way, it keeps MicroOVN focused on OVN and microcluster focused on management of the REST API and database.

[gabrielmougard]

@mkalcok is it possible to re-run the CI manually (the snap build seems to have randomly failed..) ?

[mkalcok]

@gabrielmougard Yes, we seems to be hitting some issue when building snap with build-base: devel in snapcraft.yaml. Without that line the build works fine. If we won't find a quick solution to this we'll temporarily remove it from the snapcraft.yaml in the main branch to unblock the CI.

[mkalcok]

@gabrielmougard A temporary fix for the build issue has been merged to the main #150 you can rebase your PR and the CI should start to work again.

[gabrielmougard]

@mkalcok rebased

[mkalcok]

Implementation LGTM, thank you @gabrielmougard. One last think though (and sorry for not mentioning this earlier), user facing feature like this requires also a documentation, probably a how-to page.

[gabrielmougard]

@mkalcok I added the howto's . Let me know if this is correct ;)

[mkalcok]

Overall great job on explaining underlay networks, however I think this page is slightly misleading. Reading it at face value, I'd assume that unless I use microovn init and set "custom encapsulation IP address", there won't be any underlay networking, which is not the case. MicroOVN, by default uses, hostname of the machine as Geneve endpoint.

Could you please add information about the default behavior and highlight that the custom encapsulation IP feature of microovn init is useful if user wants more control over which IP/interface is used for underlay, or if hostname resolution does not work between the hosts.

[mkalcok]

LGTM, thank you for the update and all your hard work on this one.