test(e2e): extract and extend storage tests into dedicated test suite#1321
test(e2e): extract and extend storage tests into dedicated test suite#1321
Conversation
| // Cached zone support data - maps region name to zone support boolean | ||
| // Cached zone support data - maps region name to zone support boolean. | ||
| // Kept separate from zoneList because fallbackZonalRegions only has boolean data, | ||
| // allowing SupportsZones() to work even when the API fails and zoneList is empty. |
There was a problem hiding this comment.
Is there a possibility that this happen? The code below make it looks like both will come from the same API, though not sure if there are more behind the scenes.
There was a problem hiding this comment.
I think the point is that we only hardcode the boolean collection (via fallbackZonalRegions) rather than the full list of zones which is what is being added here.
It is a bit weird though, I wonder if it would be better if we just hardcoded the full zonal list rather than a boolean collection and then we could avoid having a collection of booleans + a collection of list of zones. We could possibly update the fallback zonal regions to be read from a file we code-generate via calling the az cli + script (similar to other things) and then use embed or similar to embed that data into the application?
There was a problem hiding this comment.
This would also mean that GetAvailableZones had the same fallback behavior as SupportsZones, which would be nice.
| // We expect the stateful workload to become healthy on new node before the 6-minute force detach timeout. | ||
| // We start timer after pod binds to node because volume attachment happens during ContainerCreating | ||
| env.EventuallyExpectCreatedNodeClaimCount("==", 1) | ||
| env.EventuallyExpectCreatedNodeCount(">=", 1) |
There was a problem hiding this comment.
Do we need to check for create node >= 1 this occasionally?
If we don't reset, or check whether there is no new node created, just once isenough?
| // Cached zone support data - maps region name to zone support boolean | ||
| // Cached zone support data - maps region name to zone support boolean. | ||
| // Kept separate from zoneList because fallbackZonalRegions only has boolean data, | ||
| // allowing SupportsZones() to work even when the API fails and zoneList is empty. |
There was a problem hiding this comment.
I think the point is that we only hardcode the boolean collection (via fallbackZonalRegions) rather than the full list of zones which is what is being added here.
It is a bit weird though, I wonder if it would be better if we just hardcoded the full zonal list rather than a boolean collection and then we could avoid having a collection of booleans + a collection of list of zones. We could possibly update the fallback zonal regions to be read from a file we code-generate via calling the az cli + script (similar to other things) and then use embed or similar to embed that data into the application?
| // Cached zone support data - maps region name to zone support boolean | ||
| // Cached zone support data - maps region name to zone support boolean. | ||
| // Kept separate from zoneList because fallbackZonalRegions only has boolean data, | ||
| // allowing SupportsZones() to work even when the API fails and zoneList is empty. |
There was a problem hiding this comment.
This would also mean that GetAvailableZones had the same fallback behavior as SupportsZones, which would be nice.
|
|
||
| // ensureLoaded attempts to load zone data from Azure API if not already loaded. | ||
| // Must be called with p.mu held. | ||
| func (p *Provider) ensureLoaded(ctx context.Context) { |
There was a problem hiding this comment.
minor: You could call this ensureLoadedLocked (which is a slightly funky name) to make it more clear the expectation that this must be called w/ the lock held
4 participants
Description
Extracts storage-related E2E tests from the Integration test suite into a dedicated Storage test suite, and extends it with additional test cases for better coverage.
Changes:
test/suites/storage/suite_test.gocontaining all storage teststest/suites/integration/storage_test.go.github/workflows/e2e-matrix.yamlGetAvailableZones()andSupportsZones()methods to the test environmentGetAvailableZones()method to return the list of available zones for a regionThe new Storage test suite includes:
How was this change tested?
TEST_SUITE=Storage make az-e2etests(all 11 tests passed in ~30 minutes)Does this change impact docs?