Merge remote-tracking branch 'couchbase/trinity' into phoenix

* MB-68048: Fix no-op evictionPolicy updates during storage mode
            migration

Change-Id: Iefb9ab0362a425a172911da0ad7a264859f0a51a

Author: neelima32

apps/ns_server/src/ns_bucket.erl

@@ -2073,7 +2073,30 @@ maybe_update_eviction_policy_overrides(NewProps, BucketConfig,
                     {NewProps, ExistingDeleteKeys}
             end;
         false ->
-            case proplists:get_value(eviction_policy, NewProps) =/= undefined of
+            %% Why not use eviction_policy_changed/2 here?
+            %% Mixed eviction policy changes:
+            %% When a user mixes eviction policy changes that require a restart
+            %% with those that don’t, we delete eviction policy overrides (if
+            %% --noRestart is omitted) and reset to a clean slate. The new
+            %% eviction policy takes effect immediately.
+            %%
+            %% Exception during storage mode migration:
+            %% During a storage mode migration, the only eviction policy changes
+            %% allowed are those with --noRestart.
+            %% The REST API ignores attempts to modify bucket-level properties
+            %% that can’t be changed during migration if the value stays the
+            %% same. If the value changes, it throws an error indicating storage
+            %% migration is in progress.
+            %%
+            %% Behavior we retain:
+            %% If the user sets the eviction policy to the same value it already
+            %% has, we don’t delete overrides or trigger a restart. Any existing
+            %% overrides remain until a swap rebalance or graceful failover and
+            %% full recovery.
+            PolicySpecified =
+                proplists:get_value(eviction_policy, NewProps) =/= undefined,
+            case PolicySpecified andalso not
+                    storage_mode_migration_in_progress(BucketConfig) of
                 true ->
                     %% By default, eviction policy changes force a bucket
                     %% restart. Delete any existing eviction policy overrides.

cluster_tests/testsets/bucket_migration_test.py

@@ -1049,3 +1049,87 @@ def check_eviction_policy_changed():
             )
 
         self.cluster.delete_bucket(bucket_name)
+
+    def eviction_policy_noop_no_restart_storage_mode_migration_test(
+        self,
+    ):
+        """
+        Start a storage mode migration with an eviction policy change using
+        --noRestart.
+        Then send an evictionPolicy update without --noRestart but with the
+        identical bucket-level eviction policy value, and verify it's treated as
+        a no-op:
+        - No per-node eviction overrides disappear
+        - No per-node storage mode overrides disappear
+        """
+        bucket_name = "bucket-evict-noop-test"
+
+        # Create couchstore + valueOnly
+        create_bucket(
+            self.cluster, bucket_name, "couchstore", 1024, "valueOnly"
+        )
+
+        # Begin migration to magma and change eviction policy with --noRestart
+        # This should add per-node storage_mode=couchstore and
+        # eviction_policy=valueOnly overrides
+        update_data = {
+            "name": bucket_name,
+            "storageBackend": "magma",
+            "evictionPolicy": "fullEviction",
+            "noRestart": "true",
+        }
+        self.cluster.update_bucket(update_data)
+
+        # Capture overrides before the no-op request
+        per_node_storage_mode_before = get_per_node_storage_mode(
+            self.cluster, bucket_name
+        )
+        per_node_eviction_policy_before = get_per_node_eviction_policy(
+            self.cluster, bucket_name
+        )
+
+        # Sanity: overrides were created as expected
+        assert_per_node_storage_mode_keys_added(
+            self.cluster, bucket_name, "couchstore"
+        )
+        assert_per_node_eviction_policy_keys_added(
+            self.cluster, bucket_name, "valueOnly"
+        )
+
+        # Now send evictionPolicy again without noRestart but with identical
+        # value ("fullEviction"). This should be a no-op and must not delete the
+        # per-node eviction overrides
+        noop_update = {
+            "name": bucket_name,
+            "evictionPolicy": "fullEviction",
+            # noRestart intentionally omitted
+        }
+        self.cluster.update_bucket(noop_update)
+
+        # Verify overrides did not disappear
+        per_node_storage_mode_after = get_per_node_storage_mode(
+            self.cluster, bucket_name
+        )
+        per_node_eviction_policy_after = get_per_node_eviction_policy(
+            self.cluster, bucket_name
+        )
+
+        assert (
+            per_node_storage_mode_after == per_node_storage_mode_before
+        ), f"per-node storage mode overrides changed on no-op: "
+        f"{per_node_storage_mode_before} -> {per_node_storage_mode_after}"
+
+        assert (
+            per_node_eviction_policy_after == per_node_eviction_policy_before
+        ), f"per-node eviction overrides changed on no-op: "
+        f"{per_node_eviction_policy_before} -> {per_node_eviction_policy_after}"
+
+        # And that they are still the expected values
+        assert_per_node_storage_mode_keys_added(
+            self.cluster, bucket_name, "couchstore"
+        )
+        assert_per_node_eviction_policy_keys_added(
+            self.cluster, bucket_name, "valueOnly"
+        )
+
+        self.cluster.delete_bucket(bucket_name)