Enable AMI deprecation: InvalidAMIID.NotFound: Invalid ImageId

dgibbard-cisco commented 2 years ago

Overview of the Issue

When copying a created AMI to multiple regions with deprecate_at set, packer fails with:

eks-packer-ami-build.amazon-ebs.this: Error enable AMI deprecation: InvalidAMIID.NotFound: Invalid ImageId: ami-0123456789
==> eks-packer-ami-build.amazon-ebs.this:   status code: 400, request id: xxxxx

This looks related to:

Recent:

Issue is present in plugin v1.1.3, and not in v1.1.2

The difference between the two is that in 1.1.2, Deprecation is added to the image before copying, and in 1.1.3, Deprecation is added after the copy.

Reproduction Steps

Build an AMI, with ami_regions as a list of more than one region, and deprecate_at set.

Plugin and Packer version

Packer 1.8.3 (also tested with 1.7.9)
Plugin version 1.1.3

Simplified Packer Buildfile

locals {
  expiration_date_hours = var.ami_expiration_days * 24
  expiration_date = timeadd(timestamp(), "${local.expiration_date_hours}h")
}

source "amazon-ebs" "this" {
  ami_name = "myami"
  ami_description = "My AMI"
  ami_users = var.aws_account_ids
  ami_regions = var.aws_regions
  ami_virtualization_type = "hvm"
  ebs_optimized = true
  encrypt_boot = false
  deprecate_at = local.expiration_date
  associate_public_ip_address = false
  vpc_filter {
    filters = {
      "tag:Name": var.vpc_tag_name,
      isDefault: false
    }
  }
  subnet_filter {
    filters = {
      "tag:Name": var.subnet_tag_name
    }
  }
  security_group_filter {
    filters = {
      "tag:Name": var.security_group_name
    }
  }
  source_ami_filter {
    filters = {
        <INSERT_FILTERS_HERE>
    }
    most_recent = true
  }
  instance_type = "m5.xlarge"
  ssh_username = "ec2-user"
}

build {
  name = "eks-packer-ami-build"
  sources = ["source.amazon-ebs.this"]
}

Operating system and Environment details

Linux x86_64

Log Fragments and crash.log files

TBD

Workaround

Pinning the version to 1.1.2 resolves this for now:

packer {
  required_plugins {
    amazon = {
      version = "= 1.1.2"
      source  = "github.com/hashicorp/amazon"
    }
  }
}

wampiedriessen commented 2 years ago

Hi @dgibbard-cisco I see you mentioned my merged PR in the description of your issue. Looking at it, it seems like my changes have indeed introduced this bug.

You should also note that the deprecate_ami feature did not work as intended before my changes (the AMI's copied to other regions were never set to deprecated).

I think the fix will be an easy one and I will try to find the time to implement it somewhere in the coming week.

dgibbard-cisco commented 2 years ago

@wampiedriessen thank you for your prompt response! That makes sense - I look forward to the fix, but have pinned the version to 1.1.2 in the meantime anyway so we're not blocked by it :)

Thanks again!

lbajolet-hashicorp commented 2 years ago

Hi @dgibbard-cisco, sorry for the regression, as I approved the PR this is my mistake, apologies for that.

Thank you for pointing out the initial issue, I think it may be worth taking a step back on that and analyse a bit more what's happening here, since the change that was merged in PR #251 essentially reverted the change made in #138, which fixed the behaviour for encrypted AMIs, but made it not work again for AMIs copied to other regions. There may be a straightforward fix, but I wonder if this indicates a need for a deeper reorganisation of the code. I will investigate this on my side. That being said @wampiedriessen, if you manage to get it working, please do open a PR and let us know when it is time for a review, I'll gladly take a look at it.

Thanks again for the feedback and the efforts, much appreciated!

lbajolet-hashicorp commented 2 years ago

Hey there @wampiedriessen and @dgibbard-cisco, as you may see, I opened PR #262 that fixes this issue. I can confirm with those changes it works when depreciating an AMI copied on several regions, and added an acceptance test to make sure of it.

@dgibbard-cisco, if possible, would you be able to run the version of the plugin with those changes and confirm that it fixes your issue there?

dgibbard-cisco commented 2 years ago

I had a quick go/look but the deprecation data wasn't copied over to the other regions -- I can only assume that my setup is completely wrong and not using the locally built plugin :) The packer documentation doesn't really go into any depth about testing/using locally built plugins, which is annoying, and I can't see any information on which plugin is loaded with PACKER_LOG=1 or -debug set.

Don't wait on me though- if you're able to confirm that deprecation is copied, and I see we have tests in place; feel free to push on, and I can test once a new release is built if I don't work it out before then.

lbajolet-hashicorp commented 2 years ago

Hey @dgibbard-cisco,

In order for Packer to be able to load the plugin, it needs to be discoverable by Packer, when using PACKER_LOG=1, you should be able to see which plugins are discovered. For example on my local setup I can see the log: [DEBUG] Discovered plugin: amazon = /home/elbaj/.packer.d/plugins/packer-plugin-amazon, which indicates this is the binary built locally, not a released one.

My advice for getting it to work in your local setup would probably be to invoke make dev, as it will move the built binary in the ~/.packer.d/plugins/ directory, where it can be loaded with a higher precedance compared to the plugins downloaded by Packer.

Do note that I have only tested this on Linux, and given the way the Makefile works, I would assume this won't necessarily work on a Windows machine, so that that advice with a grain of salt.

When I mentioned that I tested it, I tested it through my acceptance tests, not in a manual workflow. I'll try to do that this morning, but in the meantime if you can confirm that on your setup, that would be a good sign we're on the right track with that fix :)

lbajolet-hashicorp commented 2 years ago

Manual testing in my environment does point out that it works with both copy + encryption + deprecation time, I think we're good

dgibbard-cisco commented 2 years ago

@lbajolet-hashicorp Thanks for the tip! I was having trouble working out where packer expects plugins to be; make dev worked :)

I've run it now with replication of non-encrypted AMIs to four regions; checked created images, deprecation set correctly on all :)

This looks perfect, thanks for the quick turnaround!

hashicorp / packer-plugin-amazon