Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 20 additions & 4 deletions lib/charms/consul_client/v0/consul_notify.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ def _on_consul_notify_ready(self, event):
"""

import logging
from pathlib import PurePosixPath

from ops.charm import CharmBase, RelationBrokenEvent, RelationChangedEvent, RelationEvent
from ops.framework import EventSource, Object, ObjectEvents
Expand All @@ -114,7 +115,7 @@ def _on_consul_notify_ready(self, event):

# Increment this PATCH version before using `charmcraft publish-lib` or reset
# to 0 if you are raising the major API version
LIBPATCH = 3
LIBPATCH = 4

DEFAULT_RELATION_NAME = "consul-notify"

Expand Down Expand Up @@ -209,13 +210,28 @@ def snap_name(self) -> str | None:

@property
def unix_socket_filepath(self) -> str | None:
"""Return UNIX socket filepath from requirer app data.
"""Return the UNIX socket file name from requirer app data.

Backward compatibility: older requirers (e.g. an openstack-hypervisor
charm that predates the content-interface socket-path fix) publish a
value that contains a directory component such as "data/shutdown.sock".
Comment thread
hemanthnakkina marked this conversation as resolved.
The socket is bind-mounted directly under the content-interface
directory, so only the file name is meaningful here. Normalize to the
basename so consumers always receive just the socket file name,
regardless of the format published by the requirer.

Returns:
The path to the UNIX socket file, or None if not available
The UNIX socket file name, or None if not available or if the
published value has no usable file name component (e.g. "/", ".").
"""
data = self._get_app_databag_from_relation()
return data.get("unix_socket_filepath")
filepath = data.get("unix_socket_filepath")
if not filepath:
return None
filename = PurePosixPath(filepath).name
if filename in ("", ".", ".."):
return None
return filename

@property
def is_ready(self) -> bool:
Expand Down
120 changes: 110 additions & 10 deletions tests/unit/test_charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ def test_consul_notify_socket_available(
datacenter = "test-dc"
join_server_addresses = ["10.20.0.10:8301"]
snap_name = "test-snap"
socket_path = "data/socket.sock"
socket_path = "shutdown.sock"

read_config.return_value = {
"bind_addr": "10.10.0.10",
Expand Down Expand Up @@ -204,7 +204,7 @@ def test_consul_notify_socket_gone(
datacenter = "test-dc"
join_server_addresses = ["10.20.0.10:8301"]
snap_name = "test-snap"
socket_path = "data/socket.sock"
socket_path = "shutdown.sock"

read_config.return_value = {
"bind_addr": "10.10.0.10",
Expand Down Expand Up @@ -266,7 +266,7 @@ def test_consul_notify_relation_properties(
datacenter = "test-dc"
join_server_addresses = ["10.20.0.10:8301"]
snap_name = "test-snap"
socket_path = "data/socket.sock"
socket_path = "shutdown.sock"

read_config.return_value = {
"bind_addr": "10.10.0.10",
Expand Down Expand Up @@ -322,7 +322,7 @@ def test_socket_config_persists_across_events(
datacenter = "test-dc"
join_server_addresses = ["10.20.0.10:8301"]
snap_name = "test-snap"
socket_path = "data/socket.sock"
socket_path = "shutdown.sock"

read_config.return_value = {
"bind_addr": "10.10.0.10",
Expand Down Expand Up @@ -432,7 +432,7 @@ def test_health_check_disabled_by_config(
datacenter = "test-dc"
join_server_addresses = ["10.20.0.10:8301"]
snap_name = "test-snap"
socket_path = "data/socket.sock"
socket_path = "shutdown.sock"

read_config.return_value = {
"bind_addr": "10.10.0.10",
Expand Down Expand Up @@ -504,7 +504,7 @@ def test_health_check_enabled_by_default(
datacenter = "test-dc"
join_server_addresses = ["10.20.0.10:8301"]
snap_name = "test-snap"
socket_path = "data/socket.sock"
socket_path = "shutdown.sock"

read_config.return_value = {
"bind_addr": "10.10.0.10",
Expand Down Expand Up @@ -620,7 +620,7 @@ def test_health_check_can_be_toggled(
datacenter = "test-dc"
join_server_addresses = ["10.20.0.10:8301"]
snap_name = "test-snap"
socket_path = "data/socket.sock"
socket_path = "shutdown.sock"

read_config.return_value = {
"bind_addr": "10.10.0.10",
Expand Down Expand Up @@ -707,7 +707,7 @@ def test_health_check_uses_healthcheck_endpoints_when_available(
join_server_addresses = ["10.20.0.10:8301", "10.20.0.11:8301"]
healthcheck_addresses = ["10.30.0.10:8301", "10.30.0.11:8301"]
snap_name = "test-snap"
socket_path = "data/socket.sock"
socket_path = "shutdown.sock"

read_config.return_value = {
"bind_addr": "10.10.0.10",
Expand Down Expand Up @@ -786,7 +786,7 @@ def test_health_check_falls_back_to_gossip_endpoints(
datacenter = "test-dc"
join_server_addresses = ["10.20.0.10:8301", "10.20.0.11:8301"]
snap_name = "test-snap"
socket_path = "data/socket.sock"
socket_path = "shutdown.sock"

read_config.return_value = {
"bind_addr": "10.10.0.10",
Expand Down Expand Up @@ -861,7 +861,7 @@ def test_health_check_with_empty_healthcheck_endpoints(
datacenter = "test-dc"
join_server_addresses = ["10.20.0.10:8301"]
snap_name = "test-snap"
socket_path = "data/socket.sock"
socket_path = "shutdown.sock"

read_config.return_value = {
"bind_addr": "10.10.0.10",
Expand Down Expand Up @@ -1173,3 +1173,103 @@ def test_ensure_snap_present_holds_snap(harness: Harness[ConsulCharm], snap):
harness.charm._ensure_snap_present()

snap_instance.hold.assert_called()


def test_consul_notify_socket_path_backward_compat(
harness: Harness[ConsulCharm],
snap,
read_config,
write_config,
connect_snap_interface,
write_health_check_script,
):
"""Old-format socket paths are normalized to the bare file name.

Backward-compatibility / upgrade scenario: a requirer (e.g. an
openstack-hypervisor charm that predates the content-interface socket-path
fix) publishes the full relative path "data/shutdown.sock" into the
consul-notify databag. When only the consul-client charm is refreshed the
databag still holds this stale value. The ConsulNotifyProvider must normalize
it to the bare file name so the rendered health-check path is
"hypervisor/shutdown.sock" and not the broken "hypervisor/data/shutdown.sock".
"""
datacenter = "test-dc"
join_server_addresses = ["10.20.0.10:8301"]
snap_name = "test-snap"
old_format_socket_path = "data/shutdown.sock"

harness.add_relation(
"consul-cluster",
"consul-server",
app_data={
"datacenter": datacenter,
"internal_gossip_endpoints": json.dumps(None),
"external_gossip_endpoints": json.dumps(join_server_addresses),
"internal_http_endpoint": json.dumps(None),
"external_http_endpoint": json.dumps(None),
},
)
harness.add_relation(
"consul-notify",
"test-app",
app_data={
"snap_name": snap_name,
"unix_socket_filepath": old_format_socket_path,
},
)

harness.begin_with_initial_hooks()

charm = harness.charm
assert charm.consul_notify.unix_socket_filepath == "shutdown.sock"

config_dict = json.loads(write_config.call_args[0][1])
args = config_dict["services"][0]["check"]["args"]
socket_path = args[args.index("--socket-path") + 1]
assert socket_path == f"{ConsulConfigBuilder.HYPERVISOR_SOCKET_PREFIX}/shutdown.sock"


@pytest.mark.parametrize("invalid_socket_path", ["/", ".", ".."])
def test_consul_notify_socket_path_without_filename_is_none(
harness: Harness[ConsulCharm],
snap,
read_config,
write_config,
connect_snap_interface,
write_health_check_script,
invalid_socket_path,
):
"""Values with no usable file name component are treated as missing.

A requirer publishing a value such as "/", ".", ".." or an empty string has
no meaningful socket file name. The provider must return None rather than an
empty/invalid basename that would render a bogus "hypervisor/" socket path.
"""
datacenter = "test-dc"
join_server_addresses = ["10.20.0.10:8301"]

harness.add_relation(
"consul-cluster",
"consul-server",
app_data={
"datacenter": datacenter,
"internal_gossip_endpoints": json.dumps(None),
"external_gossip_endpoints": json.dumps(join_server_addresses),
"internal_http_endpoint": json.dumps(None),
"external_http_endpoint": json.dumps(None),
},
)
harness.add_relation(
"consul-notify",
"test-app",
app_data={
"snap_name": "test-snap",
"unix_socket_filepath": invalid_socket_path,
},
)

harness.begin_with_initial_hooks()

charm = harness.charm
assert charm.consul_notify.unix_socket_filepath is None
assert charm.consul_notify.is_ready is False