Skip to content

Add implementation of the snap Openstack Network Agents#1

Merged
gboutry merged 1 commit intocanonical:mainfrom
fabi200123:snap-openstack-network-agents
Sep 11, 2025
Merged

Add implementation of the snap Openstack Network Agents#1
gboutry merged 1 commit intocanonical:mainfrom
fabi200123:snap-openstack-network-agents

Conversation

@fabi200123
Copy link
Copy Markdown
Contributor

@fabi200123 fabi200123 commented Sep 1, 2025

This PR adds the implementation for the snap Openstack Network Agents, which sets the following components for ovs:

@fabi200123 fabi200123 force-pushed the snap-openstack-network-agents branch from 7cb4d90 to 988a302 Compare September 1, 2025 08:00
- python3-neutron
- iproute2
organize:
usr/bin/neutron-ovn-metadata-agent: files/bin/neutron-ovn-metadata-agent.real
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Because of this organize, you'll be missing most part of the staged packages.

And this leads to complicated override-build.

Can we simplify this and just move that file in a override-stage/prime instead?


apply_from_snap_config() {
local iface bridge phys
iface="$(snapctl get external-interface || true)"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's not expose config option bare.

Let's divide them semantically, such as:

snap set openstack-network-agents settings.debug=true network.bridge=... network.physnet=...

subordinate charm via files in $SNAP_COMMON/etc/neutron.
base: core24
version: "0.1"
license: BUSL-1.1
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is not BUSL-1.1.

@fabi200123 fabi200123 force-pushed the snap-openstack-network-agents branch 2 times, most recently from 58b36a4 to 1ef1b53 Compare September 4, 2025 11:54
cp "$SNAPCRAFT_STAGE/usr/bin/neutron-ovn-metadata-agent" "$SNAPCRAFT_PRIME/files/bin/neutron-ovn-metadata-agent.real"

layout:
/etc/neutron:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The config should live in $SNAP_COMMON/etc/neutron, not binding on the host

@fabi200123 fabi200123 force-pushed the snap-openstack-network-agents branch from 1ef1b53 to 77f5c01 Compare September 8, 2025 14:57
local ovsctl=""
if command -v ovs-vsctl >/dev/null 2>&1; then
ovsctl="ovs-vsctl"
elif [[ -x /snap/microovn/current/usr/bin/ovs-vsctl ]]; then
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think the last step would be to actually consume both:

ovn-chassis plug from microovn
and get a bin slot exposing the ovs-vsctl command to ensure we get the right binary (right version)

@fabi200123 fabi200123 force-pushed the snap-openstack-network-agents branch 2 times, most recently from 454f119 to 46d8206 Compare September 10, 2025 06:56
Comment on lines +18 to +24
ovn-chassis:
interface: ovn-chassis
microovn-ovsdb:
interface: system-files
write:
- /var/snap/microovn/common/run/switch
- /var/snap/microovn/common/run/switch/db.sock
Copy link
Copy Markdown
Collaborator

@gboutry gboutry Sep 10, 2025

Choose a reason for hiding this comment

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

What does this new plug bring that is not shared by ovn-chassis https://github.com/canonical/microovn/blob/72b93b5329ff5b070685d818943b868fe2423f89/snap/snapcraft.yaml#L25C3-L25C14 ?

I believe the definition should look like:

Suggested change
ovn-chassis:
interface: ovn-chassis
microovn-ovsdb:
interface: system-files
write:
- /var/snap/microovn/common/run/switch
- /var/snap/microovn/common/run/switch/db.sock
ovn-chassis:
interface: content
target: "$SNAP_DATA/microovn/chassis"

(to be checked)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm a bit surprised you don't need the certificates?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you, indeed this fixes the problems regarding accessing the db... Removed the others in favor of this one

@fabi200123 fabi200123 force-pushed the snap-openstack-network-agents branch from 46d8206 to 53c6f45 Compare September 10, 2025 07:29
interface: network
ovn-chassis:
interface: content
target: $SNAP_DATA/ovn
Copy link
Copy Markdown
Collaborator

@gboutry gboutry Sep 10, 2025

Choose a reason for hiding this comment

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

Suggested change
target: $SNAP_DATA/ovn
target: $SNAP_DATA/microovn/chassis

Can you rename this path? rationale is, we might connect more interfaces from microovn snap, so let's put something consistent for the future

(I updated the previous comment, but you pushed in the meanwhile :) )

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, now I see that :)
Updated the target and retested it as well, it works fine

@fabi200123 fabi200123 force-pushed the snap-openstack-network-agents branch from 53c6f45 to 02631f6 Compare September 10, 2025 08:14
@gboutry gboutry merged commit 13202bb into canonical:main Sep 11, 2025
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.

2 participants