Skip to content

Simplify/Speed Up E2E Tests#993

Merged
ChristianAtDell merged 26 commits intomainfrom
priv/ccoff/remove-deprec-tests
May 28, 2025
Merged

Simplify/Speed Up E2E Tests#993
ChristianAtDell merged 26 commits intomainfrom
priv/ccoff/remove-deprec-tests

Conversation

@ChristianAtDell
Copy link
Copy Markdown
Contributor

@ChristianAtDell ChristianAtDell commented May 22, 2025

Description

This PR encompasses a large effort to reduce the length and possibility for error when performing Operator End to End testing.

  • Authorization V1 tests have been removed entirely, as the module is now deprecated.
  • All cert-csi vio tests have been replaced with cert-csi provisioning equivalents. Full volume IO validation is included in driver testing suites, Operator should not perform these tests. This is a time-saving of about 65-75% on all test cases that provision.
  • Template files in (almost) all cases are now edited in-memory, instead of altering the template file. This solves a problem where templates were left edited instead of reverted in the event of a test failure.
  • Automation has been added with an optional --install-vault flag that will deploy a Vault instance with secrets based on variables set in array-info.sh (and used elsewhere in the e2e framework). This means that a user no longer has to manually install a Vault instance to perform Authorization tests. This only installs one vault instance currently, so future improvements may install a secondary vault instance for the niche test case (not associated with any driver) that tests multi-vault installations.
  • array-info.sh has been replaced with an array-info.env.sample. E2E script looks for an array-info.env to be created by the user in advance. This removes risks of accidentally committing sensitive array-info.sh file to the repository. array-info.env has been added to .gitignore.

GitHub Issues

List the GitHub issues impacted by this PR:

GitHub Issue #
https://github.com/dell/csm/issues/1888

Checklist:

  • I have performed a self-review of my own code to ensure there are no formatting, vetting, linting, or security issues
  • I have verified that new and existing unit tests pass locally with my changes
  • I have not allowed coverage numbers to degenerate
  • I have maintained at least 90% code coverage
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have maintained backward compatibility
  • I have executed the relevant end-to-end test scenarios

How Has This Been Tested?

E2E has been run on every storage platform. Verified time savings compared to last run prior to these changes:

  • PowerMax: 2h20m->2h1m
  • PowerFlex: 1h13m->32m
  • PowerScale: 2h50m->44m
  • PowerStore: 8m -> 6m
  • Unity: 4m -> 3m

Minimal E2Es are still to-be-tested.

  • PowerScale minimal test completed. Required some changes to add isilon-certs-0 to all test cases.
  • Unity minimal test completed.
  • PowerFlex minimal test does not pass, but this appears to be a defect in Operator re: PowerFlex minimal and is not related to this PR's changes.
  • PowerMax minimal test completed without any additional changes. Required 1h16m.

atye
atye previously approved these changes May 23, 2025
@anandrajak1
Copy link
Copy Markdown
Collaborator

@ChristianAtDell - Can you remove the cert-csi reference and use the K8s e2e or OCP e2e for provisioning.

@ChristianAtDell
Copy link
Copy Markdown
Contributor Author

ChristianAtDell commented May 23, 2025

Can you remove the cert-csi reference and use the K8s e2e or OCP e2e for provisioning.

Not for free. We have no clear and concise way to do this without deep investigation. Finding out how to implement k8s e2e into our test suite, switching away from cert-csi in over sixty test cases, and re-testing everything across five platforms is a massive undertaking. It will need to be driven as a Feature, not slipped into an improvements PR. @anandrajak1

@ChristianAtDell
Copy link
Copy Markdown
Contributor Author

In addition, to my knowledge K8s E2E is a whole suite of tests that takes hours to run. I don't believe there are simple "provisioning" tests that you can run with it. This test suite is not meant to be an exhaustive storage check for the drivers, it is meant to be an 'install the driver and make sure it works'-- Operator E2E cannot be bloated to comprehensively test all drivers/modules.

If we are going to deprecate cert-csi as a testing tool, we are going to have to come up with some custom simple testing methodology to "sanity check" drivers installed by Operator can provision. I adamantly refuse bloating Operator tests to take multiple hours.

@alexemc alexemc force-pushed the priv/ccoff/remove-deprec-tests branch from 6883e19 to 0e710b8 Compare May 23, 2025 21:44
@github-actions
Copy link
Copy Markdown
Contributor

Merging this branch will not change overall coverage

Impacted Packages Coverage Δ 🤖
github.com/dell/csm-operator/tests/e2e 0.00% (ø)
github.com/dell/csm-operator/tests/e2e/scripts/vault-automation 0.00% (ø)
github.com/dell/csm-operator/tests/e2e/steps 0.00% (ø)

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/dell/csm-operator/tests/e2e/scripts/vault-automation/main.go 0.00% (ø) 0 0 0
github.com/dell/csm-operator/tests/e2e/steps/steps_def.go 0.00% (ø) 0 0 0
github.com/dell/csm-operator/tests/e2e/steps/steps_runner.go 0.00% (ø) 0 0 0

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

Changed unit test files

  • github.com/dell/csm-operator/tests/e2e/e2e_test.go

@EvgenyUglov
Copy link
Copy Markdown
Contributor

@ChristianAtDell Since Auth V1 is completely removed, can we remove karavictl references from operator as well? I don't remember 100% but I think it was used for auth v1 only

@alexemc
Copy link
Copy Markdown
Contributor

alexemc commented May 26, 2025

@ChristianAtDell Since Auth V1 is completely removed, can we remove karavictl references from operator as well? I don't remember 100% but I think it was used for auth v1 only

@EvgenyUglov Thanks for spotting this. It does seem that karavictl is no longer in use. However, I'm not comfortable removing it without retesting all authentication scenarios, so I'd prefer to leave this cleanup for a future round of improvements.

@ChristianAtDell ChristianAtDell changed the title Simplify/Speed Up E2E Tests [NO MERGES UNTIL RELEASE] Simplify/Speed Up E2E Tests May 27, 2025
@ChristianAtDell ChristianAtDell changed the title [NO MERGES UNTIL RELEASE] Simplify/Speed Up E2E Tests Simplify/Speed Up E2E Tests May 28, 2025
@ChristianAtDell ChristianAtDell merged commit 9a932dc into main May 28, 2025
6 checks passed
@ChristianAtDell ChristianAtDell deleted the priv/ccoff/remove-deprec-tests branch May 28, 2025 20:08
@meggm meggm restored the priv/ccoff/remove-deprec-tests branch May 29, 2025 09:28
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.

7 participants