From 95ffd63d1bb0e3d909eca12c12928b13def82df1 Mon Sep 17 00:00:00 2001 From: Michael Engel Date: Thu, 24 Oct 2024 21:24:27 +0200 Subject: [PATCH 1/3] Emit ControllerAddress changed signal in SwitchController method The changed signal for the agents ControllerAddress property was emitted only in the agent_reconnect function when a change in the address has been detected. This, however, led to the agent first emitting a status disconnected signal and then the address changed signal when the SwitchAddress API has been called. The order of these signals should be reversed. The address changed signal is now also emitted inside the SwitchController API function which changes the address. In order to get the up-to-date value in the orch_addr field, the new value is set here as well. The ControllerAddress property changed signal will still be emitted in the reconnect function to ensure any change in the address due to name resolution is considered. Signed-off-by: Michael Engel --- src/agent/agent.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/agent/agent.c b/src/agent/agent.c index 5c6725c06c..3f76b5d3ed 100644 --- a/src/agent/agent.c +++ b/src/agent/agent.c @@ -1939,12 +1939,16 @@ static int agent_method_switch_controller(sd_bus_message *m, void *userdata, UNU "Failed to switch controller because already connected to the controller"); } - if (!agent_set_controller_address(agent, dbus_address)) { + if (!agent_set_controller_address(agent, dbus_address) || + !agent_set_orch_address(agent, dbus_address)) { bc_log_error("Failed to set CONTROLLER ADDRESS"); return sd_bus_reply_method_errorf(m, SD_BUS_ERROR_FAILED, "Failed to set CONTROLLER ADDRESS"); } - - bc_log_infof("CONTROLLER ADDRESS changed to %s", dbus_address); + r = sd_bus_emit_properties_changed( + agent->api_bus, BC_AGENT_OBJECT_PATH, AGENT_INTERFACE, "ControllerAddress", NULL); + if (r < 0) { + bc_log_errorf("Failed to emit controller address property changed: %s", strerror(-r)); + } agent_disconnected(NULL, userdata, NULL); From ade38219f12985e5c31bd636fcc22a0f7e242b1f Mon Sep 17 00:00:00 2001 From: Michael Engel Date: Tue, 29 Oct 2024 09:55:53 +0100 Subject: [PATCH 2/3] Rename orch_addr to assembled_controller_address Signed-off-by: Michael Engel --- src/agent/agent.c | 67 +++++++++++++++++++++++++++-------------------- src/agent/agent.h | 12 ++++----- src/agent/main.c | 2 +- 3 files changed, 45 insertions(+), 36 deletions(-) diff --git a/src/agent/agent.c b/src/agent/agent.c index 3f76b5d3ed..03f99f4192 100644 --- a/src/agent/agent.c +++ b/src/agent/agent.c @@ -523,7 +523,7 @@ void agent_unref(Agent *agent) { free_and_null(agent->name); free_and_null(agent->host); - free_and_null(agent->orch_addr); + free_and_null(agent->assembled_controller_address); free_and_null(agent->api_bus_service_name); free_and_null(agent->controller_address); free_and_null(agent->peer_socket_options); @@ -568,8 +568,8 @@ bool agent_set_controller_address(Agent *agent, const char *address) { return copy_str(&agent->controller_address, address); } -bool agent_set_orch_address(Agent *agent, const char *address) { - return copy_str(&agent->orch_addr, address); +bool agent_set_assembled_controller_address(Agent *agent, const char *address) { + return copy_str(&agent->assembled_controller_address, address); } bool agent_set_host(Agent *agent, const char *host) { @@ -1924,25 +1924,31 @@ static int agent_method_switch_controller(sd_bus_message *m, void *userdata, UNU int r = sd_bus_message_read(m, "s", &dbus_address); if (r < 0) { - bc_log_errorf("Failed to read DbusAddress parameter: %s", strerror(-r)); + bc_log_errorf("Failed to read D-Bus address parameter: %s", strerror(-r)); return sd_bus_reply_method_errorf( m, SD_BUS_ERROR_FAILED, - "Failed to read DbusAddress parameter: %s", + "Failed to read D-Bus address parameter: %s", strerror(-r)); } - if (agent->orch_addr && streq(agent->orch_addr, dbus_address)) { + if (agent->assembled_controller_address && streq(agent->assembled_controller_address, dbus_address)) { return sd_bus_reply_method_errorf( m, SD_BUS_ERROR_FAILED, "Failed to switch controller because already connected to the controller"); } - if (!agent_set_controller_address(agent, dbus_address) || - !agent_set_orch_address(agent, dbus_address)) { - bc_log_error("Failed to set CONTROLLER ADDRESS"); - return sd_bus_reply_method_errorf(m, SD_BUS_ERROR_FAILED, "Failed to set CONTROLLER ADDRESS"); + /* The assembled controller address is the field used for the ControllerAddress property. + * However, this field gets assembled in the reconnect based on the configuration of host + port + * or controller address. + * So in order to enable emitting the address changed signal be sent before the disconnect AND keep + * the new value, both fields need to be set with the new address. + */ + if (!agent_set_assembled_controller_address(agent, dbus_address) || + !agent_set_controller_address(agent, dbus_address)) { + bc_log_error("Failed to set controller address"); + return sd_bus_reply_method_errorf(m, SD_BUS_ERROR_FAILED, "Failed to set controller address"); } r = sd_bus_emit_properties_changed( agent->api_bus, BC_AGENT_OBJECT_PATH, AGENT_INTERFACE, "ControllerAddress", NULL); @@ -2038,7 +2044,7 @@ static const sd_bus_vtable agent_vtable[] = { SD_BUS_PROPERTY("ControllerAddress", "s", NULL, - offsetof(Agent, orch_addr), + offsetof(Agent, assembled_controller_address), SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_VTABLE_END }; @@ -2411,15 +2417,15 @@ int agent_init_units(Agent *agent, sd_bus_message *m) { return 0; } -static bool ensure_orch_address(Agent *agent) { +static bool ensure_assembled_controller_address(Agent *agent) { int r = 0; - if (agent->orch_addr != NULL) { + if (agent->assembled_controller_address != NULL) { return true; } if (agent->controller_address != NULL) { - return agent_set_orch_address(agent, agent->controller_address); + return agent_set_assembled_controller_address(agent, agent->controller_address); } if (agent->host == NULL) { @@ -2452,11 +2458,11 @@ static bool ensure_orch_address(Agent *agent) { bc_log_errorf("INET4: Invalid host option '%s'", ip_address); return false; } - _cleanup_free_ char *orch_addr = assemble_tcp_address(&host); - if (orch_addr == NULL) { + _cleanup_free_ char *assembled_controller_address = assemble_tcp_address(&host); + if (assembled_controller_address == NULL) { return false; } - agent_set_orch_address(agent, orch_addr); + agent_set_assembled_controller_address(agent, assembled_controller_address); } else if (host_is_ipv6 || is_ipv6(ip_address)) { struct sockaddr_in6 host6; memset(&host6, 0, sizeof(host6)); @@ -2467,16 +2473,16 @@ static bool ensure_orch_address(Agent *agent) { bc_log_errorf("INET6: Invalid host option '%s'", ip_address); return false; } - _cleanup_free_ char *orch_addr = assemble_tcp_address_v6(&host6); - if (orch_addr == NULL) { + _cleanup_free_ char *assembled_controller_address = assemble_tcp_address_v6(&host6); + if (assembled_controller_address == NULL) { return false; } - agent_set_orch_address(agent, orch_addr); + agent_set_assembled_controller_address(agent, assembled_controller_address); } else { bc_log_errorf("Unknown protocol for '%s'", ip_address); } - return agent->orch_addr != NULL; + return agent->assembled_controller_address != NULL; } bool agent_start(Agent *agent) { @@ -2494,7 +2500,7 @@ bool agent_start(Agent *agent) { return false; } - if (!ensure_orch_address(agent)) { + if (!ensure_assembled_controller_address(agent)) { return false; } @@ -2691,9 +2697,10 @@ void agent_stop(Agent *agent) { static bool agent_connect(Agent *agent) { peer_bus_close(agent->peer_dbus); - bc_log_infof("Connecting to controller on %s", agent->orch_addr); + bc_log_infof("Connecting to controller on %s", agent->assembled_controller_address); - agent->peer_dbus = peer_bus_open(agent->event, "peer-bus-to-controller", agent->orch_addr); + agent->peer_dbus = peer_bus_open( + agent->event, "peer-bus-to-controller", agent->assembled_controller_address); if (agent->peer_dbus == NULL) { bc_log_error("Failed to open peer dbus"); return false; @@ -2799,18 +2806,20 @@ static bool agent_connect(Agent *agent) { } static bool agent_reconnect(Agent *agent) { - _cleanup_free_ char *orch_addr = NULL; + _cleanup_free_ char *assembled_controller_address = NULL; // resolve FQDN again in case the system changed // e.g. bluechi controller has been migrated to a different host - if (agent->orch_addr != NULL) { - orch_addr = steal_pointer(&agent->orch_addr); + if (agent->assembled_controller_address != NULL) { + assembled_controller_address = steal_pointer(&agent->assembled_controller_address); } - if (!ensure_orch_address(agent)) { + if (!ensure_assembled_controller_address(agent)) { return false; } - if (agent->orch_addr && orch_addr && !streq(agent->orch_addr, orch_addr)) { + // If the controller address has changed, emit the respective signal + if (agent->assembled_controller_address && assembled_controller_address && + !streq(agent->assembled_controller_address, assembled_controller_address)) { int r = sd_bus_emit_properties_changed( agent->api_bus, BC_AGENT_OBJECT_PATH, AGENT_INTERFACE, "ControllerAddress", NULL); if (r < 0) { diff --git a/src/agent/agent.h b/src/agent/agent.h index 9a41b9c821..4a05a5fe8f 100644 --- a/src/agent/agent.h +++ b/src/agent/agent.h @@ -56,9 +56,13 @@ struct Agent { bool systemd_user; char *name; + char *api_bus_service_name; + char *host; int port; char *controller_address; + char *assembled_controller_address; + long heartbeat_interval_msec; long controller_heartbeat_threshold_msec; @@ -69,11 +73,6 @@ struct Agent { uint64_t disconnect_timestamp; uint64_t disconnect_timestamp_monotonic; - char *orch_addr; - char *api_bus_service_name; - - bool metrics_enabled; - SocketOptions *peer_socket_options; sd_event *event; @@ -82,6 +81,7 @@ struct Agent { sd_bus *systemd_dbus; sd_bus *peer_dbus; + bool metrics_enabled; sd_bus_slot *metrics_slot; LIST_HEAD(SystemdRequest, outstanding_requests); @@ -110,7 +110,7 @@ void agent_unref(Agent *agent); bool agent_set_port(Agent *agent, const char *port); bool agent_set_host(Agent *agent, const char *host); -bool agent_set_orch_address(Agent *agent, const char *address); +bool agent_set_assembled_controller_address(Agent *agent, const char *address); bool agent_set_name(Agent *agent, const char *name); bool agent_set_heartbeat_interval(Agent *agent, const char *interval_msec); void agent_set_systemd_user(Agent *agent, bool systemd_user); diff --git a/src/agent/main.c b/src/agent/main.c index 5485eba421..264300c4c7 100644 --- a/src/agent/main.c +++ b/src/agent/main.c @@ -161,7 +161,7 @@ int main(int argc, char *argv[]) { return EXIT_FAILURE; } - if (opt_address && !agent_set_orch_address(agent, opt_address)) { + if (opt_address && !agent_set_assembled_controller_address(agent, opt_address)) { return EXIT_FAILURE; } From 29c572d855d38afca8cfe72a7a7ef970e37663f1 Mon Sep 17 00:00:00 2001 From: Michael Engel Date: Tue, 29 Oct 2024 10:09:45 +0100 Subject: [PATCH 3/3] Renamed orch_src to ctrl_src in meson files Signed-off-by: Michael Engel --- src/controller/meson.build | 6 +++--- src/controller/test/controller/meson.build | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/controller/meson.build b/src/controller/meson.build index 99dd2914ea..508b6a2d0c 100644 --- a/src/controller/meson.build +++ b/src/controller/meson.build @@ -3,9 +3,9 @@ # # SPDX-License-Identifier: LGPL-2.1-or-later -# orch build configuration +# controller build configuration -orch_src = [ +ctrl_src = [ 'controller.h', 'controller.c', 'node.h', @@ -23,7 +23,7 @@ orch_src = [ executable( 'bluechi-controller', - orch_src, + ctrl_src, dependencies: [ systemd_dep, inih_dep, diff --git a/src/controller/test/controller/meson.build b/src/controller/test/controller/meson.build index 4c0b51bb7e..eb56c4c004 100644 --- a/src/controller/test/controller/meson.build +++ b/src/controller/test/controller/meson.build @@ -9,7 +9,7 @@ controller_src = [ # setup controller test src files to include in compilation controller_test_src = [] -foreach src : orch_src +foreach src : ctrl_src # skip main to avoid duplicate main function error if src == 'main.c' continue