Skip to content

Comments

AWS system test eks and fargate profile: get subnets as external parameter#29276

Merged
o-nikolas merged 1 commit intoapache:mainfrom
aws-mwaa:vincbeck/system-tests/eks
Feb 1, 2023
Merged

AWS system test eks and fargate profile: get subnets as external parameter#29276
o-nikolas merged 1 commit intoapache:mainfrom
aws-mwaa:vincbeck/system-tests/eks

Conversation

@vincbeck
Copy link
Contributor

In both system tests example_eks_with_fargate_profile and example_eks_with_fargate_in_one_step subnets are created within the system test. In order to create these subnets, an available CIDR block is created with the function _get_next_available_cidr. When both tests are running at the same time, some race condition can happen and both tests might pick the same CIDR blocks which then result as a failure.

To avoid that, I rather create these subnets outside of the system test and fetch them as external parameter

Copy link
Contributor

@vandonr-amz vandonr-amz left a comment

Choose a reason for hiding this comment

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

nice, with a side benefit of making the test simpler to read, and also faster as deleting the nat gateway was taking ~45s :D

@o-nikolas
Copy link
Contributor

Agreed, great change. So beautiful when you can just use the variable fetcher to hide the nasty bits of creating/managing resources 😄

Copy link
Contributor

@ferruzzi ferruzzi left a comment

Choose a reason for hiding this comment

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

I do love a negative linecount.

For posterity: generally we would prefer to set up and tear down in the test, this is only getting changed because of the race condition it was causing.

@o-nikolas
Copy link
Contributor

For posterity: generally we would prefer to set up and tear down in the test, this is only getting changed because of the race condition it was causing.

Good call out 👍

@o-nikolas
Copy link
Contributor

FYI: @vincbeck is just making some internal preparations (to create the subnets that this test now expects to be present). I'll merge these changes tomorrow when that is done.

@o-nikolas o-nikolas merged commit e626131 into apache:main Feb 1, 2023
@vincbeck vincbeck deleted the vincbeck/system-tests/eks branch May 19, 2023 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants