docker-archive / for-aws

92 stars 26 forks source link

Upgrade from 17.09 to 17.12 issue. EFS related. #138

Open netflash opened 6 years ago

netflash commented 6 years ago

Expected behavior

IF EncryptEFS set to false - nothing related to EFS should be happening.

Actual behavior

It doesn't matter if I set it to true or false, CF always wants to recreate EFS resources.

Information

Steps to reproduce the behavior

Try to upgrade from 17.09 to 17.12 and you'll that CloudFormation will re-create EFS resources. I went ahead and removed

                "Encrypted": {
                    "Ref": "EncryptEFS"
                },

lines from both EFS FileSystems - in this case CF doesn't want to do anything with EFS resources.

I believe it is actually an issue on AWS side.

netflash commented 6 years ago

Another test:

Brand new simple CF template:

{
    "AWSTemplateFormatVersion": "2010-09-09",
    "Metadata": {
        "AWS::CloudFormation::Designer": {
            "ea9bbbc6-6828-4bc0-a1e9-208989c8a465": {
                "size": {
                    "width": 60,
                    "height": 60
                },
                "position": {
                    "x": 78,
                    "y": 66
                },
                "z": 0
            }
        }
    },
    "Resources": {
        "EFSFS3MJLT": {
            "Type": "AWS::EFS::FileSystem",
            "Properties": {
                "FileSystemTags": [
                    {
                        "Key": "Name",
                        "Value": "Test"
                    }
                ],
                "PerformanceMode": "generalPurpose"
            },
            "Metadata": {
                "AWS::CloudFormation::Designer": {
                    "id": "ea9bbbc6-6828-4bc0-a1e9-208989c8a465"
                }
            }
        }
    }
}

Then I've updated it, by adding "Encrypted":false to the parameters and it leads to

https://www.dropbox.com/s/nquz1z0kfxorw96/Screenshot%202018-01-31%2011.51.48.png?dl=0

I'm going to open a support ticket (AWS).

FrenchBen commented 6 years ago

Thanks for the issue and details around it - If you don't mind opening the ticket on the AWS and then linking it here, it would help in figuring out what the next step should be.

netflash commented 6 years ago

Quick answer - if you add the "Encrypted" parameter, it doesn't matter if it's set to true or false, CFN will recreate the resource anyway.

So at least you guys should document this in Upgrade guide.

As a workaround we may use something like this:

+                    "Fn::If":[
+                        "Encrypted",
+                    {"Ref":"Encrypted"},
+                    {"Ref" : "AWS::NoValue"}
+                    ]

in the EFS resource. + it has do be added to conditions section as condition, same way as you have

CloudStorEfsSelected

configured.

I don't have time to test it myself.

netflash commented 6 years ago

Full response from AWS Support. I've been authorized to quote it here:

Thank you for contacting AWS Support. My name is Sinisa, and I will be glad to assist you. As I can see from your explanation, you are getting the change preview for a replacement of fs-XXXXXX after modifying its parameter in the CloudFormation template. This is an expected behavior for adding the "Encrypted" property to AWS::EFS::FileSystem resource. If you take a look at the update requirement AWS::EFS::FileSystem - Properties for the property, you'll see it requires a replacement. I fully understand how you'd expect CFN to ignore this property if it's set to false, but from the CloudFormation perspective it isn't so. Not having an "Encrypted" property or having it set to false makes a big difference. Please take a look at our document which explains the Update Behaviors of Stack Resources in more details. I've looked through the list of your EFS file systems and, while I couldn't find fs-XXXXXX, I noticed none of them were encrypted. To provide a resolution to this situation, I'd advise you not to introduce "Encrypted" property to your AWS::EFS::FileSystem resource in the template. Net result will be the same, only there will be no need for replacement. If you need your volume encrypted at any future point, you will need to replace it anyway, so that would be the good time to add this property. In that case, create another stack with "Encrypted" and copy the data from old EFS volume to the new one. We have a document about Amazon EFS File Sync to help you with this. Looking at the screenshot you posted, I see CloudFormation console warned you about this action. This is due to the way CFN calculates the changes and displays them under 'Preview your changes' section, thus allowing you to review them before they are executed. You can find more information about Change Sets in this document Updating Stacks Using Change Sets. Hopefully this information will help you achieve the same goal, although it may not be the answer you were looking for.

Sinisa M. Amazon Web Services

FrenchBen commented 6 years ago

@netflash thanks for the follow up - I'll add it to our docs FAQ.

update: https://github.com/docker/docker.github.io/pull/5926

hairyhenderson commented 6 years ago

FYI - based on https://github.com/docker/for-aws/issues/138#issuecomment-362643159 I've had success by patching the template with a jq script similar to this:

script='.Conditions.EFSEncrypted = { "Fn::Equals": [ { Ref: "EncryptEFS" }, "true" ] } |
.Resources.FileSystemGP.Properties.Encrypted = { "Fn::If" : [ "EFSEncrypted", {Ref: "EncryptEFS"}, {Ref: "AWS::NoValue"} ] } |
.Resources.FileSystemMaxIO.Properties.Encrypted = { "Fn::If" : [ "EFSEncrypted", {Ref: "EncryptEFS"}, {Ref: "AWS::NoValue"} ] }'
jq "$script" < Docker-no-vpc.tmpl > Docker-no-vpc.patched.tmpl

@FrenchBen would it be possible to get the CFN template updated with this for 18.03.1 or later?

FrenchBen commented 6 years ago

@hairyhenderson Thanks for the patch - it's been added to the template and will be available in the next Edge release (or 18.03.1 patch - whichever comes first)

hairyhenderson commented 6 years ago

@FrenchBen you're welcome, and thank you! 😉