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

Zoning E2E Test + E2E Improvements #818

Merged
merged 17 commits into from
Dec 10, 2024
Merged

Conversation

ChristianAtDell
Copy link
Contributor

@ChristianAtDell ChristianAtDell commented Dec 9, 2024

Description

The primary goal of this PR is to add an E2E test scenario (and robust testing script) that covers the new zoning work. In addition to that, some improvements were added to the E2E test suite.

  1. All custom tests in E2E scenarios are now part of an 'array' of custom tests for their scenario, and can be selected individually. This allows for multiple custom tests to be run per scenario, which was necessary for adding and removing zone labels to k8s nodes. Future scenarios added will need to have their custom tests indented inwards to be part of an array, but legacy support for the previous 'run custom test' E2E step was left intact.
  2. A new way of deploying secrets as a part of E2E tests was introduced, that creates the secret by reading in the template file, editing the string in-memory, and deploying from that string. This was necessary because the previous way templates were used, editing in place, created an impossible scenario to restore from in our testing: if any two template keys had the same value, it was impossible to reverse the string replacement that was done.

GitHub Issues

List the GitHub issues impacted by this PR:

GitHub Issue #
dell/csm#1613

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 80% 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 scenario has been run standalone. It passes, though that does not actually test zoning yet, as zoning work is not completed.

Comment on lines +790 to +791
fileArg := "--from-literal=config=" + fileString
err = execCommand("kubectl", "create", "secret", "generic", "-n", namespace, name, fileArg)
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tested this with special shell characters in fileString, e.g. dollar signs and exclamation marks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Definitely exclamation marks, because one of our test secrets includes one in a field. Otherwise, no comprehensive testing was done on it, but I'm not too worried about it-- this is just running e2e tests using controlled templates and provided inputs.

donatwork
donatwork previously approved these changes Dec 10, 2024
anathoodell
anathoodell previously approved these changes Dec 10, 2024
@ChristianAtDell ChristianAtDell changed the title Zoning E2E Test + E2E Improvements (Do Not Merge) Zoning E2E Test + E2E Improvements Dec 10, 2024
@ChristianAtDell ChristianAtDell marked this pull request as ready for review December 10, 2024 17:27
tests/e2e/testfiles/scenarios.yaml Show resolved Hide resolved
tests/e2e/modify_zoning_labels.sh Show resolved Hide resolved
@ChristianAtDell ChristianAtDell merged commit b734947 into main Dec 10, 2024
7 checks passed
@ChristianAtDell ChristianAtDell deleted the pub/zoning-e2e-tests branch December 10, 2024 21:05
ChristianAtDell added a commit that referenced this pull request Dec 10, 2024
@ChristianAtDell
Copy link
Contributor Author

Ignore the commit to revert, I went to click 'delete branch' and hit 'revert' instead, which opened a new branch with a revert commit. I've deleted the branch.

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