Do not reset keep-connections flag after host add

[gbregman]

Fixes: #2144

[Copilot]

🔵 Needs a closer look

Fix the keep-connection assertion and propagate gateway wait failures.

Pull request overview

Fixes an HA race where re-adding a host prematurely cleared the pending keep-connections indication.

Changes:

• Preserves the indication during host re-addition.
• Adds HA setup and delayed-state configuration.
• Expands regression coverage for rapid host deletion and re-addition.

File summaries

File	Summary
control/grpc.py	Preserves keep-connections state during host re-addition.
tests/ha/keep_connection.sh	Tests delayed propagation and rapid host re-addition.
tests/ha/start_up_keep_connection.sh	Configures delayed gateway state updates.
tests/ha/wait_gateways_keep_connection.sh	Waits for gateways and restores configuration.
tests/ha/setup_keep_connection.sh	Sets up HA test resources.

Review details

Suppressed comments (2)

tests/ha/keep_connection.sh:322

• GatewayState.add_host() explicitly removes connected-del-host_${subsys}_${host} from OMAP when a host is re-added (control/state.py:317-321). Since line 318 re-adds host3, this positive grep will find no match and abort the test before the final no-keep deletion; assert that the key is absent here (or remove this check), while the preceding log assertions already verify that the delayed remove carried keep connections: True.

make -s exec SVC=ceph OPTS=-T CMD="rados --pool rbd listomapvals nvmeof.state" | grep "connected-del-host_${NQN}_${NQN}host3"

tests/ha/wait_gateways_keep_connection.sh:8

• If wait_gateways.sh exits nonzero, this wrapper continues to restore the config and the successful mv becomes the script's exit status because no set -e is enabled. A gateway readiness failure can therefore be reported as a passing setup step; propagate the wait command's failure before restoring the file.

$test_dir/wait_gateways.sh 2

• Files reviewed: 5/5 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Unresolved persistent-state handling and HA test failures block approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

tests/ha/keep_connection.sh:322

• This repeats the same inverted expectation for host3: adding a host clears its connected-host indication in GatewayState.add_host() (control/state.py:312-321), so this positive grep fails before the cleanup delete is reached. Please assert that the key is absent here (or drop this redundant check).

make -s exec SVC=ceph OPTS=-T CMD="rados --pool rbd listomapvals nvmeof.state" | grep "connected-del-host_${NQN}_${NQN}host3"

tests/ha/wait_gateways_keep_connection.sh:8

• This wrapper does not enable errexit, so if wait_gateways.sh fails, execution continues to the restore commands and the final mv normally returns success. The workflow can therefore mark the wait step successful and run the test against gateways that never became ready; enable set -e before invoking the helper.

$test_dir/wait_gateways.sh 2

• Files reviewed: 5/5 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Unresolved stale keep-indication behavior and insufficient regression assertions block approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

control/grpc.py:6283

• This removal can leave a stale in-memory keep indication on a peer that observes an immediate delete/re-add with an unchanged host payload. In that case GatewayStateHandler.update() sees the marker addition but no host-key change, so no remove_host_safe() call resets the indication; a later non-keep deletion on a connected host then uses the stale indication and sends keep_connections=True despite the authoritative request being false. The pending indication needs an explicit lifecycle that preserves it through the replayed keep-removal but clears it when the later non-keep removal is applied.

        return pb2.req_status(status=0, error_message=host_add_warning)

tests/ha/keep_connection.sh:325

• This cleanup path does not verify the behavior that can regress here: host3 has no active connection, and the only check is that the OMAP marker disappears. A peer retaining the stale indication could still process this non-keep delete as keep_connections=True while this test passes. Wait for propagation and assert both gateway logs report keep connections: False (or exercise an active connection and verify it is closed).

cephnvmf_func1 host del --subsystem ${NQN} --host-nqn ${NQN}host3
set +e
    make -s exec SVC=ceph OPTS=-T CMD="rados --pool rbd listomapvals nvmeof.state" | grep "connected-del-host_${NQN}_${NQN}host3"

• Files reviewed: 6/6 changed files
• Comments generated: 1
• Review effort level: Lite