VPC attachment for InternetGateway
#48
Conversation
RedbackThomson
force-pushed
the
vpc-ig-attach
branch
from
071059e to
debab66
Compare
|
/retest |
| # In the case where it shows the attachment as being in detached state | ||
| if len(igw["Attachments"]) == 1: | ||
| assert igw["Attachments"][0]["VpcId"] == vpc_id | ||
| assert igw["Attachments"][0]["State"] == "detached" |
There was a problem hiding this comment.
Should this be an or check? "detached" or "detaching"
There was a problem hiding this comment.
It only takes a few fractions of a second to detach. Our sleep above should suffice for this.
| def test_vpc_association(self, ec2_client): | ||
| resource_name = random_suffix_name("ig-ack-test", 24) | ||
|
|
||
| test_vpc = get_bootstrap_resources().EmptyVPC |
There was a problem hiding this comment.
If this VPC is only used here, then would you be opposed to creating the VPC in this test and exercise the resource reference code paths?
There was a problem hiding this comment.
Yeah happy to move it into the test as a fixture, itself. Not sure about using it as a ref, though. I think we need to have a holistic discussion around testing references in general. Adding a test for every field, and then another test for every ref, is going to blow up the size of our tests and I don't know how much value it actually brings (since the reference resolution code is generated deterministically). I'll leave that for another day, though.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: brycahta, RedbackThomson, vijtrip2 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Closes aws-controllers-k8s/community#1216
Description of changes:
Add support for attaching an
InternetGatewayto aVPC. The EC2 API exposes a list ofAttachmentswhen you describeInternetGateway, but manual testing shows that you can only attach a gateway to a single VPC - so this is treated as a list of at most one element. I had to create a new VPC for integration tests, since the default VPC we use for all of the other tests already contains an IGW, and you can have at most 1 per VPC.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.