Skip to content

perf(hooks): reduce service reconciliation overhead #205

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.

Sign up for GitHub

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

Merged
hemanthnakkina merged 2 commits into from
Aug 13, 2026
+294 −78
Merged

perf(hooks): reduce service reconciliation overhead #205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
332e7a2
perf(hooks): skip converged service operations
gboutry Jul 16, 2026
eb762ec
perf(hooks): batch service reconciliation
gboutry Jul 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 84 additions & 42 deletions openstack_hypervisor/hooks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
from netifaces import AF_INET, gateways, ifaddresses
from pyroute2 import IPRoute
from pyroute2.netlink.exceptions import NetlinkError
from snaphelpers import Snap
from snaphelpers import Snap, SnapCtl
from snaphelpers._conf import UnknownConfigKey

from openstack_hypervisor import netplan, pci
Expand Down Expand Up @@ -414,6 +414,11 @@ def _get_local_ip_by_default_route() -> str:
# Required config can be a section like "identity" in which case all keys must
# be set or a single key like "identity.password".
REQUIRED_CONFIG = {
"file-transfer": [
"compute.cacert",
"compute.cert",
"compute.key",
],
"nova-compute": [
"identity.password",
"identity.username",
Expand Down Expand Up @@ -649,11 +654,56 @@ def __exit__(self, exc_type, exc_value, exc_traceback):
restart_services.extend(self.files[file].get("services", []))

restart_services = set([s for s in restart_services if s not in self.exclude_services])
services = self.snap.services.list()
for service in restart_services:
logging.info(f"Restarting {service}")
services[service].stop()
services[service].start(enable=True)
if not restart_services:
return

_restart_services(self.snap, restart_services)


def _service_subset(snap: Snap, names: typing.Iterable[str]) -> tuple[list[str], Dict[str, Any]]:
"""Return sorted unique service names and their current state."""
requested = sorted(set(names))
if not requested:
return requested, {}
services = snap.services.list()
missing = [name for name in requested if name not in services]
if missing:
raise RuntimeError(f"Services are not defined by the snap: {', '.join(missing)}")
return requested, services


# snapd applies service tasks only after the hook exits, so these helpers must
# not re-read status to verify their queued state transitions.
def _ensure_services_started(snap: Snap, names: typing.Iterable[str]) -> None:
"""Queue one start-and-enable operation for non-converged services."""
requested, services = _service_subset(snap, names)
pending = [
name for name in requested if not (services[name].enabled and services[name].active)
]
if not pending:
return
SnapCtl(env=snap.environ).start(*pending, enable=True)


def _ensure_services_stopped(snap: Snap, names: typing.Iterable[str]) -> None:
"""Queue one stop-and-disable operation for non-converged services."""
requested, services = _service_subset(snap, names)
pending = [name for name in requested if services[name].enabled or services[name].active]
if not pending:
return
SnapCtl(env=snap.environ).stop(*pending, disable=True)


def _restart_services(snap: Snap, names: typing.Iterable[str]) -> None:
"""Queue batched stop and start operations for the services."""
requested, _ = _service_subset(snap, names)
if not requested:
return
for service in requested:
logging.info("Restarting %s", service)
snapctl = SnapCtl(env=snap.environ)
snapctl.stop(*requested)

@Raven-182 Raven-182 Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Are these batch stop and start requests queued as separate tasks? in other words if service X stops but Y fails, will the queued batch start task still run, or could X remain stopped?

@gboutry gboutry Aug 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

start and stop tasks are not executed within the confines of a configure hook. When you send the order to snapd, they are queued for after the execution of the configure hook ends.

Meaning that a failure here, would be a snapd failure, and would result in a failed hook.

snapctl.start(*requested, enable=True)


def _update_default_config(snap: Snap) -> None:
Expand Down Expand Up @@ -2597,22 +2647,22 @@ def _configure_monitoring_services(snap: Snap) -> None:
:type snap: Snap
:return: None
"""
services = snap.services.list()
enable_monitoring = snap.config.get("monitoring.enable")
if enable_monitoring:
ovs_external = is_ovs_external()
if ovs_external:
logging.info("Enabling exporter services (skipping external OVS exporters).")
else:
logging.info("Enabling all exporter services.")
for service in MONITORING_SERVICES:
if ovs_external and service in EXTERNAL_OVS_SERVICES:
continue
services[service].start(enable=True)
desired = [
service
for service in MONITORING_SERVICES
if not (ovs_external and service in EXTERNAL_OVS_SERVICES)
]
_ensure_services_started(snap, desired)
else:
logging.info("Disabling all exporter services.")
for service in MONITORING_SERVICES:
services[service].stop(disable=True)
_ensure_services_stopped(snap, MONITORING_SERVICES)


def _configure_masakari_services(snap: Snap) -> None:
Expand All @@ -2622,21 +2672,19 @@ def _configure_masakari_services(snap: Snap) -> None:
:type snap: Snap
:return: None
"""
services = snap.services.list()
enable_masakari = snap.config.get("masakari.enable")
if enable_masakari:
logging.info("Enabling all masakari services.")
for service in MASAKARI_SERVICES:
services[service].start(enable=True)
_ensure_services_started(snap, MASAKARI_SERVICES)
else:
logging.info("Disabling all masakari services.")
for service in MASAKARI_SERVICES:
services[service].stop(disable=True)
_ensure_services_stopped(snap, MASAKARI_SERVICES)


def services() -> List[str]:
"""List of services managed by hooks."""
return sorted(list(set([w for v in TEMPLATES.values() for w in v.get("services", [])])))
templates = {**TEMPLATES, **TLS_TEMPLATES}
return sorted(set(w for v in templates.values() for w in v.get("services", [])))


def _section_complete(section: str, context: dict) -> bool:
Expand Down Expand Up @@ -2832,13 +2880,13 @@ def process_whitelisted_sriov_pfs(


def _configure_sriov_agent_service(snap: Snap, enabled: bool) -> None:
sriov_service = snap.services.list()["neutron-sriov-nic-agent"]
service = ["neutron-sriov-nic-agent"]

@freyes freyes Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: s/service/services/ ... since it's a list.

if enabled:
logging.info("SR-IOV mappings detected, enabling SR-IOV agent.")
sriov_service.start(enable=True)
_ensure_services_started(snap, service)
else:
logging.info("No SR-IOV mappings detected, disabling SR-IOV agent.")
sriov_service.stop(disable=True)
_ensure_services_stopped(snap, service)


def _set_config_context(context, group, key, val):
Expand Down Expand Up @@ -3030,16 +3078,14 @@ def configure(snap: Snap) -> None:

context = _get_configure_context(snap)
exclude_services = _get_exclude_services(context)
services = snap.services.list()
ovs_external = is_ovs_external()
logging.info(
"OVS management: %s", "external (microovn)" if ovs_external else "internal (hypervisor)"
)
internal_ovs_deferred = not ovs_external and not _internal_ovs_ready(snap)
external_ovs_deferred = ovs_external and not _external_ovs_ready(snap)
ovs_deferred = internal_ovs_deferred or external_ovs_deferred
for service in exclude_services:
services[service].stop(disable=True)
_ensure_services_stopped(snap, exclude_services)

with RestartOnChange(snap, {**TEMPLATES, **TLS_TEMPLATES}, exclude_services):
_render_templates(snap, context)
Expand Down Expand Up @@ -3239,28 +3285,24 @@ def _ensure_internal_ovs_services(snap: Snap, exclude_services: list[str]) -> No
snap: The snap reference.
exclude_services: Services that should remain stopped.
"""
services = snap.services.list()
enable_monitoring = snap.config.get("monitoring.enable")

for service in EXTERNAL_OVS_SERVICES:
if service in exclude_services:
continue
# ovs-exporter should only be enabled if monitoring is enabled
if service in MONITORING_SERVICES and not enable_monitoring:
continue
logging.info("Ensuring internal OVS service is enabled: %s", service)
services[service].start(enable=True)
desired = [
service
for service in EXTERNAL_OVS_SERVICES
if service not in exclude_services
and not (service in MONITORING_SERVICES and not enable_monitoring)
]
logging.info("Ensuring internal OVS services are enabled: %s", desired)
_ensure_services_started(snap, desired)


def _ensure_internal_ovs_dependent_services(snap: Snap, exclude_services: list[str]) -> None:
"""Ensure services that need internal OVS are enabled when not excluded."""
services = snap.services.list()

for service in INTERNAL_OVS_DEPENDENT_SERVICES:
if service in exclude_services:
continue
logging.info("Ensuring internal OVS-dependent service is enabled: %s", service)
services[service].start(enable=True)
desired = [
service for service in INTERNAL_OVS_DEPENDENT_SERVICES if service not in exclude_services
]
logging.info("Ensuring internal OVS-dependent services are enabled: %s", desired)
_ensure_services_started(snap, desired)


def _get_exclude_services(context: dict) -> list[str]:
Expand Down
Loading
Loading