aws-cloudformation / cfn-lint

CloudFormation Linter
MIT No Attribution
2.43k stars 586 forks source link

Validate UpdatePolicy for AutoScalingRollingUpdate #362

Closed adamchainz closed 3 weeks ago

adamchainz commented 5 years ago

cfn-lint version: (cfn-lint --version) 0.7.2

Description of issue.

Using a template like:

Resources:
  AutoScalingGroup:
    Type: AWS::AutoScaling::AutoScalingGroup
    Properties:
      # ...
      MinSize: "1"
      MaxSize: "1"
      # ...
    UpdatePolicy:
      AutoScalingRollingUpdate:
        MinInstancesInService: 1

will fail at deployment time with an error from CloudFormation like:

AWS::AutoScaling::AutoScalingGroup AutoScalingGroup UPDATE_FAILED: MinInstancesInService must be less than the autoscaling group's MaxSize

It would be nice to have protection against this. I guess there are other constraints, some are in the docs. I submitted a docs PR for this one.

cmmeyer commented 5 years ago

Agreed. This would be a good business logic addition to compare values. I'll label this as an enhancement.

kddejong commented 5 years ago

Started with rule E3016 to check the basic configuration of the UpdatePolicy (attributes, values are of the correct type), etc.

Moving down the chain to this one next.

kalpik commented 5 years ago

I have an issue with UpdatePolicy validation. I am using a different update policy for spot instances (replacingupdate) and regular instances (rolling update)

So something like:

UpdatePolicy:
            !If
                - spotInstances
                - AutoScalingReplacingUpdate:
                      WillReplace: true
                - AutoScalingRollingUpdate:
                      MinInstancesInService: !Ref DedicatedIngestNodeCount
                      MinSuccessfulInstancesPercent: 100
                      MaxBatchSize: 1
                      PauseTime: PT15M
                      WaitOnResourceSignals: true
                      SuspendProcesses:
                          - HealthCheck
                          - ReplaceUnhealthy
                          - AZRebalance
                          - AlarmNotification
                          - ScheduledActions

But this gives the following linting error:

E3016 UpdatePolicy doesn't support type Fn::If

This code actually works in CFN, so can the linting be fixed for this?

Thanks!

kddejong commented 5 years ago

Sorry @kalpik I lost track of this. I have created pull request #468 totally rewritten to handle conditions on each level. Used your example for adding to tests. Thanks for reporting this.

digash commented 3 years ago

I am getting error E3016 UpdatePolicy doesn't support type AutoScalingRollingUpdate from this AWS example: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/quickref-autoscaling.html#scenario-as-updatepolicy

kddejong commented 3 weeks ago

Closing this issue as it is covered in v1. Any improvements we can do we will do as requested.