Handle case where bucket spaces have differing maintenance state for a node

Only skip deactivating buckets if the entire _node_ is marked as
maintenance state, i.e. the node has maintenance state across all
bucket spaces provided in the bundle. Otherwise treat the state
transition as if the node goes down, deactivating all buckets.

Also ensure that the bucket deactivation logic above the SPI is
identical to that within Proton. This avoids bucket DBs getting
out of sync between the two.

1 files changed, 83 insertions, 26 deletions

diff --git a/storage/src/tests/persistence/filestorage/deactivatebucketstest.cpp b/storage/src/tests/persistence/filestorage/deactivatebucketstest.cpp
index 597bb4b07ff..5978f71db2f 100644
--- a/storage/src/tests/persistence/filestorage/deactivatebucketstest.cpp
+++ b/storage/src/tests/persistence/filestorage/deactivatebucketstest.cpp
@@ -14,11 +14,46 @@ using namespace ::testing;
 namespace storage {
 
 struct DeactivateBucketsTest : FileStorTestFixture {
-    bool isActive(const document::BucketId&) const;
+    std::unique_ptr<TestFileStorComponents> _c;
+
+    [[nodiscard]] bool is_active(const document::BucketId&) const;
+
+    void SetUp() override {
+        FileStorTestFixture::SetUp();
+        _c = std::make_unique<TestFileStorComponents>(*this);
+
+        std::string up_state("storage:2 distributor:2");
+        _node->getStateUpdater().setClusterState(
+                std::make_shared<const lib::ClusterState>(up_state));
+
+        createBucket(test_bucket());
+
+        api::BucketInfo serviceLayerInfo(1, 2, 3, 4, 5, true, true);
+        {
+            StorBucketDatabase::WrappedEntry entry(
+                    _node->getStorageBucketDatabase().get(test_bucket(), "foo",
+                                                          StorBucketDatabase::CREATE_IF_NONEXISTING));
+            entry->info = serviceLayerInfo;
+            entry.write();
+        }
+    }
+
+    void TearDown() override {
+        _c.reset();
+        FileStorTestFixture::TearDown();
+    }
+
+    static document::BucketId test_bucket() noexcept {
+        return {8, 123};
+    }
+
+    static std::shared_ptr<const lib::ClusterState> state_of(const char* str) {
+        return std::make_shared<const lib::ClusterState>(str);
+    }
 };
 
 bool
-DeactivateBucketsTest::isActive(const document::BucketId& bucket) const
+DeactivateBucketsTest::is_active(const document::BucketId& bucket) const
 {
     StorBucketDatabase::WrappedEntry entry(
             _node->getStorageBucketDatabase().get(bucket, "foo"));
@@ -26,31 +61,53 @@ DeactivateBucketsTest::isActive(const document::BucketId& bucket) const
     return entry->info.isActive();
 }
 
-TEST_F(DeactivateBucketsTest, buckets_in_database_deactivated_when_node_down_in_cluster_state) {
-    TestFileStorComponents c(*this);
-    // Must set state to up first, or down-edge case won't trigger.
-    std::string upState("storage:2 distributor:2");
-    _node->getStateUpdater().setClusterState(
-            lib::ClusterState::CSP(new lib::ClusterState(upState)));
-
-    document::BucketId bucket(8, 123);
-
-    createBucket(bucket);
-    api::BucketInfo serviceLayerInfo(1, 2, 3, 4, 5, true, true);
-    {
-        StorBucketDatabase::WrappedEntry entry(
-                _node->getStorageBucketDatabase().get(bucket, "foo",
-                        StorBucketDatabase::CREATE_IF_NONEXISTING));
-        entry->info = serviceLayerInfo;
-        entry.write();
-    }
-    EXPECT_TRUE(isActive(bucket));
-    std::string downState("storage:2 .1.s:d distributor:2");
-    _node->getStateUpdater().setClusterState(
-            lib::ClusterState::CSP(new lib::ClusterState(downState)));
-
+TEST_F(DeactivateBucketsTest, buckets_deactivated_when_node_marked_down)
+{
+    EXPECT_TRUE(is_active(test_bucket()));
+    _node->getStateUpdater().setClusterState(state_of("storage:2 .1.s:d distributor:2"));
     // Buckets should have been deactivated in content layer
-    EXPECT_FALSE(isActive(bucket));
+    EXPECT_FALSE(is_active(test_bucket()));
+}
+
+TEST_F(DeactivateBucketsTest, buckets_not_deactivated_when_node_marked_maintenance)
+{
+    EXPECT_TRUE(is_active(test_bucket()));
+    _node->getStateUpdater().setClusterState(state_of("storage:2 .1.s:m distributor:2"));
+    EXPECT_TRUE(is_active(test_bucket()));
+}
+
+TEST_F(DeactivateBucketsTest, buckets_deactivated_when_node_goes_from_maintenance_to_up)
+{
+    EXPECT_TRUE(is_active(test_bucket()));
+    _node->getStateUpdater().setClusterState(state_of("storage:2 .1.s:m distributor:2"));
+    _node->getStateUpdater().setClusterState(state_of("storage:2 distributor:2"));
+    EXPECT_FALSE(is_active(test_bucket()));
 }
 
+TEST_F(DeactivateBucketsTest, buckets_deactivated_when_node_goes_from_maintenance_to_down)
+{
+    EXPECT_TRUE(is_active(test_bucket()));
+    _node->getStateUpdater().setClusterState(state_of("storage:2 .1.s:m distributor:2"));
+    _node->getStateUpdater().setClusterState(state_of("storage:2 distributor:2"));
+    EXPECT_FALSE(is_active(test_bucket()));
+}
+
+// If we only have a subset of the bucket spaces in maintenance mode (i.e. global
+// bucket merge enforcement), we treat this as the node being down from the perspective
+// of default space bucket deactivation.
+TEST_F(DeactivateBucketsTest, bucket_space_subset_in_maintenance_deactivates_buckets)
+{
+    EXPECT_TRUE(is_active(test_bucket()));
+    auto derived = lib::ClusterStateBundle::BucketSpaceStateMapping({
+        {document::FixedBucketSpaces::default_space(), state_of("storage:2 .1.s:m distributor:2")},
+        {document::FixedBucketSpaces::global_space(),  state_of("storage:2 distributor:2")}
+    });
+    _node->getStateUpdater().setClusterStateBundle(
+            std::make_shared<const lib::ClusterStateBundle>(*state_of("storage:2 .1.s:m distributor:2"),
+                                                            std::move(derived)));
+    EXPECT_FALSE(is_active(test_bucket()));
+}
+
+// TODO should also test SPI interaction
+
 } // namespace storage