test(robot): add test strict local volume disabled revision conuter by default

e2e/keywords/common.resource

@@ -51,4 +51,6 @@ Cleanup test resources
     cleanup_disks
     cleanup_backing_images
     cleanup_engine_images
-    reset_backupstore
+    reset_backupstore
+    reset_backupstore
+    reset_settings

e2e/keywords/engine.resource

@@ -15,3 +15,7 @@ Engine state should eventually be ${expected_engine_state}
 Engine state should be ${expected_engine_state}
     ${engine_current_state} =    get_engine_state    ${volume_name}    ${volume_attached_node}
     check_workload_state    ${engine_current_state}    ${expected_engine_state}
+
+Volume ${volume_id} engine ${setting_name} should be ${setting_value}
+    ${volume_name} =    generate_name_with_suffix    volume    ${volume_id}
+    validate_engine_setting    ${volume_name}    ${setting_name}    ${setting_value}

e2e/keywords/replica.resource

@@ -0,0 +1,10 @@
+*** Settings ***
+Documentation       Longhorn replica related keywords
+
+Library             ../libs/keywords/common_keywords.py
+Library             ../libs/keywords/replica_keywords.py
+
+*** Keywords ***
+Volume ${volume_id} replica ${setting_name} should be ${setting_value}
+    ${volume_name} =    generate_name_with_suffix    volume    ${volume_id}
+    validate_replica_setting    ${volume_name}    ${setting_name}    ${setting_value}

e2e/keywords/volume.resource

@@ -273,6 +273,7 @@ Upgrade volume ${volume_id} engine to compatible engine image
     ${volume_name} =    generate_name_with_suffix    volume    ${volume_id}
     upgrade_engine_image    ${volume_name}    ${compatible_engine_image_name}
     wait_for_engine_image_upgrade_completed    ${volume_name}    ${compatible_engine_image_name}
+
 Check volume ${volume_id} data is backup ${backup_id} created in another cluster
     ${volume_name} =    generate_name_with_suffix    volume    ${volume_id}
     ${current_checksum} =    get_volume_checksum    ${volume_name}
@@ -288,3 +289,7 @@ Wait for volume ${volume_id} restoration from backup ${backup_id} in another clu
     ${volume_name} =    generate_name_with_suffix    volume    ${volume_id}
     ${backup_name} =    get_backup_name_from_backup_list    ${backups_before_uninstall}    ${backup_id}
     wait_for_volume_restoration_completed    ${volume_name}    ${backup_name}
+
+Volume ${volume_id} setting ${setting_name} should be ${setting_value}
+    ${volume_name} =    generate_name_with_suffix    volume    ${volume_id}
+    validate_volume_setting    ${volume_name}    ${setting_name}    ${setting_value}

e2e/libs/engine/crd.py

@@ -9,7 +9,7 @@ class CRD(Base):
     def __init__(self):
         self.obj_api = client.CustomObjectsApi()
 
-    def get_engines(self, volume_name, node_name):
+    def get_engines(self, volume_name, node_name=None):
         if volume_name == "" or node_name == "":
             logging.info("getting all engines")
         else:
@@ -56,3 +56,9 @@ def delete_engine(self, volume_name, node_name):
                 name=engine_name
             )
         logging.info("finished delete engines")
+
+    def validate_engine_setting(self, volume_name, setting_name, value):
+        engines = self.get_engines(volume_name)
+        for engine in engines:
+            assert str(engine["spec"][setting_name]) == value, \
+            f"Expected volume {volume_name} engine setting {setting_name} is {value}, but it's {str(engine['spec'][setting_name])}"

e2e/libs/engine/engine.py

@@ -41,3 +41,6 @@ def get_engine_state(self, volume_name, node_name):
             engine_state = engine['status']['currentState']
             engines_states[engine_name] = engine_state
         return engines_states
+
+    def validate_engine_setting(self, volume_name, setting_name, value):
+        return self.engine.validate_engine_setting(volume_name, setting_name, value)

e2e/libs/keywords/engine_keywords.py

@@ -8,3 +8,6 @@ def __init__(self):
 
     def get_engine_instance_manager_name(self, volume_name):
         return self.engine.get_engine_instance_manager_name(volume_name)
+
+    def validate_engine_setting(self, volume_name, setting_name, value):
+        return self.engine.validate_engine_setting(volume_name, setting_name, value)

e2e/libs/keywords/replica_keywords.py

@@ -0,0 +1,9 @@
+from replica import Replica
+
+class replica_keywords:
+
+    def __init__(self):
+        self.replica = Replica()
+
+    def validate_replica_setting(self, volume_name, setting_name, value):
+        return self.replica.validate_replica_setting(volume_name, setting_name, value)

e2e/libs/keywords/setting_keywords.py

@@ -13,3 +13,6 @@ def set_backupstore(self):
 
     def reset_backupstore(self):
         self.setting.reset_backupstore()
+
+    def reset_settings(self):
+        self.setting.reset_settings()

e2e/libs/keywords/volume_keywords.py

@@ -29,9 +29,9 @@ def cleanup_volumes(self):
         for volume in volumes['items']:
             self.delete_volume(volume['metadata']['name'])
 
-    def create_volume(self, volume_name, size="2Gi", numberOfReplicas=3, frontend="blockdev", migratable=False, accessMode="RWO", dataEngine="v1", backingImage="", Standby=False, fromBackup=""):
+    def create_volume(self, volume_name, size="2Gi", numberOfReplicas=3, frontend="blockdev", migratable=False, dataLocality="disabled", accessMode="RWO", dataEngine="v1", backingImage="", Standby=False, fromBackup=""):
         logging(f'Creating volume {volume_name}')
-        self.volume.create(volume_name, size, numberOfReplicas, frontend, migratable, accessMode, dataEngine, backingImage, Standby, fromBackup)
+        self.volume.create(volume_name, size, numberOfReplicas, frontend, migratable, dataLocality, accessMode, dataEngine, backingImage, Standby, fromBackup)
 
     def delete_volume(self, volume_name):
         logging(f'Deleting volume {volume_name}')
@@ -283,9 +283,7 @@ def create_persistentvolumeclaim_for_volume(self, volume_name, retry=True):
 
     def record_volume_replica_names(self, volume_name):
         replica_list = self.replica.get(volume_name, node_name="")
-        assert replica_list != "", f"failed to get replicas"
-
-        replica_names = [replica['metadata']['name'] for replica in replica_list['items']]
+        replica_names = [replica['metadata']['name'] for replica in replica_list]
         logging(f"Found volume {volume_name} replicas: {replica_names}")
 
         replica_names_str = ",".join(replica_names)
@@ -296,9 +294,7 @@ def check_volume_replica_names_recorded(self, volume_name):
         expected_replica_names = sorted(replica_names_str.split(","))
 
         replica_list = self.replica.get(volume_name, node_name="")
-        assert replica_list != "", f"failed to get replicas"
-
-        actual_replica_names = [replica['metadata']['name'] for replica in replica_list['items']]
+        actual_replica_names = [replica['metadata']['name'] for replica in replica_list]
         actual_replica_names = sorted(actual_replica_names)
 
         assert actual_replica_names == expected_replica_names, \
@@ -314,3 +310,6 @@ def wait_for_engine_image_upgrade_completed(self, volume_name, engine_image_name
 
     def get_volume_checksum(self, volume_name):
         return self.volume.get_checksum(volume_name)
+
+    def validate_volume_setting(self, volume_name, setting_name, value):
+        return self.volume.validate_volume_setting(volume_name, setting_name, value)

e2e/libs/replica/crd.py

@@ -10,11 +10,11 @@ class CRD(Base):
     def __init__(self):
         self.obj_api = client.CustomObjectsApi()
 
-    def get(self, volume_name, node_name):
+    def get(self, volume_name, node_name=None):
         label_selector = []
         if volume_name != "":
             label_selector.append(f"longhornvolume={volume_name}")
-        if node_name != "":
+        if node_name:
             label_selector.append(f"longhornnode={node_name}")
 
         replicas = self.obj_api.list_namespaced_custom_object(
@@ -24,7 +24,7 @@ def get(self, volume_name, node_name):
             plural="replicas",
             label_selector=",".join(label_selector)
         )
-        return replicas
+        return replicas["items"]
 
     def delete(self, volume_name, node_name):
         if volume_name == "" or node_name == "":
@@ -33,10 +33,7 @@ def delete(self, volume_name, node_name):
             logging(
                 f"Deleting volume {volume_name} on node {node_name} replicas")
 
-        resp = self.get(volume_name, node_name)
-        assert resp != "", f"failed to get replicas"
-
-        replicas = resp['items']
+        replicas = self.get(volume_name, node_name)
         if len(replicas) == 0:
             return
 
@@ -56,3 +53,9 @@ def wait_for_rebuilding_start(self, volume_name, node_name):
 
     def wait_for_rebuilding_complete(self, volume_name, node_name):
         Rest().wait_for_rebuilding_complete(volume_name, node_name)
+
+    def validate_replica_setting(self, volume_name, setting_name, value):
+        replicas = self.get(volume_name)
+        for replica in replicas:
+            assert str(replica["spec"][setting_name]) == value, \
+            f"Expected volume {volume_name} replica setting {setting_name} is {value}, but it's {str(replica['spec'][setting_name])}"

e2e/libs/replica/replica.py

@@ -24,3 +24,6 @@ def wait_for_rebuilding_start(self, volume_name, node_name):
 
     def wait_for_rebuilding_complete(self, volume_name, node_name):
         return self.replica.wait_for_rebuilding_complete(volume_name,node_name)
+
+    def validate_replica_setting(self, volume_name, setting_name, value):
+        return self.replica.validate_replica_setting(volume_name, setting_name, value)

e2e/libs/setting/setting.py

@@ -60,3 +60,48 @@ def get_backup_target(self):
 
     def get_secret(self):
         return self.get_setting(self.SETTING_BACKUP_TARGET_CREDENTIAL_SECRET)
+
+    def reset_settings(self):
+        client = get_longhorn_client()
+        for setting in client.list_setting():
+            setting_name = setting.name
+            setting_default_value = setting.definition.default
+            setting_readonly = setting.definition.readOnly
+
+            # We don't provide the setup for the storage network, hence there is no
+            # default value. We need to skip here to avoid test failure when
+            # resetting this to an empty default value.
+            if setting_name == "storage-network":
+                continue
+            # The test CI deploys Longhorn with the setting value longhorn-critical
+            # for the setting priority-class. Don't reset it to empty (which is
+            # the default value defined in longhorn-manager code) because this will
+            # restart Longhorn managed components and fail the test cases.
+            # https://github.com/longhorn/longhorn/issues/7413#issuecomment-1881707958
+            if setting.name == "priority-class":
+                continue
+
+            # The version of the support bundle kit will be specified by a command
+            # option when starting the manager. And setting requires a value.
+            #
+            # Longhorn has a default version for each release provided to the
+            # manager when starting. Meaning this setting doesn't have a default
+            # value.
+            #
+            # The design grants the ability to update later by cases for
+            # troubleshooting purposes. Meaning this setting is editable.
+            #
+            # So we need to skip here to avoid test failure when resetting this to
+            # an empty default value.
+            if setting_name == "support-bundle-manager-image":
+                continue
+
+            if setting_name == "registry-secret":
+                continue
+
+            s = client.by_id_setting(setting_name)
+            if s.value != setting_default_value and not setting_readonly:
+                try:
+                    client.update(s, value=setting_default_value)
+                except Exception as e:
+                    logging(f"Failed to reset {setting_name} to {setting_default_value}: {e}")

e2e/libs/volume/crd.py

@@ -27,7 +27,7 @@ def __init__(self):
         self.retry_count, self.retry_interval = get_retry_count_and_interval()
         self.engine = Engine()
 
-    def create(self, volume_name, size, numberOfReplicas, frontend, migratable, accessMode, dataEngine, backingImage, Standby, fromBackup):
+    def create(self, volume_name, size, numberOfReplicas, frontend, migratable, dataLocality, accessMode, dataEngine, backingImage, Standby, fromBackup):
         size_suffix = size[-2:]
         size_number = size[:-2]
         size_unit = MEBIBYTE if size_suffix == "Mi" else GIBIBYTE
@@ -48,6 +48,7 @@ def create(self, volume_name, size, numberOfReplicas, frontend, migratable, acce
                 "size": size,
                 "numberOfReplicas": int(numberOfReplicas),
                 "migratable": migratable,
+                "dataLocality": dataLocality,
                 "accessMode": accessMode,
                 "dataEngine": dataEngine,
                 "backingImage": backingImage,
@@ -76,6 +77,7 @@ def create(self, volume_name, size, numberOfReplicas, frontend, migratable, acce
             assert volume['spec']['numberOfReplicas'] == int(numberOfReplicas), f"expect volume numberOfReplicas is {numberOfReplicas}, but it's {volume['spec']['numberOfReplicas']}"
             assert volume['spec']['frontend'] == frontend, f"expect volume frontend is {frontend}, but it's {volume['spec']['frontend']}"
             assert volume['spec']['migratable'] == migratable, f"expect volume migratable is {migratable}, but it's {volume['spec']['migratable']}"
+            assert volume['spec']['dataLocality'] == dataLocality, f"expect volume dataLocality is {dataLocality}, but it's {volume['spec']['dataLocality']}"
             assert volume['spec']['accessMode'] == accessMode, f"expect volume accessMode is {accessMode}, but it's {volume['spec']['accessMode']}"
             assert volume['spec']['backingImage'] == backingImage, f"expect volume backingImage is {backingImage}, but it's {volume['spec']['backingImage']}"
             assert volume['spec']['Standby'] == Standby, f"expect volume Standby is {Standby}, but it's {volume['spec']['Standby']}"
@@ -504,3 +506,8 @@ def upgrade_engine_image(self, volume_name, engine_image_name):
 
     def wait_for_engine_image_upgrade_completed(self, volume_name, engine_image_name):
         return Rest().wait_for_engine_image_upgrade_completed(volume_name, engine_image_name)
+
+    def validate_volume_setting(self, volume_name, setting_name, value):
+        volume = self.get(volume_name)
+        assert str(volume["spec"][setting_name]) == value, \
+            f"Expected volume {volume_name} setting {setting_name} is {value}, but it's {str(volume['spec'][setting_name])}"

e2e/libs/volume/rest.py

@@ -48,7 +48,7 @@ def list(self):
             time.sleep(self.retry_interval)
         return vol_list
 
-    def create(self, volume_name, size, numberOfReplicas, frontend, migratable, accessMode, dataEngine, backingImage, Standby, fromBackup):
+    def create(self, volume_name, size, numberOfReplicas, frontend, migratable, dataLocality, accessMode, dataEngine, backingImage, Standby, fromBackup):
         return NotImplemented
 
     def attach(self, volume_name, node_name, disable_frontend):

e2e/libs/volume/volume.py

@@ -15,8 +15,8 @@ def __init__(self):
         else:
             self.volume = Rest()
 
-    def create(self, volume_name, size, numberOfReplicas, frontend, migratable, accessMode, dataEngine, backingImage, Standby, fromBackup):
-        return self.volume.create(volume_name, size, numberOfReplicas, frontend, migratable, accessMode, dataEngine, backingImage, Standby, fromBackup)
+    def create(self, volume_name, size, numberOfReplicas, frontend, migratable, dataLocality, accessMode, dataEngine, backingImage, Standby, fromBackup):
+        return self.volume.create(volume_name, size, numberOfReplicas, frontend, migratable, dataLocality, accessMode, dataEngine, backingImage, Standby, fromBackup)
 
     def delete(self, volume_name):
         return self.volume.delete(volume_name)
@@ -151,3 +151,6 @@ def upgrade_engine_image(self, volume_name, engine_image_name):
 
     def wait_for_engine_image_upgrade_completed(self, volume_name, engine_image_name):
         return self.volume.wait_for_engine_image_upgrade_completed(volume_name, engine_image_name)
+
+    def validate_volume_setting(self, volume_name, setting_name, value):
+        return self.volume.validate_volume_setting(volume_name, setting_name, value)

e2e/tests/regression/test_basic.robot

@@ -4,11 +4,15 @@ Documentation    Basic Test Cases
 Test Tags    regression
 
 Resource    ../keywords/common.resource
+Resource    ../keywords/node.resource
+Resource    ../keywords/setting.resource
 Resource    ../keywords/deployment.resource
 Resource    ../keywords/persistentvolumeclaim.resource
 Resource    ../keywords/recurringjob.resource
 Resource    ../keywords/statefulset.resource
 Resource    ../keywords/volume.resource
+Resource    ../keywords/engine.resource
+Resource    ../keywords/replica.resource
 Resource    ../keywords/snapshot.resource
 Resource    ../keywords/node.resource
 
@@ -97,6 +101,19 @@ Test Snapshot
 
     And Check volume 0 data is data 1
 
+Test Strict Local Volume Disabled Revision Counter By Default
+    [Tags]    coretest
+    [Documentation]
+    ...    1. Set the global setting disable-revision-counter to false
+    ...    2. Create a volume with 1 replica and strict-local data locality
+    ...    3. See that the revisionCounterDisabled: true for volume/engine/replica CRs
+    Given Set setting disable-revision-counter to false
+    When Create volume 0 with    numberOfReplicas=1    dataLocality=strict-local
+    And Wait for volume 0 detached
+    Then Volume 0 setting revisionCounterDisabled should be True
+    And Volume 0 engine revisionCounterDisabled should be True
+    And Volume 0 replica revisionCounterDisabled should be True
+
 Replica Rebuilding
     [Documentation]    -- Manual test plan --
     ...                1. Create and attach a volume.