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

Extend e2e chainsaw tests #249

Merged
merged 32 commits into from
Mar 4, 2025
Merged

Conversation

jstudler
Copy link
Contributor

@jstudler jstudler commented Feb 28, 2025

Adds new e2e tests for IpAddressClaim and IpRangeClaim and aligns naming of all test resources (folder names, chainsaw test names and prefixes for k8s CRs).

Changes:

  • Remove netboxOperatorRestorationHash from assertions everywhere except in the restoration and parentPrefixSelector tests
  • Naming: All tests now have a .metadata.name compatible name. This name is used in the path of the file in git, in the .metadata.name of the chainsaw Test yaml and as name (prefix) for all CRs involved (e.g. PrefixClaim, IpAddressClaim, IpRangeClaim)
  • Steps are now smaller to be more self explanatory. Descriptions were removed where the step name was sufficient.
  • Added an annotation called description to Chainsaw Test resources to allow for further explanation (What does the test do?)
  • Updated the data loading script to include a description to prefixes (helps for better visibility in NetBox)
  • Tweaked timeouts for chainsaw tests and resource limits for the NetBox deployment (was OOMKilled which had quite the impact on the Pipeline) to make pipeline more reliable
  • Added apply & update test for IPv4+IPv6 and IpAddressClaim+IpRangeClaim that expects updates to description and comments to succeed
  • Added Prefix exhaustion test for IPv4+IPv6 and IpAddressClaim+IpRangeClaim that expects for the Claim CR to fail if the ParentPrefix is exhausted
  • Added Restore test for IPv4+IPv6 and IpAddressClaim+IpRangeClaim that expects the same IP Range or IP Address to be assigned when restoring the resource from NetBox.
  • Added a set of tests that verify that invalid CRs will fail. This is currently limited mostly to IP Range and one test for Prefix. All invalid tests created for IP Range can be scaled to IP Address and Prefix in a later stage.

Closes #145

@jstudler jstudler marked this pull request as ready for review February 28, 2025 15:58
@jstudler jstudler changed the title Extend e2e chainsaw Extend e2e chainsaw tests Feb 28, 2025
…nged back when we have the leaselocker deadlocks/timeouts under control)
…is OOMKilled with current limits of 750m/768Mi)
…created during the chainsaw test (description e2e/...)
Copy link
Contributor

@faebr faebr left a comment

Choose a reason for hiding this comment

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

Very nice these tests, I like that it checks all fields and we also make sure not to accidentally change a status field. Only small comments, I did not repeat them for all tests but obviously they apply to all the tests of the same type. Will you add the update of custom fields still in this mr?

jstudler and others added 5 commits March 3, 2025 16:17
…store/chainsaw-test.yaml

Co-authored-by: Fabian <30692464+faebr@users.noreply.github.com>
…-test.yaml

Co-authored-by: Fabian <30692464+faebr@users.noreply.github.com>
…aw-test.yaml

Co-authored-by: Fabian <30692464+faebr@users.noreply.github.com>
@jstudler
Copy link
Contributor Author

jstudler commented Mar 3, 2025

Very nice these tests, I like that it checks all fields and we also make sure not to accidentally change a status field. Only small comments, I did not repeat them for all tests but obviously they apply to all the tests of the same type. Will you add the update of custom fields still in this mr?

Thanks for the thorough review. I'd keep the todos since they'd require some adjustments to the Controllers and/or some troubleshooting.

@jstudler jstudler requested a review from faebr March 3, 2025 17:34
@faebr
Copy link
Contributor

faebr commented Mar 4, 2025

Looks good to me, just a small question, do you know why this error is here: https://github.com/netbox-community/netbox-operator/actions/runs/13636803517/job/38117487010?pr=249#step:7:1007

@faebr
Copy link
Contributor

faebr commented Mar 4, 2025

Looks good to me, just a small question, do you know why this error is here: https://github.com/netbox-community/netbox-operator/actions/runs/13636803517/job/38117487010?pr=249#step:7:1007

its during cleanup from the cleanup script so all good

@jstudler jstudler merged commit 784347b into main Mar 4, 2025
8 checks passed
@henrybear327 henrybear327 deleted the feature/e2e-chainsaw-tests-for-ip-and-ipr branch March 5, 2025 13:11
@jstudler jstudler self-assigned this Mar 6, 2025
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.

Add tests for IpRange and IpRangeClaim Controllers
2 participants