-
Notifications
You must be signed in to change notification settings - Fork 53
Add ElasticIPAddress
CRD
#39
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
Conversation
/test ec2-kind-e2e |
/approve |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Left one comment below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@RedbackThomson I don't actually think we should support PubicIP (EC2-Classic). EC2-Classic is being fully retired on August 15, 2022 which is about the closest AWS will ever get to removing an API :)
https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/ec2-classic-platform.html
14e913a
to
6881515
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks solid to me
PublicIp: | ||
print: | ||
name: PUBLIC-IP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We wanted to remove this, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it not still something that's relevant,even for EC2-VPC? I think being able to see the allocated IP is still relevant. I just don't use it as the identifier anywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[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 |
Issue #, if available: aws-controllers-k8s/community#1218
Description of changes:
Creates the
ElasticIPAddress
custom resource, fromAllocateAddress
andDeallocateAddress
. This API uses eitherAllocationID
orPublicIP
as the identifier depending on whether the domain is set to"vpc"
(for EC2-VPC) or otherwise (for EC2-Classic), respectively. For the adoption, I have chosen to setAllocationID
as the primary key for now, since EC2-VPC is the recommended method for managing resources in EC2.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.