Modeled Autoscaling EBS parameters #28
Conversation
|
Reducing our test coverage, tsk, tsk. |
| @@ -203,18 +203,20 @@ trait Autoscaling { | |||
| userData: `Fn::Base64`, | |||
| iam: Option[Token[ResourceRef[`AWS::IAM::InstanceProfile`]]] = None, | |||
| condition: Option[ConditionRef] = None, | |||
| dependsOn: Option[Seq[String]] = None | |||
| dependsOn: Option[Seq[String]] = None, | |||
| blockDevices: Option[Seq[BlockDeviceMapping]] = None | |||
There was a problem hiding this comment.
Typically we keep Condition and DependsOn last, but putting BlockDevices last allows backward compatibility.
There was a problem hiding this comment.
@ddgenome I can change it if you want, but I do know that microservices-vpc doesn't name parameters when it uses this, so it'd need to be updated when we move to this release of cftg. Let me know your preference.
There was a problem hiding this comment.
I think they do use named parameters now in microservices-vpc, but it's probably better to keep backwards compatibility.
|
For |
| } | ||
|
|
||
| case class AutoScalingEBS( | ||
| DeleteOnTermination: Option[Token[Boolean]] = None, |
There was a problem hiding this comment.
Are you sure a Token of a Boolean is allowed?
|
Regarding "For BlockDeviceMapping, since you can specify either Ebs, VirtualName, or neither but not both." I'm actually wondering in what scenario you would want neither. Would that just be a device name and a "NoDevice" case? What would that even be used for? Suppressing a default mapping from the AMI or something like that? |
|
I don't know what it would be used for, it just seems CloudFormation allows it. If you don't think it useful, feel free to just have two constructors. |
|
Cool, I'm going to stick with the two then. I've pushed that. I think I've covered all your comments. Thanks for the great feedback! And yeah, I did drop our test coverage by 0.6% from 29.62% to 29.02%. Sorry. :) |
I'm using this in the Vault VPC to specify an additional volume for the vault data so I can snapshot it without an outage. Looks to work fine.