Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(robot): add storageclass for different replica count #1871

Merged
merged 2 commits into from
Apr 29, 2024

Conversation

chriscchien
Copy link
Contributor

Which issue(s) this PR fixes:

Issue longhorn/longhorn#8440

What this PR does / why we need it:

Create 1 replica storageclass longhorn-test-1-replica and 2 replicas storageclass longhorn-test-2-replicas at test step for PVC use

Special notes for your reviewer:

Local test code

*** Test Cases ***
Replica Test
    Given Create persistentvolumeclaim 0 using RWO volume
    And Create persistentvolumeclaim 1 using RWO volume with 1-replica storageclass
    And Create persistentvolumeclaim 2 using RWO volume with 2-replicas storageclass

    And Create deployment 0 with persistentvolumeclaim 0
    And Create deployment 1 with persistentvolumeclaim 1
    And Create deployment 2 with persistentvolumeclaim 2

Additional documentation or context

report.zip

@chriscchien chriscchien self-assigned this Apr 26, 2024
@chriscchien chriscchien requested a review from a team as a code owner April 26, 2024 08:27
@@ -38,10 +38,14 @@ def __init__(self):

def init_storageclasses(self):
create_storageclass('longhorn-test')
create_storageclass('longhorn-test-1-replica')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eventually we will pass arbitrary parameters to customize storage class to meet test cases requirements, like fromBackup, migratable, dataLocality, backingImageName and so on (you can reference https://longhorn.io/docs/1.6.1/references/storage-class-parameters/ to see all parameters). Is it possible to modify the mechanism to dynamically create custom storage class on demand (before you create the PVC) and destroy it when the test case completed?

@@ -19,6 +19,12 @@ def create_storageclass(name):

with open(filepath, 'r') as f:
manifest_dict = yaml.safe_load(f)
manifest_dict['metadata']['name'] = name
# correct replica count
if name == 'longhorn-test-1-replica':
Copy link
Member

@yangchiu yangchiu Apr 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to adopt this mechanism https://github.com/longhorn/longhorn-tests/pull/1867/files#diff-6422ed87ee022691c861ebd060f6651dd79d25bef034662a78dc201f40a8838bR14 to make storage class accept arbitrary option settings instead of hard-coding it?

So the corresponding test step would be:

And Create persistentvolumeclaim 1 using RWO volume with    replica_count=1

@chriscchien chriscchien force-pushed the dynamic_workload_volume_replica branch from 8facf8d to e689f74 Compare April 29, 2024 10:10
@chriscchien
Copy link
Contributor Author

Updated:

  • Add keywords for create storageclass with name and params for PVC use, below test code will create storageclass longhorn-test-test-storageclass (longhorn-test and longhorn-test-strict-local storageclass still auto created at test stepup steps)

  • Test code

*** Test Cases ***
Storageclass Test
    Given Create storageclass test-storageclass with    replica_count=2    migratable=true
    
    And Create persistentvolumeclaim 0 using RWO volume
    And Create persistentvolumeclaim 1 using RWX volume
    And Create persistentvolumeclaim 2 using RWO volume with strict-local storageclass
    And Create persistentvolumeclaim 3 using RWO volume with test-storageclass storageclass

    And Create deployment 0 with persistentvolumeclaim 0
    And Create deployment 1 with persistentvolumeclaim 1
    And Create deployment 2 with persistentvolumeclaim 2
    And Create deployment 3 with persistentvolumeclaim 3
  • storageclass longhorn-test-test-storageclass
> k get storageclass longhorn-test-test-storageclass -o yaml | grep param -A4
parameters:
  migratable: "true"
  numberOfReplicas: "2"
  staleReplicaTimeout: "30"
provisioner: driver.longhorn.io

report.zip

@chriscchien chriscchien force-pushed the dynamic_workload_volume_replica branch from e689f74 to 650b584 Compare April 29, 2024 10:22
@chriscchien chriscchien requested a review from yangchiu April 29, 2024 10:27
Copy link
Member

@yangchiu yangchiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yangchiu yangchiu merged commit 89be495 into longhorn:master Apr 29, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants