Merge pull request #594 from lmiccini/split-brain-def

Add network partition detection to prevent unsafe fencing

Author: openshift-merge-bot[bot]

apis/instanceha/v1beta1/instanceha_webhook.go

@@ -121,6 +121,18 @@ func (r *InstanceHa) ValidateUpdate(ctx context.Context, c client.Client, old ru
 	var allWarn []string
 	basePath := field.NewPath("spec")
 
+	oldInstance, ok := old.(*InstanceHa)
+	if !ok {
+		return nil, apierrors.NewInternalError(fmt.Errorf("unable to convert existing object"))
+	}
+
+	if r.Spec.OpenStackCloud != oldInstance.Spec.OpenStackCloud {
+		allErrs = append(allErrs, field.Forbidden(
+			basePath.Child("openStackCloud"),
+			"openStackCloud cannot be changed after creation",
+		))
+	}
+
 	allErrs = append(allErrs, r.Spec.ValidateTopology(basePath, r.Namespace)...)
 	allErrs = append(allErrs, r.validateUniqueOpenStackCloud(ctx, c, basePath)...)
 	allErrs = append(allErrs, r.validateCipherSuites(basePath)...)

docs/instanceha_application_credentials.md

@@ -9,8 +9,7 @@ access that can be rotated independently of the service user password.
 
 - A Keystone user with sufficient privileges to query and evacuate VMs
   (typically the `admin` role on the relevant project).
-- The `keystone-operator` deployed with Application Credential support
-  ([keystone-operator PR #567](https://github.com/openstack-k8s-operators/keystone-operator/pull/567)).
+- The `keystone-operator` deployed with `KeystoneApplicationCredential` CR support.
 - The `clouds.yaml` ConfigMap must still be present — InstanceHA reads
   `auth_url` and `region_name` from it even when using Application Credentials.
 

docs/instanceha_architecture.md

@@ -4,8 +4,6 @@
 
 InstanceHA is a high-availability service for OpenStack that automatically detects and evacuates instances from failed compute nodes.
 
-**Version**: 2.5
-
 ## Table of Contents
 
 1. [Core Components](#core-components)
@@ -126,13 +124,15 @@ _config_map: Dict[str, ConfigItem] = {
     'KDUMP_TIMEOUT': ConfigItem('int', 30, 5, 300),
     'CHECK_HEARTBEAT': ConfigItem('bool', False),
     'HEARTBEAT_TIMEOUT': ConfigItem('int', 120, 30, 600),
+    'HEARTBEAT_CLIFF_THRESHOLD': ConfigItem('int', 50, 10, 100),
     'DISABLED': ConfigItem('bool', False),
     'SSL_VERIFY': ConfigItem('bool', True),
     'FENCING_TIMEOUT': ConfigItem('int', 30, 5, 120),
     'HASH_INTERVAL': ConfigItem('int', 60, 30, 300),
     'ORCHESTRATED_RESTART': ConfigItem('bool', False),
     'SKIP_SERVERS_WITH_NAME': ConfigItem('list', []),
     'EVACUATION_RETRIES': ConfigItem('int', DEFAULT_EVACUATION_RETRIES, 1, 20),  # DEFAULT_EVACUATION_RETRIES = 5
+    'K8S_API_CHECK_INTERVAL': ConfigItem('int', 15, 5, 120),
 }
 ```
 
@@ -158,10 +158,11 @@ def __init__(self, config_manager: ConfigManager, cloud_client: Optional[OpenSta
 
     # Health monitoring
     self.current_hash = ""
-    self.hash_update_successful = True
+    self.hash_update_successful = False
     self._last_hash_time = 0
     self._previous_hash = ""
     self.ready = False                              # Readiness flag (set after first poll)
+    self.k8s_api_reachable = False                  # K8s API connectivity (partition detection)
 
     # Caching
     self._host_servers_cache = {}
@@ -188,6 +189,7 @@ def __init__(self, config_manager: ConfigManager, cloud_client: Optional[OpenSta
     self.heartbeat_hosts_timestamp = defaultdict(float)
     self.heartbeat_listener_stop_event = threading.Event()
     self.heartbeat_listener_start_time = 0.0
+    self.heartbeat_previous_active_count = 0        # Cliff detection baseline
 
     # Host processing tracking
     self.hosts_processing = defaultdict(float)
@@ -1108,6 +1110,7 @@ In addition, each event emission site also increments a corresponding Prometheus
 | `AggregateThresholdExceeded` | Warning | aggregate name | Failed hosts in aggregate exceed `instanceha:max_failures` metadata limit |
 | `HostReachable` | Warning | compute host | Host reported down by Nova but still reachable via heartbeat (CHECK_HEARTBEAT) |
 | `HostReenabled` | Normal | compute host | Host re-enabled after successful evacuation |
+| `HeartbeatCliff` | Warning | `cluster` | Sudden heartbeat loss detected — possible network partition, fencing skipped for this cycle |
 | `OrphanedHostRecovered` | Warning | compute host | Startup reconciliation recovered a fenced host left without evacuation marker |
 
 ### Event Structure
@@ -1181,39 +1184,7 @@ kubectl get events -n openstack --field-selector involvedObject.kind=InstanceHa,
 
 ### Prometheus Metrics
 
-The agent exposes Prometheus-format metrics at `:8080/metrics` on the same HTTP server used for health checks. Metrics are served using the `prometheus_client` Python library.
-
-**Counters** (monotonically increasing, reset on pod restart):
-
-| Metric | Labels | Description |
-|--------|--------|-------------|
-| `instanceha_fencing_total` | `host`, `result` | Fencing operations (`started`/`succeeded`/`failed`) |
-| `instanceha_evacuation_total` | `host`, `result` | Host-level evacuations |
-| `instanceha_instance_evacuation_total` | `host`, `result` | Per-instance evacuations (smart/orchestrated) |
-| `instanceha_host_down_total` | `host` | Host-down detections |
-| `instanceha_host_reachable_total` | `host` | Hosts still reachable via heartbeat despite Nova reporting down |
-| `instanceha_host_reenabled_total` | `host` | Hosts re-enabled after evacuation |
-| `instanceha_threshold_exceeded_total` | | Evacuations blocked by global threshold |
-| `instanceha_aggregate_threshold_exceeded_total` | `aggregate` | Evacuations blocked by per-aggregate threshold |
-| `instanceha_recovery_completed_total` | `host` | Full recovery workflows completed |
-| `instanceha_processing_failed_total` | `host` | Service processing failures |
-| `instanceha_orphaned_host_recovered_total` | | Orphaned hosts recovered at startup |
-| `instanceha_poll_cycles_total` | `result` | Poll cycles (`success`/`error`) |
-
-**Gauges** (current value):
-
-| Metric | Description |
-|--------|-------------|
-| `instanceha_poll_consecutive_failures` | Current consecutive Nova API failures |
-| `instanceha_hosts_processing` | Hosts currently being processed |
-
-**Histograms**:
-
-| Metric | Labels | Buckets (s) | Description |
-|--------|--------|-------------|-------------|
-| `instanceha_instance_evacuation_duration_seconds` | `host` | 10, 30, 60, 120, 180, 300, 600 | Per-instance evacuation duration |
-
-Each metric increment is co-located with the corresponding `_emit_k8s_event()` call, so the event catalog and metric catalog map 1:1.
+The agent exposes Prometheus-format metrics at `:8080/metrics` on the same HTTP server used for health checks. Each metric increment is co-located with the corresponding `_emit_k8s_event()` call. See [instanceha_prometheus.md](instanceha_prometheus.md) for the full metric catalog, alert rules, and Grafana dashboard queries.
 
 ---
 
@@ -1303,7 +1274,48 @@ Minimum packet size: 45 bytes (4 magic + 8 timestamp + 1 hostname + 32 HMAC).
 
 ---
 
-### 3. Reserved Hosts
+### 3. K8s API Connectivity Check
+
+**Purpose**: Detect when the InstanceHA pod's worker node is network-isolated from the Kubernetes API, preventing it from fencing healthy hosts it can no longer observe.
+
+**Architecture**:
+- Background thread started by `start_k8s_health_check()`
+- Polls `GET /api/v1/namespaces/{namespace}` using the in-cluster service account token
+- Interval controlled by `K8S_API_CHECK_INTERVAL` (default 15s, range 5–120)
+
+**Configuration**: `K8S_API_CHECK_INTERVAL: 15`
+
+**Behavior**:
+1. The health check thread pings the K8s API every `K8S_API_CHECK_INTERVAL` seconds
+2. Any non-5xx response (status code < 500) is treated as reachable — the API is responding, even if RBAC denies the specific request
+3. After 3 consecutive failures, `k8s_api_reachable` is set to `False` and `ready` is set to `False`
+4. The main poll loop checks `k8s_api_reachable` before fencing — if `False`, the entire fencing cycle is skipped
+5. When connectivity restores, `k8s_api_reachable` is set back to `True`; readiness is restored after the next successful Nova poll
+
+**Prometheus**: `instanceha_k8s_api_reachable` gauge (1=ok, 0=isolated)
+
+---
+
+### 4. Heartbeat Cliff Detection
+
+**Purpose**: Detect sudden mass heartbeat loss that is more likely caused by a listener-side network partition than simultaneous host failures.
+
+**Architecture**:
+- Integrated into `_filter_reachable_hosts()`, runs during each poll cycle when `CHECK_HEARTBEAT` is enabled
+- Compares current active heartbeat count against the previous cycle's count stored in `heartbeat_previous_active_count`
+
+**Configuration**: `HEARTBEAT_CLIFF_THRESHOLD: 50` (default 50%, range 10–100)
+
+**Behavior**:
+1. At each poll cycle, count hosts with a fresh heartbeat (within `HEARTBEAT_TIMEOUT`)
+2. If the previous count was ≥ 3 and the drop percentage exceeds `HEARTBEAT_CLIFF_THRESHOLD`, skip fencing for this cycle
+3. Emit a `HeartbeatCliff` K8s event (Warning) and increment `instanceha_heartbeat_cliff_total`
+4. The baseline count is preserved during a cliff event, so the cliff re-triggers on subsequent cycles until heartbeats recover
+5. When no cliff is detected, update `heartbeat_previous_active_count` for the next cycle
+
+---
+
+### 5. Reserved Hosts
 
 **Purpose**: Maintain spare capacity by auto-enabling reserved hosts and optionally forcing evacuation to them.
 
@@ -1349,7 +1361,7 @@ Reserved Hosts: reserved-01 (aggregate-A), reserved-02 (aggregate-B)
 
 ---
 
-### 4. Caching System
+### 6. Caching System
 
 **Purpose**: Cache evacuable resources to reduce Nova API calls.
 
@@ -1367,7 +1379,7 @@ Reserved Hosts: reserved-01 (aggregate-A), reserved-02 (aggregate-B)
 
 ---
 
-### 5. Threshold Protection
+### 7. Threshold Protection
 
 **Purpose**: Prevent mass evacuations during datacenter-level failures.
 
@@ -1844,35 +1856,7 @@ These values are passed as `METRICS_TLS_MIN_VERSION` and `METRICS_TLS_CIPHERS` e
 
 ## Performance
 
-### 1. Caching Performance
-
-**Benchmark** (100 flavors, 100 images):
-- First call: ~50ms (with API call)
-- Cached call: ~0.1ms (without API call)
-
----
-
-### 2. Concurrent Processing
-
-**ThreadPoolExecutor** for smart evacuation:
-```python
-with concurrent.futures.ThreadPoolExecutor(max_workers=WORKERS) as executor:
-    future_to_server = {
-        executor.submit(_server_evacuate_future, conn, srv): srv
-        for srv in evacuables
-    }
-```
-
-**Concurrency**: WORKERS controls the number of hosts processed concurrently and, for smart/orchestrated mode, the per-host server concurrency (capped at `MAX_TOTAL_EVACUATION_THREADS=32`).
-
----
-
-### 3. Memory Management
-
-**Cleanup Strategies**:
-- Kdump timestamp cleanup (>2000 entries)
-- Host processing expiration
-- Generic cleanup helper
+For heartbeat filter latency, UDP listener throughput, lock contention, memory overhead, and scalability projections at 1000-node scale, see [instanceha_heartbeat_performance.md](instanceha_heartbeat_performance.md).
 
 ---
 

docs/instanceha_guide.md

@@ -51,7 +51,7 @@ InstanceHA runs as a Kubernetes-managed workload alongside the OpenStack control
 
 ### How It Works
 
-1. **Detection**: The agent polls the Nova API every `POLL` seconds (default: 45). It queries all `nova-compute` services and checks each one's `updated_at` timestamp and `state` field. A service is considered failed if it is reported as `down` or if its heartbeat is staler than `DELTA` seconds (default: 30). When heartbeat verification is enabled (`CHECK_HEARTBEAT`), both the Nova service-list poll and the UDP heartbeat channel must agree that a host is unreachable before it is marked as failed.
+1. **Detection**: The agent polls the Nova API every `POLL` seconds (default: 45). It queries all `nova-compute` services and checks each one's `updated_at` timestamp. A service is considered failed if it is reported as `down` or if its `updated_at` timestamp is older than `DELTA` seconds (default: 30). When heartbeat verification is enabled (`CHECK_HEARTBEAT`), both the Nova service-list poll and the UDP heartbeat channel must agree that a host is unreachable before it is marked as failed.
 
 2. **Safety checks**: Before acting, the agent verifies that the percentage of failed hosts does not exceed the `THRESHOLD` (default: 50%) — calculated against only active services, excluding already-disabled and already-forced-down hosts — and that at least one `nova-scheduler` is running. These checks prevent cascading evacuations during infrastructure-wide outages. A `ThresholdExceeded` K8s event is emitted when the limit is hit.
 
@@ -121,6 +121,7 @@ InstanceHA emits Kubernetes events on the InstanceHa CR to provide observability
 | `ThresholdExceeded` | Warning | Too many hosts down simultaneously |
 | `AggregateThresholdExceeded` | Warning | Per-aggregate failure threshold exceeded |
 | `OrphanedHostRecovered` | Warning | Recovered fenced host from prior crash |
+| `HeartbeatCliff` | Warning | Sudden heartbeat loss detected — possible network partition, fencing skipped |
 
 Events can be viewed with `oc describe instanceha <name>` or `oc get events --field-selector involvedObject.name=<name>`.
 
@@ -144,50 +145,24 @@ In addition to Kubernetes Events, the agent exposes Prometheus metrics at `http:
 | `instanceha_processing_failed_total` | `host` | Service processing failures |
 | `instanceha_orphaned_host_recovered_total` | | Orphaned hosts recovered at startup |
 | `instanceha_poll_cycles_total` | `result` | Poll cycles executed (result: `success`, `error`) |
+| `instanceha_heartbeat_rejected_total` | `reason` | Heartbeat packets rejected (`hmac_failed`, `timestamp_invalid`) |
+| `instanceha_heartbeat_cliff_total` | | Fencing skipped due to sudden heartbeat loss (possible network partition) |
 
 #### Gauges
 
 | Metric | Description |
 |--------|-------------|
 | `instanceha_poll_consecutive_failures` | Current consecutive Nova API poll failures (resets to 0 on success) |
 | `instanceha_hosts_processing` | Number of hosts currently being processed |
+| `instanceha_k8s_api_reachable` | Whether the Kubernetes API is reachable (1=yes, 0=no) |
 
 #### Histograms
 
 | Metric | Labels | Buckets (seconds) | Description |
 |--------|--------|-------------------|-------------|
 | `instanceha_instance_evacuation_duration_seconds` | `host` | 10, 30, 60, 120, 180, 300, 600 | Duration of individual instance evacuations |
 
-#### Example Alert Rules
-
-```yaml
-groups:
-  - name: instanceha
-    rules:
-      - alert: InstanceHAFencingFailure
-        expr: increase(instanceha_fencing_total{result="failed"}[5m]) > 0
-        for: 0m
-        labels:
-          severity: critical
-        annotations:
-          summary: "Fencing failed for host {{ $labels.host }}"
-
-      - alert: InstanceHANovaAPIDown
-        expr: instanceha_poll_consecutive_failures > 3
-        for: 2m
-        labels:
-          severity: warning
-        annotations:
-          summary: "InstanceHA cannot reach Nova API"
-
-      - alert: InstanceHAEvacuationSlow
-        expr: histogram_quantile(0.95, rate(instanceha_instance_evacuation_duration_seconds_bucket[15m])) > 300
-        for: 5m
-        labels:
-          severity: warning
-        annotations:
-          summary: "Instance evacuations taking longer than 5 minutes (p95)"
-```
+See [instanceha_prometheus.md](instanceha_prometheus.md) for alert rules, Grafana dashboard queries, and PromQL examples.
 
 #### Scraping Configuration
 
@@ -215,6 +190,27 @@ spec:
 
 When the pod receives SIGTERM (during scaling, rolling updates, or node drain), InstanceHA finishes any in-flight fencing or evacuation work before exiting. The Deployment is configured with a 30-second termination grace period. The agent's poll loop and evacuation workers check a shutdown flag between cycles, allowing timely response to shutdown signals.
 
+### Network Partition Detection
+
+Two complementary mechanisms prevent the agent from fencing healthy hosts when the pod's worker node is network-isolated:
+
+**K8s API connectivity check:**
+- A background thread pings the K8s API every `K8S_API_CHECK_INTERVAL` seconds (default: 15) using the in-cluster service account.
+- After 3 consecutive failures, the agent marks itself not-ready and blocks all fencing until connectivity restores and a Nova poll succeeds.
+- The `instanceha_k8s_api_reachable` Prometheus gauge tracks the current state (1=ok, 0=isolated).
+
+**Heartbeat cliff detection:**
+- The agent tracks the count of active heartbeat hosts between poll cycles.
+- If the count drops by more than `HEARTBEAT_CLIFF_THRESHOLD`% (default: 50%) in a single cycle (minimum 3 hosts previously active), fencing is skipped for that cycle. A sudden mass-silence is far more likely to indicate a listener-side network issue than simultaneous host failures.
+- A `HeartbeatCliff` K8s event (Warning) is emitted and the `instanceha_heartbeat_cliff_total` counter is incremented.
+- The detection re-evaluates on the next cycle — the baseline is preserved during a cliff, so fencing stays blocked until heartbeats recover.
+
+```yaml
+config:
+  K8S_API_CHECK_INTERVAL: "15"
+  HEARTBEAT_CLIFF_THRESHOLD: "50"
+```
+
 ---
 
 ## Deployment
@@ -334,6 +330,7 @@ metadata:
     metallb.universe.tf/loadBalancerIPs: 172.17.0.80
 spec:
   type: LoadBalancer
+  externalTrafficPolicy: Local
   selector:
     service: instanceha  # Replace with your CR name if different
   ports:
@@ -347,6 +344,8 @@ spec:
     protocol: UDP
 ```
 
+> **Important:** `externalTrafficPolicy: Local` is required when kdump detection is enabled (`CHECK_KDUMP: "true"`). The kdump listener identifies the crashing host via reverse DNS lookup on the packet's source IP. Without `Local` policy, kube-proxy masquerades (SNATs) the source IP when forwarding traffic to the pod, replacing the compute node's real IP with an internal node address — causing the reverse DNS lookup to fail. `Local` policy preserves the original source IP by delivering traffic directly to the pod on the receiving node without cross-node forwarding or SNAT. (Heartbeat packets are unaffected — the sender's hostname is embedded in the HMAC-authenticated packet payload, so source IP rewriting does not break heartbeat identification.)
+
 Replace `172.17.0.80` with an IP from your `internalapi` MetalLB address pool. The `allow-shared-ip` annotation lets this Service share the same IP with other OpenStack services (e.g., keystone, nova, cinder) that already use the `internalapi` pool — since the InstanceHA ports (7410/UDP, 7411/UDP) don't conflict with any existing service ports, sharing is safe and avoids consuming an additional IP. If you prefer a dedicated IP, pick an unused address and remove the `allow-shared-ip` annotation.
 
 > **Note:** The `address-pool` value must match the actual MetalLB IPAddressPool name. In a standard openstack-k8s-operators deployment this is `internalapi`. You can verify the pool name with `oc get ipaddresspools -n metallb-system`.
@@ -880,6 +879,8 @@ All values are strings in the ConfigMap. The agent converts and validates them a
 |-----------|------|---------|-------------|
 | `CHECK_KDUMP` | bool | false | Enable kdump detection via UDP listener |
 | `CHECK_HEARTBEAT` | bool | false | Enable heartbeat verification via UDP listener |
+| `HEARTBEAT_CLIFF_THRESHOLD` | int | 50 (range: 10–100) | Percentage drop in active heartbeat hosts that triggers cliff detection (skips fencing for that cycle) |
+| `K8S_API_CHECK_INTERVAL` | int | 15 (range: 5–120) | Seconds between Kubernetes API reachability checks |
 
 #### Security
 

docs/instanceha_heartbeat_performance.md

@@ -120,6 +120,21 @@ nova-compute crashed (heartbeat still arriving). It acquires the
 against the timestamp map. Filter performance is unaffected by the
 protocol version since it operates on the in-memory timestamp dictionary.
 
+#### Cliff Detection Overhead
+
+Before filtering individual hosts, the function performs cliff detection:
+it counts the number of hosts with a fresh heartbeat (within
+`HEARTBEAT_TIMEOUT`) by scanning all entries in the timestamp snapshot,
+and compares this count against the previous cycle's value. If the drop
+exceeds `HEARTBEAT_CLIFF_THRESHOLD` (default 50%) and the previous count
+was ≥ 3, fencing is skipped entirely for that cycle.
+
+This adds an O(n) pass over all timestamps (where n = total nodes, not
+just down hosts). At 1000 nodes this is a single dict iteration with one
+comparison per entry — well under 100 us and negligible compared to the
+Nova API call latency. The per-host filter logic that follows is
+unchanged.
+
 ### Latency by Down-Host Count
 
 The THRESHOLD determines the maximum number of simultaneously down hosts