Skip to content
This repository has been archived by the owner on Jan 8, 2024. It is now read-only.

feat: aws-ecr-pull builder #3396

Merged
merged 5 commits into from
Jun 2, 2022
Merged

feat: aws-ecr-pull builder #3396

merged 5 commits into from
Jun 2, 2022

Conversation

thiskevinwang
Copy link
Contributor

@thiskevinwang thiskevinwang commented May 31, 2022

Resolves #3390

Checklist

  • deploys via local runner
  • deploys via remote runner; did not verify since 20bdea4

Usage

app "deno-http" {
  build {
+   use "aws-ecr-pull" {
+     region     = var.region
+     repository = "deno-http"
+     tag        = var.tag
+   }
  }

  deploy {
    use "aws-lambda" {
      region = var.region
      memory = 512
    }
  }

  release {
    use "lambda-function-url" {

    }
  }
}

@vercel
Copy link

vercel bot commented May 31, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
waypoint-ui ✅ Ready (Inspect) Visit Preview Jun 2, 2022 at 4:48AM (UTC)

Copy link
Contributor

@evanphx evanphx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just one minor thing to work around the ECR architecture issue you pointed out.

Comment on lines 161 to 166
// TODO(kevinwang): Do we need to get architecture?
// If we do, we can pull the image and inspect it via `cli.ImageInspectWithRaw`,
// - prior art: /builtin/docker/builder.go -> Build
// There is also an open issue for the ECR team to build a architecture feature into
// the UI, which probably comes with a CLI/API change.
// - see https://github.com/aws/containers-roadmap/issues/1591
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since ECR doesn't expose the architecture in an obvious way here yet, I'd probably make an option on the plugin called force_architecture that will set the architecture on output. This will make it at least work with the other bits we've been adding architecture to.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added force_architecture in 20bdea4

force_architecture = "x86_64"
CleanShot 2022-06-02 at 00 35 58@2x

force_architecture omitted
CleanShot 2022-06-02 at 00 35 08@2x

Copy link
Contributor

@evanphx evanphx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@evanphx evanphx requested a review from a team June 2, 2022 17:21
@paladin-devops paladin-devops added the ecosystem Things related to waypoint interacting with external systems label Jun 2, 2022
Copy link
Contributor

@paladin-devops paladin-devops left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this out with a local & remote runner, looks great! I've also added an example usage of this plugin to the examples repo.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
core ecosystem Things related to waypoint interacting with external systems plugin/aws plugin website
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feature request: plugin/ecr-pull
3 participants