Add support for storing subnet id in ec2instance and report vpc for stored subnet id in describeInstances #58
Conversation
…fault-configuration-for-aws-infrastructure-nodes-vpcs [Do not merge] Task/casp 621 add default configuration for aws infrastructure nodes vpcs
| * @param subnetId The subnet id. | ||
| * @return The VPC id. Returns null, if no matching subnet is found. | ||
| */ | ||
| private String getVpcForSubnetId(final String subnetId) { |
There was a problem hiding this comment.
It would be more meaningful if we change the name to getVpcIdForSubnet
| return subnet.getVpcId(); | ||
| } | ||
| } | ||
| return null; |
There was a problem hiding this comment.
Since it's always expected that a VPC is tied to a subnet - it's better if we throw an Exception than returning null. It will also be difficult to catch if for some reason we do return null.
There was a problem hiding this comment.
If we use a default value for not provided subnetIds, then we need here either a check for MOCK_SUBNET_ID where we return MOCK_VPC_ID. Or we just return MOCK_VPC_ID by default.
if (MOCK_SUBNET_ID.equals(subnetId)) { return MOCK_VPC_ID; }
Or
private String getVpcIdForSubnet(final String subnetId) { for (MockSubnet subnet : mockSubnetController.describeSubnets()) { if (subnet.getSubnetId().equals(subnetId)) { return subnet.getVpcId(); } } return MOCK_VPC_ID; }
What would be your prefered solution?
There was a problem hiding this comment.
I feel we can return MOCK_VPC_ID if no matching vpcId found.
| @@ -15,7 +15,7 @@ | |||
| * | |||
| * @see Serializable | |||
| */ | |||
| private static final long serialVersionUID = 1L; | |||
| private static final long serialVersionUID = 2L; | |||
There was a problem hiding this comment.
Could you explain why the version is incremented for this class?
There was a problem hiding this comment.
You are right, the base model would be a better place for the increment.
There was a problem hiding this comment.
Yeah not necessary a change here.
| final String[] subnetIdArray = queryParams.get("SubnetId"); | ||
| String subnetId = null; | ||
| if (subnetIdArray != null) { | ||
| subnetId = subnetIdArray[0]; |
There was a problem hiding this comment.
What if SubnetId is not provided? Can we allocate a random uuid as the subnetId for an instance?
There was a problem hiding this comment.
Another possibilty would be, that we use the MOCK_SUBNET_ID as default.
What do you think?
There was a problem hiding this comment.
I agree that we use MOCK_SUBNET_ID as default.
|
Hi @KraProgrammer really sorry for the late response. |
|
I'm planning on having this work merged in. |
Add support for persisting subnet id. The subnet id is also used to derive the vpc id.
Previously the default subnet id and vpc id were returned for all instances.