Skip to content

fix(consul-notify): normalize requirer socket path to basename - #35

Merged
ahmad-can merged 1 commit into
canonical:mainfrom
hemanthnakkina:fix/socket-path-basename-on-upgrade
Jun 9, 2026
Merged

fix(consul-notify): normalize requirer socket path to basename#35
ahmad-can merged 1 commit into
canonical:mainfrom
hemanthnakkina:fix/socket-path-basename-on-upgrade

Conversation

@hemanthnakkina

Copy link
Copy Markdown
Collaborator

The TCP health-check --socket-path is built from the unix_socket_filepath published by the requirer over the consul-notify relation, with the hypervisor/ content-interface directory prepended by the consul-client charm.

Older requirers (e.g. an openstack-hypervisor charm predating the content-interface socket-path fix) publish a value that contains a directory component such as data/shutdown.sock. When only the consul-client charm is refreshed, the relation databag still holds this stale value, so the rendered path becomes hypervisor/data/shutdown.sock, which does not exist in the snap namespace and makes every nic-down alert fail with ENOENT.

Normalize the requirer-provided value to its basename in the ConsulNotifyProvider, so consumers always receive just the socket file name regardless of the format published by the requirer. The rendered path is then always hypervisor/<filename>, decoupling consul-client from requirer upgrade ordering and stale relation data.

Default unit tests now feed a bare filename (the current contract); a dedicated test covers the old data/shutdown.sock format for backward compatibility.

Assisted-By: claude-opus-4.8

Copilot AI review requested due to automatic review settings June 9, 2026 03:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the consul-notify provider to normalize the requirer-published UNIX socket path to a basename, ensuring the TCP health check always targets hypervisor/<filename> even when stale relation data contains an older data/<filename> format.

Changes:

  • Normalize unix_socket_filepath to a basename in ConsulNotifyProvider for backward compatibility with older requirers.
  • Update existing unit tests to use the “bare filename” contract.
  • Add a dedicated unit test covering the legacy data/shutdown.sock relation value.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
lib/charms/consul_client/v0/consul_notify.py Normalizes requirer-provided socket path to basename and bumps lib patch version.
tests/unit/test_charm.py Updates socket-path test inputs to bare filenames and adds a backward-compatibility regression test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/charms/consul_client/v0/consul_notify.py Outdated
The TCP health-check `--socket-path` is built from the `unix_socket_filepath`
published by the requirer over the consul-notify relation, with the
`hypervisor/` content-interface directory prepended by the consul-client charm.

Older requirers (e.g. an openstack-hypervisor charm predating the
content-interface socket-path fix) publish a value that contains a directory
component such as `data/shutdown.sock`. When only the consul-client charm is
refreshed, the relation databag still holds this stale value, so the rendered
path becomes `hypervisor/data/shutdown.sock`, which does not exist in the snap
namespace and makes every nic-down alert fail with ENOENT.

Normalize the requirer-provided value to its basename in the
ConsulNotifyProvider, so consumers always receive just the socket file name
regardless of the format published by the requirer. The rendered path is then
always `hypervisor/<filename>`, decoupling consul-client from requirer upgrade
ordering and stale relation data.

Default unit tests now feed a bare filename (the current contract); a dedicated
test covers the old `data/shutdown.sock` format for backward compatibility.

Assisted-By: claude-opus-4.8
Signed-off-by: Hemanth Nakkina <hemanth.nakkina@canonical.com>
@hemanthnakkina
hemanthnakkina force-pushed the fix/socket-path-basename-on-upgrade branch from 9552e64 to d968861 Compare June 9, 2026 03:56
@hemanthnakkina
hemanthnakkina requested a review from ahmad-can June 9, 2026 03:56
Comment thread lib/charms/consul_client/v0/consul_notify.py
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