BUG/MAJOR: reconfigure runtime client and HAPorxy if path changes in the configuration file

If the user manually changes the configuration, the file watcher will be notified, the configuration will be reloaded, and the runtime client will be reconfigured if needed.
To be able to reconfigure the runtime client, HAProxy must be reloaded. Otherwise, the new socket does not exist yet, and reconfiguration fails. We use the Reload agent to schedule a reload and reconfigure the runtime client.

Signed-off-by: Vincent Gramer <[email protected]>

Author: vgramer

client-native/cn.go

@@ -7,8 +7,8 @@ import (
 	"sync"
 	"time"
 
-	clientnative "github.com/haproxytech/client-native/v5"
-	"github.com/haproxytech/client-native/v5/models"
+	clientnative "github.com/haproxytech/client-native/v4"
+	"github.com/haproxytech/client-native/v4/models"
 
 	"github.com/haproxytech/client-native/v4/configuration"
 	configuration_options "github.com/haproxytech/client-native/v4/configuration/options"

configure_data_plane.go

@@ -194,10 +194,13 @@ func configureAPI(api *operations.DataPlaneAPI) http.Handler {
 	signal.Notify(sigs, syscall.SIGUSR1, syscall.SIGUSR2)
 	go handleSignals(ctx, cancel, sigs, client, haproxyOptions, users)
 
+	ra := configureReloadAgent(ctx, haproxyOptions, client)
+
 	if !haproxyOptions.DisableInotify {
-		if err := startWatcher(ctx, client, haproxyOptions, users); err != nil {
+		if err := startWatcher(ctx, client, haproxyOptions, users, ra); err != nil {
 			haproxyOptions.DisableInotify = true
 			client = configureNativeClient(clientCtx, haproxyOptions, mWorker)
+			ra = configureReloadAgent(ctx, haproxyOptions, client)
 		}
 	}
 
@@ -206,24 +209,6 @@ func configureAPI(api *operations.DataPlaneAPI) http.Handler {
 		go cfg.MapSync.SyncAll(client)
 	}
 
-	// Initialize reload agent
-	raParams := haproxy.ReloadAgentParams{
-		Delay:      haproxyOptions.ReloadDelay,
-		ReloadCmd:  haproxyOptions.ReloadCmd,
-		RestartCmd: haproxyOptions.RestartCmd,
-		StatusCmd:  haproxyOptions.StatusCmd,
-		ConfigFile: haproxyOptions.ConfigFile,
-		BackupDir:  haproxyOptions.BackupsDir,
-		Retention:  haproxyOptions.ReloadRetention,
-		Ctx:        ctx,
-	}
-
-	ra, e := haproxy.NewReloadAgent(raParams)
-	if e != nil {
-		// nolint:gocritic
-		log.Fatalf("Cannot initialize reload agent: %v", e)
-	}
-
 	// setup discovery handlers
 	api.DiscoveryGetAPIEndpointsHandler = discovery.GetAPIEndpointsHandlerFunc(func(params discovery.GetAPIEndpointsParams, principal interface{}) middleware.Responder {
 		ends, err := misc.DiscoverChildPaths("", SwaggerJSON)
@@ -816,6 +801,26 @@ func configureAPI(api *operations.DataPlaneAPI) http.Handler {
 	return setupGlobalMiddleware(api.Serve(setupMiddlewares), adpts...)
 }
 
+func configureReloadAgent(ctx context.Context, haproxyOptions dataplaneapi_config.HAProxyConfiguration, client client_native.HAProxyClient) *haproxy.ReloadAgent {
+	raParams := haproxy.ReloadAgentParams{
+		Delay:      haproxyOptions.ReloadDelay,
+		ReloadCmd:  haproxyOptions.ReloadCmd,
+		RestartCmd: haproxyOptions.RestartCmd,
+		StatusCmd:  haproxyOptions.StatusCmd,
+		ConfigFile: haproxyOptions.ConfigFile,
+		BackupDir:  haproxyOptions.BackupsDir,
+		Retention:  haproxyOptions.ReloadRetention,
+		Ctx:        ctx,
+	}
+
+	ra, e := haproxy.NewReloadAgent(raParams)
+	if e != nil {
+		// nolint:gocritic
+		log.Fatalf("Cannot initialize reload agent: %v", e)
+	}
+	return ra
+}
+
 // The TLS configuration before HTTPS server starts.
 func configureTLS(tlsConfig *tls.Config) {
 	// Make all necessary changes to the TLS configuration here.
@@ -962,12 +967,40 @@ func reloadConfigurationFile(client client_native.HAProxyClient, haproxyOptions
 	client.ReplaceConfiguration(confClient)
 }
 
-func startWatcher(ctx context.Context, client client_native.HAProxyClient, haproxyOptions dataplaneapi_config.HAProxyConfiguration, users *dataplaneapi_config.Users) error {
+func startWatcher(ctx context.Context, client client_native.HAProxyClient, haproxyOptions dataplaneapi_config.HAProxyConfiguration, users *dataplaneapi_config.Users, reloadAgent *haproxy.ReloadAgent) error {
 	cb := func() {
-		reloadConfigurationFile(client, haproxyOptions, users)
 		configuration, err := client.Configuration()
 		if err != nil {
-			log.Warningf("Failed to increment configuration version: %v", err)
+			log.Warningf("Failed to get configuration: %s", err)
+			return
+		}
+
+		// save old runtime configuration to know if the runtime client must be configured after the new configuration is
+		// reloaded by HAProxy. Logic is done by cn.ReconfigureRuntime() function.
+		_, globalConf, err := configuration.GetGlobalConfiguration("")
+		if err != nil {
+			log.Warningf("Failed to get global configuration section: %s", err)
+			return
+		}
+		runtimeAPIsOld := globalConf.RuntimeAPIs
+
+		// reload configuration from config file.
+		reloadConfigurationFile(client, haproxyOptions, users)
+
+		// reload runtime client if necessary.
+		callbackNeeded, reconfigureFunc, err := cn.ReconfigureRuntime(client, runtimeAPIsOld)
+		if err != nil {
+			log.Warningf("Failed to check if native client need to be reloaded: %s", err)
+			return
+		}
+		if callbackNeeded {
+			reloadAgent.ReloadWithCallback(reconfigureFunc)
+		}
+
+		// get the last configuration which has been updated by reloadConfigurationFile and increment version in config file.
+		configuration, err = client.Configuration()
+		if err != nil {
+			log.Warningf("Failed to get configuration: %s", err)
 			return
 		}
 		if err := configuration.IncrementVersion(); err != nil {

e2e/tests/global/replace.bats

@@ -88,3 +88,33 @@ load 'utils/_helpers'
 	resource_get "$_RUNTIME_MAP_FILES_BASE_PATH" ""
     assert_equal "$SC" 200
 }
+
+
+@test "global: Manually replace a global configuration with socket path changed" {
+    # check HAPRoxy is configured with the expected socket
+	resource_get "$_GLOBAL_BASE_PATH" ""
+	assert_equal "$SC" 200
+	assert_equal "$(get_json_path "$BODY" '.data.runtime_apis[0].address')" "/var/lib/haproxy/stats"
+
+    pre_logs_count=$(dpa_docker_exec 'cat /var/log/dataplaneapi.log' | wc -l)
+
+	# manually change configuration
+	run dpa_docker_exec "sed -i 's@/var/lib/haproxy/stats@/var/lib/haproxy/stats-new@' /etc/haproxy/haproxy.cfg"
+
+    sleep 5
+    # check configuration has been reloaded
+    resource_get "$_GLOBAL_BASE_PATH" ""
+	assert_equal "$SC" 200
+	assert_equal "$(get_json_path "$BODY" '.data.runtime_apis[0].address')" "/var/lib/haproxy/stats-new"
+
+    # check that runtime client has been reconfigured with the new socket
+    post_logs_count=$(dpa_docker_exec 'sh /var/log/dataplaneapi.log' | wc -l)
+    new_logs_count=$(( $pre_logs_count - $post_logs_count ))
+    new_logs=$(dpa_docker_exec 'cat /var/log/dataplaneapi.log' | tail -n $new_logs_count)
+
+    echo "$new_logs" # this will help debugging if the test fails
+    assert echo -e "$new_logs" | grep -q "reload callback completed, runtime API reconfigured"
+
+	resource_get "$_RUNTIME_MAP_FILES_BASE_PATH" ""
+    assert_equal "$SC" 200
+}