From c2871b2b5aee04d572d04a7350d50fec7210f4fa Mon Sep 17 00:00:00 2001 From: Yang Chiu Date: Mon, 2 Sep 2024 15:16:08 +0800 Subject: [PATCH] test(robot): add test strict local volume disabled revision conuter by default Signed-off-by: Yang Chiu --- e2e/keywords/common.resource | 1 + e2e/keywords/engine.resource | 4 +++ e2e/keywords/replica.resource | 10 ++++++ e2e/keywords/volume.resource | 4 +++ e2e/libs/engine/crd.py | 12 +++++-- e2e/libs/engine/engine.py | 3 ++ e2e/libs/keywords/engine_keywords.py | 3 ++ e2e/libs/keywords/replica_keywords.py | 9 ++++++ e2e/libs/keywords/setting_keywords.py | 3 ++ e2e/libs/keywords/volume_keywords.py | 15 +++++---- e2e/libs/replica/crd.py | 17 +++++----- e2e/libs/replica/replica.py | 3 ++ e2e/libs/setting/setting.py | 45 +++++++++++++++++++++++++++ e2e/libs/volume/crd.py | 8 ++++- e2e/libs/volume/rest.py | 2 +- e2e/libs/volume/volume.py | 7 +++-- e2e/tests/regression/test_basic.robot | 17 ++++++++++ 17 files changed, 141 insertions(+), 22 deletions(-) create mode 100644 e2e/keywords/replica.resource create mode 100644 e2e/libs/keywords/replica_keywords.py diff --git a/e2e/keywords/common.resource b/e2e/keywords/common.resource index 642aaae18f..f27864bb65 100644 --- a/e2e/keywords/common.resource +++ b/e2e/keywords/common.resource @@ -51,6 +51,7 @@ Cleanup test resources cleanup_backing_images cleanup_engine_images reset_backupstore + reset_settings Cleanup test resources include off nodes Power on off node diff --git a/e2e/keywords/engine.resource b/e2e/keywords/engine.resource index cfb5c6d8f6..222cfc94c5 100644 --- a/e2e/keywords/engine.resource +++ b/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} diff --git a/e2e/keywords/replica.resource b/e2e/keywords/replica.resource new file mode 100644 index 0000000000..a1df20eec3 --- /dev/null +++ b/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} diff --git a/e2e/keywords/volume.resource b/e2e/keywords/volume.resource index 085188f074..930e133273 100644 --- a/e2e/keywords/volume.resource +++ b/e2e/keywords/volume.resource @@ -260,3 +260,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} + +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} diff --git a/e2e/libs/engine/crd.py b/e2e/libs/engine/crd.py index 0d88e385ba..6ee578bc40 100644 --- a/e2e/libs/engine/crd.py +++ b/e2e/libs/engine/crd.py @@ -9,15 +9,15 @@ class CRD(Base): def __init__(self): self.obj_api = client.CustomObjectsApi() - def get_engines(self, volume_name, node_name): - if volume_name == "" or node_name == "": + def get_engines(self, volume_name, node_name=None): + if not volume_name or not node_name: logging.info("getting all engines") else: logging.info( f"getting the volume {volume_name} on node {node_name} engine") label_selector = [] - if volume_name != "": + if volume_name: label_selector.append(f"longhornvolume={volume_name}") if node_name: label_selector.append(f"longhornnode={node_name}") @@ -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])}" diff --git a/e2e/libs/engine/engine.py b/e2e/libs/engine/engine.py index 18849254f3..d57f06777f 100644 --- a/e2e/libs/engine/engine.py +++ b/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) diff --git a/e2e/libs/keywords/engine_keywords.py b/e2e/libs/keywords/engine_keywords.py index 23b7eb7e75..ebdd2425c5 100644 --- a/e2e/libs/keywords/engine_keywords.py +++ b/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) diff --git a/e2e/libs/keywords/replica_keywords.py b/e2e/libs/keywords/replica_keywords.py new file mode 100644 index 0000000000..86454f4689 --- /dev/null +++ b/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) diff --git a/e2e/libs/keywords/setting_keywords.py b/e2e/libs/keywords/setting_keywords.py index 18f9cc5923..8ee4b551bb 100644 --- a/e2e/libs/keywords/setting_keywords.py +++ b/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() diff --git a/e2e/libs/keywords/volume_keywords.py b/e2e/libs/keywords/volume_keywords.py index 726f693e4d..b10f822f6f 100644 --- a/e2e/libs/keywords/volume_keywords.py +++ b/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, \ @@ -311,3 +307,6 @@ def upgrade_engine_image(self, volume_name, engine_image_name): def wait_for_engine_image_upgrade_completed(self, volume_name, engine_image_name): 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) diff --git a/e2e/libs/replica/crd.py b/e2e/libs/replica/crd.py index 2174ec6801..c238bfa5e9 100644 --- a/e2e/libs/replica/crd.py +++ b/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])}" diff --git a/e2e/libs/replica/replica.py b/e2e/libs/replica/replica.py index 1b28227979..55893c0cbe 100644 --- a/e2e/libs/replica/replica.py +++ b/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) diff --git a/e2e/libs/setting/setting.py b/e2e/libs/setting/setting.py index 14605d7e60..d479b11f56 100644 --- a/e2e/libs/setting/setting.py +++ b/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}") diff --git a/e2e/libs/volume/crd.py b/e2e/libs/volume/crd.py index ec89be8f77..729a14ccaa 100644 --- a/e2e/libs/volume/crd.py +++ b/e2e/libs/volume/crd.py @@ -28,7 +28,7 @@ def __init__(self, node_exec): 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 @@ -49,6 +49,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']}" @@ -495,3 +497,7 @@ 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(self.node_exec).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])}" diff --git a/e2e/libs/volume/rest.py b/e2e/libs/volume/rest.py index 4ec867df15..30114f0b50 100644 --- a/e2e/libs/volume/rest.py +++ b/e2e/libs/volume/rest.py @@ -49,7 +49,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): diff --git a/e2e/libs/volume/volume.py b/e2e/libs/volume/volume.py index 7d8fcec34b..3f28290f41 100644 --- a/e2e/libs/volume/volume.py +++ b/e2e/libs/volume/volume.py @@ -18,8 +18,8 @@ def __init__(self): else: self.volume = Rest(node_exec) - 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) diff --git a/e2e/tests/regression/test_basic.robot b/e2e/tests/regression/test_basic.robot index a381497149..7f143e2cce 100644 --- a/e2e/tests/regression/test_basic.robot +++ b/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 Test Setup Set test environment @@ -96,6 +100,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.