From 5e06d0ad6f90d7dc8adeedfd81dcad7b3bba395e Mon Sep 17 00:00:00 2001
From: Kamil Braun <kbraun@scylladb.com>
Date: Tue, 10 May 2022 18:52:44 +0200
Subject: [PATCH] raft: server: fix bad_variant_access in `modify_config`

`modify_config` would call `execute_modify_config` or
`_rpc->send_modify_config`, which returned a reply of type
`add_entry_reply`. This is a variant of 3 options: `entry_id`,
`not_a_leader`, or `commit_status_unknown`. The code would check
for the `entry_id` option and otherwise assume that it was `not_a_leader`.
During nemesis testing however, the reply was sometimes
`commit_status_unknown`, which caused a `bad_variant_access` exception
during `std::get` call. Fix this.

There is a similar piece of code in `add_entry`, but there it should be
impossible to obtain `commit_status_unknown` even though the types don't
enforce it. Make it more explicit with a comment and an assertion.
---
 raft/server.cc | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/raft/server.cc b/raft/server.cc
index b5e6374235..56c5195ccb 100644
--- a/raft/server.cc
+++ b/raft/server.cc
@@ -502,8 +502,15 @@ future<> server_impl::add_entry(command command, wait_type type, seastar::abort_
             }();
             if (std::holds_alternative<raft::entry_id>(reply)) {
                 co_return co_await wait_for_entry(std::get<raft::entry_id>(reply), type, as);
+            } else if (std::holds_alternative<raft::commit_status_unknown>(reply)) {
+                // It should be impossible to obtain `commit_status_unknown` here
+                // because neither `execute_add_entry` nor `send_add_entry` wait for the entry
+                // to be committed/applied.
+                on_internal_error(logger, "add_entry: `execute_add_entry` or `send_add_entry`"
+                        " returned `commit_status_unknown`");
+            } else {
+                leader = std::get<raft::not_a_leader>(reply).leader;
             }
-            leader = std::get<raft::not_a_leader>(reply).leader;
         }
     }
 }
@@ -596,7 +603,11 @@ future<> server_impl::modify_config(std::vector<server_address> add, std::vector
                 // See also #9981.
                 co_return;
             }
-            leader = std::get<raft::not_a_leader>(reply).leader;
+            if (auto nal = std::get_if<raft::not_a_leader>(&reply)) {
+                leader = nal->leader;
+            } else {
+                throw std::get<raft::commit_status_unknown>(reply);
+            }
         }
     }
 }