Skip to content

Commit

Permalink
fix: respect image.location uri for containers (#5555)
Browse files Browse the repository at this point in the history
Addresses: #5501 (comment)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.
  • Loading branch information
Lou1415926 authored Dec 12, 2023
1 parent 98bacee commit dd7f30c
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 36 deletions.
7 changes: 3 additions & 4 deletions e2e/sidecars/sidecars_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ var docker *client.Docker
var appName string
var envName string
var svcName string
var sidecarImageURI string
var sidecarRepoName string
var mainRepoName string

// The Sidecars suite runs creates a new service with sidecar containers.
func TestSidecars(t *testing.T) {
Expand All @@ -40,12 +39,12 @@ var _ = BeforeSuite(func() {
appName = fmt.Sprintf("e2e-sidecars-%d", time.Now().Unix())
envName = "test"
svcName = "hello"
sidecarRepoName = fmt.Sprintf("e2e-sidecars-nginx-%d", time.Now().Unix())
mainRepoName = fmt.Sprintf("e2e-sidecars-main-%d", time.Now().Unix())
})

var _ = AfterSuite(func() {
_, appDeleteErr := cli.AppDelete()
repoDeleteErr := command.Run("aws", []string{"ecr", "delete-repository", "--repository-name", sidecarRepoName, "--force"})
repoDeleteErr := command.Run("aws", []string{"ecr", "delete-repository", "--repository-name", mainRepoName, "--force"})
Expect(appDeleteErr).NotTo(HaveOccurred())
Expect(repoDeleteErr).NotTo(HaveOccurred())
})
Expand Down
54 changes: 27 additions & 27 deletions e2e/sidecars/sidecars_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type: Load Balanced Web Service
image:
# Path to your service's Dockerfile.
build: hello/Dockerfile
location: %s
# Port exposed through your container to route traffic to it.
port: 3000
depends_on:
Expand All @@ -52,9 +52,10 @@ count: 1
sidecars:
nginx:
port: 80
image: %s # Image URL for sidecar container.
image:
build: nginx/Dockerfile
variables:
NGINX_PORT: %s
NGINX_PORT: 80
env_file: ./magic.env
logging:
env_file: ./magic.env
Expand All @@ -65,9 +66,30 @@ logging:
log_stream_prefix: copilot/
`

const nginxPort = "80"
var mainImageURI string

var _ = Describe("sidecars flow", func() {
Context("build and push main image to ECR repo", func() {
var uri string
var err error
It("create new ECR repo for main container", func() {
uri, err = aws.CreateECRRepo(mainRepoName)
Expect(err).NotTo(HaveOccurred(), "create ECR repo for main container")
mainImageURI = fmt.Sprintf("%s:mytag", uri)
})
It("push main container image", func() {
var password string
password, err = aws.ECRLoginPassword()
Expect(err).NotTo(HaveOccurred(), "get ecr login password")
err = docker.Login(uri, password)
Expect(err).NotTo(HaveOccurred(), "docker login")
err = docker.Build(mainImageURI, "./hello")
Expect(err).NotTo(HaveOccurred(), "build main container image")
err = docker.Push(mainImageURI)
Expect(err).NotTo(HaveOccurred(), "push to ECR repo")
})
})

Context("when creating a new app", Ordered, func() {
var (
initErr error
Expand Down Expand Up @@ -167,33 +189,11 @@ var _ = Describe("sidecars flow", func() {
})
})

Context("build and push sidecar image to ECR repo", func() {
var uri string
var err error
tag := "vortexstreet"
It("create new ECR repo for sidecar", func() {
uri, err = aws.CreateECRRepo(sidecarRepoName)
Expect(err).NotTo(HaveOccurred(), "create ECR repo for sidecar")
sidecarImageURI = fmt.Sprintf("%s:%s", uri, tag)
})
It("push sidecar image", func() {
var password string
password, err = aws.ECRLoginPassword()
Expect(err).NotTo(HaveOccurred(), "get ecr login password")
err = docker.Login(uri, password)
Expect(err).NotTo(HaveOccurred(), "docker login")
err = docker.Build(sidecarImageURI, "./nginx")
Expect(err).NotTo(HaveOccurred(), "build sidecar image")
err = docker.Push(sidecarImageURI)
Expect(err).NotTo(HaveOccurred(), "push to ECR repo")
})
})

Context("write local manifest and addon files", func() {
var newManifest string
It("overwrite existing manifest", func() {
logGroupName := fmt.Sprintf("%s-test-%s", appName, svcName)
newManifest = fmt.Sprintf(manifest, sidecarImageURI, nginxPort, logGroupName)
newManifest = fmt.Sprintf(manifest, mainImageURI, logGroupName)
err := os.WriteFile("./copilot/hello/manifest.yml", []byte(newManifest), 0644)
Expect(err).NotTo(HaveOccurred(), "overwrite manifest")
})
Expand Down
5 changes: 4 additions & 1 deletion internal/pkg/deploy/cloudformation/stack/transformers.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,10 @@ func convertSidecars(s map[string]*manifest.SidecarConfig, exposedPorts map[stri
sort.Strings(keys)
for _, name := range keys {
config := s[name]
imageURI := rc.PushedImages[name].URI()
var imageURI string
if image, ok := rc.PushedImages[name]; ok {
imageURI = image.URI()
}
if uri, hasLocation := config.ImageURI(); hasLocation {
imageURI = uri
}
Expand Down
8 changes: 4 additions & 4 deletions internal/pkg/deploy/cloudformation/stack/workload.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@ func (w *wkld) Parameters() ([]*cloudformation.Parameter, error) {
if w.image != nil {
img = w.image.GetLocation()
}
if w.rc.PushedImages != nil {
img = w.rc.PushedImages[w.name].URI()
if image, ok := w.rc.PushedImages[w.name]; ok {
img = image.URI()
}
return []*cloudformation.Parameter{
{
Expand Down Expand Up @@ -416,8 +416,8 @@ func (w *appRunnerWkld) Parameters() ([]*cloudformation.Parameter, error) {
if w.image != nil {
img = w.image.GetLocation()
}
if w.rc.PushedImages != nil {
img = w.rc.PushedImages[w.name].URI()
if image, ok := w.rc.PushedImages[w.name]; ok {
img = image.URI()
}

imageRepositoryType, err := apprunner.DetermineImageRepositoryType(img)
Expand Down

0 comments on commit dd7f30c

Please sign in to comment.