NMS-20110: HA config sync observability, version guard, and gated status

core/daemon/src/main/java/org/opennms/netmgt/vmmgr/Invoker.java

@@ -92,6 +92,11 @@ public class Invoker {
     private List<InvokerService> m_services;
 
     private final Path m_statusPath;
+    /** Written instead of a service list when the process is up but its
+     * services have not been started; bin/opennms reports this distinctly
+     * rather than as a failure to determine status. */
+    public static final String GATED_STATUS_MARKER = "OpenNMS: gated";
+
     private static SignalHandler s_handler;
 
     /**
@@ -168,10 +173,15 @@ private void writeStatusUpdate() {
 
         final var output = new StringBuilder();
 
-        // Before setServices() (e.g. blocked at the HA startup gate) there are
-        // no services to report; an empty status file truthfully reads as such.
-        final List<InvokerService> services = getServices() != null ? getServices() : List.of();
-        for (final var invokerService : services) {
+        // Before setServices() — e.g. blocked at a startup gate — there is no
+        // service to report on. An empty file reads to bin/opennms as "failed
+        // to determine running services", so say what is actually true.
+        if (getServices() == null) {
+            writeStatusFile(GATED_STATUS_MARKER + "\n");
+            return;
+        }
+
+        for (final var invokerService : getServices()) {
             final var serviceName = invokerService.getService().getName();
             for (final var invoke : invokerService.getService().getInvokes()) {
                 if ("status".equals(invoke.getMethod())) {
@@ -188,8 +198,12 @@ private void writeStatusUpdate() {
             }
         }
 
+        writeStatusFile(output.toString());
+    }
+
+    private void writeStatusFile(final String content) {
         try {
-            Files.writeString(m_statusPath, output.toString(), Charset.defaultCharset(), CREATE, TRUNCATE_EXISTING);
+            Files.writeString(m_statusPath, content, Charset.defaultCharset(), CREATE, TRUNCATE_EXISTING);
         } catch (final IOException e) {
             System.err.println("ERROR: failed to write current status to " + m_statusPath);
             e.printStackTrace();

docs/modules/operation/pages/deep-dive/admin/high-availability.adoc

@@ -137,6 +137,36 @@ While non-ACTIVE, a node replicates `etc/` from the active node every `sync-inte
 On identically installed nodes leave `<sync-excludes>` empty; list only files that genuinely differ per node (for example a host-specific certificate store).
 A file excluded on either node is neither overwritten nor deleted by sync.
 
+`<sync-roots>` selects which directories under `$OPENNMS_HOME` are replicated — `etc` by default, plus `deploy` to replicate plugins, which land while the standby's Karaf is stopped and install when it promotes.
+Never list `data`: it is each node's own Karaf cache.
+
+Both nodes must run the same release: the manifest advertises the serving node's version, and a standby skips the cycle with a warning while the versions differ, so a rolling upgrade never mixes configuration formats.
+
+Startup-only files sync like any other, but the running process keeps the values it booted with, so that node reports `RESTART REQUIRED` until restarted.
+In practice this means `opennms.properties.d/`, where your own settings belong; `opennms.properties` itself is product-owned, is replaced by upgrades, and is identical across nodes of the same release.
+The credential vault is exempt — it reloads when the file changes, so a rotated credential needs no restart.
+After installing a standby, let one sync cycle finish and restart it once, so it boots with the active node's configuration.
+
+=== Read the status output
+
+`opennms:ha-status` prints one row per node, then a `Config sync` block:
+
+[source]
+----
+INSTANCE          | ROLE      | STATE   | HEARTBEAT AGE | STALE
+opennms-primary   | PRIMARY   | ACTIVE  | 3s ago        | no
+opennms-secondary | SECONDARY | STANDBY | 2s ago        | no
+
+Config sync
+===========
+  opennms-secondary  last success 45s ago
+  opennms-primary    NEVER SUCCEEDED — credentials not available
+----
+
+`STALE` means only that a heartbeat is older than the failover threshold; role and state are reported as they are, so a completed failover (a SECONDARY holding ACTIVE) is not a fault.
+`NEVER SUCCEEDED` is the line to act on: nothing is being replicated, and a promotion would start from divergent configuration.
+`GET /rest/ha/status` reports the same as `heartbeat-stale`, `last-sync-success`, `last-sync-error` and `restart-required`.
+
 == Supervision modes
 
 `<mode>coordinator</mode>` (default) — the node runs the HA state machine described above itself.

features/ha-management/ha-api/src/main/java/org/opennms/netmgt/ha/HaConfiguration.java

@@ -49,6 +49,9 @@
 @XmlAccessorType(XmlAccessType.NONE)
 public class HaConfiguration implements Serializable {
 
+    /** The sync root every pair replicates; others are opt-in. */
+    public static final String DEFAULT_SYNC_ROOT = "etc";
+
     private static final long serialVersionUID = 1L;
 
     @XmlElement(name = "enabled", defaultValue = "false")
@@ -123,6 +126,15 @@ public class HaConfiguration implements Serializable {
      * relative to {@code $OPENNMS_HOME/etc}. A trailing {@code /} excludes a
      * subtree. {@code ha-configuration.xml} is always excluded regardless.
      */
+    /**
+     * Directories under {@code $OPENNMS_HOME} to synchronize. Defaults to
+     * {@code etc} alone; add {@code deploy} to replicate installed plugins.
+     * {@code data} must never be listed — it is each node's own Karaf cache.
+     */
+    @XmlElementWrapper(name = "sync-roots")
+    @XmlElement(name = "root")
+    private List<String> syncRoots;
+
     @XmlElementWrapper(name = "sync-excludes")
     @XmlElement(name = "exclude")
     private List<String> syncExcludes = new ArrayList<>();
@@ -227,6 +239,14 @@ public List<String> getSyncExcludes() {
         return syncExcludes == null ? new ArrayList<>() : syncExcludes;
     }
 
+    public List<String> getSyncRoots() {
+        return syncRoots == null || syncRoots.isEmpty() ? List.of(DEFAULT_SYNC_ROOT) : syncRoots;
+    }
+
+    public void setSyncRoots(List<String> syncRoots) {
+        this.syncRoots = syncRoots;
+    }
+
     public void setSyncExcludes(List<String> syncExcludes) {
         this.syncExcludes = syncExcludes;
     }

features/ha-management/ha-daemon/src/main/java/org/opennms/netmgt/ha/HaConfigSyncer.java

@@ -80,8 +80,37 @@ public class HaConfigSyncer {
     private static final Pattern SCV_PATTERN =
             Pattern.compile("^\\$\\{scv:([^:|}]+):([^:|}]+)(?:\\|([^|}]*))?\\}$");
 
+    /** Where a cycle's outcome is published so operators can see it without
+     * reading a standby's log. */
+    public interface StatusRecorder {
+        void syncSucceeded();
+        void syncFailed(String reason);
+        /** A file read only at JVM startup was replaced; this node needs a
+         * restart before the new values take effect. */
+        void bootConfigChanged();
+    }
+
+    private static final StatusRecorder NO_RECORDER = new StatusRecorder() {
+        @Override public void syncSucceeded() {}
+        @Override public void syncFailed(String reason) {}
+        @Override public void bootConfigChanged() {}
+    };
+
+    /** Read once into system properties before services start, so replacing
+     * one leaves the running JVM behind its own files until it restarts. In
+     * practice this fires for opennms.properties.d/, where operator settings
+     * belong; opennms.properties itself is product-owned and identical across
+     * same-version nodes. The credential vault is deliberately absent — it
+     * reloads on change, and flagging routine rotation would devalue the
+     * signal (residual: a rotated credential Bootstrap already expanded from a
+     * ${scv:...} expression stays stale unflagged). */
+    private static final String BOOT_ONLY_DIR = "opennms.properties.d/";
+    private static final List<String> BOOT_ONLY_FILES = List.of(
+            "opennms.properties", "bootstrap.properties", "libraries.properties");
+
     private final Supplier<HaConfiguration> configSupplier;
     private final Supplier<HaInstanceState> stateSupplier;
+    private final StatusRecorder recorder;
     private final HttpClient httpClient;
 
     /** Constructor used in tests and when no state tracking is needed (always treats self as STANDBY). */
@@ -100,8 +129,14 @@ public HaConfigSyncer(HaConfiguration config, Supplier<HaInstanceState> stateSup
      * cycle without restarting the syncer.
      */
     public HaConfigSyncer(Supplier<HaConfiguration> configSupplier, Supplier<HaInstanceState> stateSupplier) {
+        this(configSupplier, stateSupplier, NO_RECORDER);
+    }
+
+    public HaConfigSyncer(Supplier<HaConfiguration> configSupplier, Supplier<HaInstanceState> stateSupplier,
+                          StatusRecorder recorder) {
         this.configSupplier = configSupplier;
         this.stateSupplier = stateSupplier;
+        this.recorder = recorder != null ? recorder : NO_RECORDER;
         this.httpClient = HttpClient.newBuilder()
                 .connectTimeout(Duration.ofSeconds(10))
                 .build();
@@ -126,13 +161,15 @@ public void sync() {
 
         if (config.getPartnerRestUrl() == null || config.getPartnerRestUrl().isBlank()) {
             LOG.warn("HA sync: partner-rest-url is not configured; skipping config sync");
+            recorder.syncFailed("partner-rest-url is not configured");
             return;
         }
 
         String resolvedPassword = resolveScvExpression(config.getSyncPassword());
         if (config.getSyncUsername() == null || resolvedPassword == null) {
             LOG.warn("HA sync: credentials not available (check SCV entry '{}'); skipping sync",
                     extractScvAlias(config.getSyncPassword()));
+            recorder.syncFailed("credentials not available");
             return;
         }
 
@@ -147,21 +184,31 @@ public void sync() {
         if (!HaSyncFiles.isManifest(manifestText)) {
             LOG.warn("HA sync: response from {} is not an HA manifest; skipping cycle "
                     + "(check partner-rest-url — a wrong host can answer 200 with unrelated text)", baseUrl);
+            recorder.syncFailed("partner response is not an HA manifest");
+            return;
+        }
+        String partnerVersion = HaSyncFiles.parseManifestVersion(manifestText);
+        String localVersion = HaSyncFiles.localVersion();
+        if (partnerVersion != null && localVersion != null && !partnerVersion.equals(localVersion)) {
+            LOG.warn("HA sync: partner runs {} but this node runs {}; skipping cycle until both nodes match "
+                    + "(configuration formats can differ between releases)", partnerVersion, localVersion);
+            recorder.syncFailed("version mismatch: partner " + partnerVersion + ", local " + localVersion);
             return;
         }
         List<HaSyncFiles.Entry> manifest = HaSyncFiles.parseManifestText(manifestText);
         if (manifest.isEmpty()) {
             LOG.warn("HA sync: manifest from {} was empty; skipping cycle (refusing to delete everything)", baseUrl);
+            recorder.syncFailed("partner manifest was empty");
             return;
         }
 
-        Path etcRoot = HaSyncFiles.etcRoot();
         int fetched = 0;
         int failed = 0;
+        boolean bootConfigChanged = false;
         Set<String> manifestPaths = new HashSet<>();
 
         for (HaSyncFiles.Entry entry : manifest) {
-            manifestPaths.add(entry.relativePath());
+            manifestPaths.add(entry.root() + '/' + entry.relativePath());
             // The serving side applies exclusions too, but the local list may
             // legitimately be stricter — never let the partner overwrite an
             // excluded file.
@@ -175,11 +222,15 @@ public void sync() {
                 return;
             }
             try {
-                if (localMatches(etcRoot, entry)) {
+                Path root = HaSyncFiles.root(entry.root());
+                if (localMatches(root, entry)) {
                     continue;
                 }
-                fetchFile(baseUrl, authHeader, etcRoot, entry);
+                fetchFile(baseUrl, authHeader, root, entry);
                 fetched++;
+                if (isBootOnly(entry)) {
+                    bootConfigChanged = true;
+                }
             } catch (Exception e) {
                 if (stateSupplier.get() == HaInstanceState.ACTIVE) {
                     LOG.warn("HA sync: this instance became ACTIVE mid-cycle; aborting sync");
@@ -197,13 +248,29 @@ public void sync() {
         if (excludes != null) {
             deletionExcludes.addAll(excludes);
         }
-        int deleted = propagateDeletions(etcRoot, manifestPaths, deletionExcludes);
+        int deleted = propagateDeletions(config.getSyncRoots(), manifestPaths, deletionExcludes);
 
         if (fetched > 0 || failed > 0 || deleted > 0) {
             LOG.info("HA sync complete: {} files fetched, {} deleted, {} failed", fetched, deleted, failed);
         } else {
             LOG.debug("HA sync complete: no changes");
         }
+        if (bootConfigChanged) {
+            LOG.warn("HA sync: replaced configuration that is only read at startup; this node must be "
+                    + "restarted before those values take effect");
+            recorder.bootConfigChanged();
+        }
+        recorder.syncSucceeded();
+    }
+
+    /** True if the entry is read once at JVM startup (so syncing it leaves the
+     * running process behind its own files until restarted). */
+    private static boolean isBootOnly(HaSyncFiles.Entry entry) {
+        if (!HaSyncFiles.DEFAULT_ROOT.equals(entry.root())) {
+            return false;
+        }
+        String p = entry.relativePath();
+        return BOOT_ONLY_FILES.contains(p) || p.startsWith(BOOT_ONLY_DIR);
     }
 
     // -------------------------------------------------------------------------
@@ -247,7 +314,8 @@ private void fetchFile(String baseUrl, String authHeader, Path etcRoot, HaSyncFi
 
         String encoded = URLEncoder.encode(entry.relativePath(), StandardCharsets.UTF_8);
         HttpRequest request = HttpRequest.newBuilder()
-                .uri(URI.create(baseUrl + "/rest/ha/sync/file?f=" + encoded))
+                .uri(URI.create(baseUrl + "/rest/ha/sync/file?root="
+                        + URLEncoder.encode(entry.root(), StandardCharsets.UTF_8) + "&f=" + encoded))
                 .header("Authorization", authHeader)
                 .GET()
                 .timeout(Duration.ofSeconds(120))
@@ -286,13 +354,14 @@ private void fetchFile(String baseUrl, String authHeader, Path etcRoot, HaSyncFi
 
     /** Deletes local in-scope files that no longer exist on the partner.
      * {@code excludes} must already be the union of both nodes' exclusions. */
-    private int propagateDeletions(Path etcRoot, Set<String> manifestPaths, List<String> excludes) {
+    private int propagateDeletions(List<String> roots, Set<String> manifestPaths, List<String> excludes) {
         int deleted = 0;
         try {
-            for (HaSyncFiles.Entry local : HaSyncFiles.buildManifest(etcRoot, excludes)) {
-                if (manifestPaths.contains(local.relativePath())) {
+            for (HaSyncFiles.Entry local : HaSyncFiles.buildManifest(roots, excludes)) {
+                if (manifestPaths.contains(local.root() + '/' + local.relativePath())) {
                     continue;
                 }
+                Path etcRoot = HaSyncFiles.root(local.root());
                 if (stateSupplier.get() == HaInstanceState.ACTIVE) {
                     LOG.warn("HA sync: this instance became ACTIVE mid-cycle; aborting deletion propagation");
                     return deleted;

features/ha-management/ha-daemon/src/main/java/org/opennms/netmgt/ha/HaStartupCoordinator.java

@@ -437,6 +437,7 @@ private static boolean configEquivalent(HaConfiguration a, HaConfiguration b) {
             && Objects.equals(a.getPartnerRestUrl(),    b.getPartnerRestUrl())
             && Objects.equals(a.getSyncUsername(),      b.getSyncUsername())
             && Objects.equals(a.getSyncPassword(),      b.getSyncPassword())
+            && Objects.equals(a.getSyncRoots(),         b.getSyncRoots())
             && Objects.equals(a.getSyncExcludes(),      b.getSyncExcludes());
     }
 
@@ -731,7 +732,7 @@ private void startSyncIfApplicable() {
             LOG.debug("HA: this instance is ACTIVE; not starting config sync");
             return;
         }
-        HaConfigSyncer syncer = new HaConfigSyncer(this::getConfig, this::getCurrentState);
+        HaConfigSyncer syncer = new HaConfigSyncer(this::getConfig, this::getCurrentState, syncStatusRecorder());
         LOG.info("HA: config sync started — partner {}, interval {}s",
                 cfg.getPartnerRestUrl(), cfg.getSyncIntervalSeconds());
         syncFuture = scheduler.scheduleAtFixedRate(() -> {
@@ -741,6 +742,39 @@ private void startSyncIfApplicable() {
         }, 0, cfg.getSyncIntervalSeconds(), TimeUnit.SECONDS);
     }
 
+    /** Publishes each sync cycle's outcome into this node's own row, so a
+     * broken sync is visible from the partner instead of only in this node's
+     * log — a gated standby serves no REST and raises no events. */
+    private HaConfigSyncer.StatusRecorder syncStatusRecorder() {
+        return new HaConfigSyncer.StatusRecorder() {
+            @Override public void syncSucceeded() {
+                writeSyncStatus("last_sync_attempt = NOW(), last_sync_success = NOW(), last_sync_error = NULL", null);
+            }
+            @Override public void syncFailed(String reason) {
+                writeSyncStatus("last_sync_attempt = NOW(), last_sync_error = ?", reason);
+            }
+            @Override public void bootConfigChanged() {
+                writeSyncStatus("boot_config_changed_at = NOW()", null);
+            }
+        };
+    }
+
+    private void writeSyncStatus(String assignments, String errorParam) {
+        try (Connection conn = dbFactory.getConnection()) {
+            String sql = "UPDATE ha_instance_status SET " + assignments + " WHERE instance_id = ?";
+            try (PreparedStatement ps = conn.prepareStatement(sql)) {
+                int idx = 1;
+                if (errorParam != null) {
+                    ps.setString(idx++, errorParam);
+                }
+                ps.setString(idx, config.getInstanceId());
+                ps.executeUpdate();
+            }
+        } catch (Exception e) {
+            LOG.debug("HA: could not record sync status", e);
+        }
+    }
+
     private static boolean cancelIfActive(ScheduledFuture<?> f) {
         if (f != null && !f.isDone() && !f.isCancelled()) {
             f.cancel(false); // graceful: let in-flight task complete
@@ -832,6 +866,7 @@ private void writeInitialStatus(HaInstanceState initialState) throws Exception {
                          "current_state = EXCLUDED.current_state, " +
                          "last_heartbeat = NOW(), " +
                          "hostname = EXCLUDED.hostname, " +
+                         "boot_config_changed_at = NULL, " +
                          "active_since = EXCLUDED.active_since";
             try (PreparedStatement ps = conn.prepareStatement(sql)) {
                 ps.setString(1, config.getInstanceId());

features/ha-management/ha-daemon/src/main/java/org/opennms/netmgt/ha/HaStatusSchema.java

@@ -66,6 +66,13 @@ last_heartbeat  TIMESTAMPTZ NOT NULL DEFAULT now(),
                 active_since    TIMESTAMPTZ,
                 hostname        TEXT
             )""",
+            // v2: config-sync observability and boot-config staleness
+            """
+            ALTER TABLE ha_instance_status
+                ADD COLUMN IF NOT EXISTS last_sync_attempt      TIMESTAMPTZ,
+                ADD COLUMN IF NOT EXISTS last_sync_success      TIMESTAMPTZ,
+                ADD COLUMN IF NOT EXISTS last_sync_error        TEXT,
+                ADD COLUMN IF NOT EXISTS boot_config_changed_at TIMESTAMPTZ""",
     };
 
     private HaStatusSchema() {}