Skip to content
This repository has been archived by the owner on Jun 13, 2021. It is now read-only.

Fix reference for service images we know by tag #716

Merged
merged 3 commits into from Oct 29, 2019

Conversation

ndeloof
Copy link
Contributor

@ndeloof ndeloof commented Oct 25, 2019

- What I did
Freeze service images at build step
https://docker.atlassian.net/browse/APP-294

- How I did it
For all service image we don't build, resolve addressable-digest from registry

- How to verify it
docker app build do generate a bundle.json with only digested references

- Description for the changelog
Service images are resolved into digested reference during build

- A picture of a cute animal (not mandatory but encouraged)
image

internal/commands/build/build.go Outdated Show resolved Hide resolved
@rumpl
Copy link
Member

rumpl commented Oct 25, 2019 via email

@ndeloof ndeloof force-pushed the APP-294 branch 2 times, most recently from d3a5aae to 035df07 Compare October 26, 2019 09:07
@ndeloof ndeloof force-pushed the APP-294 branch 5 times, most recently from 2f59a59 to 6eea0a8 Compare October 28, 2019 11:13
@codecov
Copy link

codecov bot commented Oct 28, 2019

Codecov Report

Merging #716 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #716   +/-   ##
======================================
  Coverage    72.9%   72.9%           
======================================
  Files          59      59           
  Lines        3351    3351           
======================================
  Hits         2443    2443           
  Misses        594     594           
  Partials      314     314

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0631b74...4aac7f0. Read the comment docs.

@ndeloof ndeloof marked this pull request as ready for review October 28, 2019 12:10
Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

LGTM

@ndeloof ndeloof force-pushed the APP-294 branch 2 times, most recently from 292fb7d to 75deb14 Compare October 28, 2019 15:38
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
internal/commands/build/build.go Outdated Show resolved Hide resolved
Aligned with docker service create --no-resolve-image
Prevent image freeze during the build
Usefull for offline builds, as well as for our e2e tests :P

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
@eunomie
Copy link
Member

eunomie commented Oct 29, 2019

@ndeloof I tried with my version of dockercoins and I have the following result:

could not resolve image docker.io/library/redis: object required

docker-compose.yml:

version: "3.6"

services:
  rng:
    image: dockercoins_rng
    build: rng
    ports:
    - "8001:80"

  hasher:
    image: dockercoins_hasher
    build: hasher
    ports:
    - "8002:80"

  webui:
    image: dockercoins_webui
    build: webui
    ports:
    - "8000:80"
    volumes:
    - "/Users/yvesbrissaud/src/github.com/eunomie/dockercoins/webui/files/:/files/"
    environment:
    - REDIS=redis

  redis:
    image: redis

  worker:
    image: dockercoins_worker
    build: worker
    environment:
    - REDIS=redis
    - HASHER=hasher
    - RNG=rng

@ndeloof ndeloof force-pushed the APP-294 branch 3 times, most recently from d8e7129 to a8a8d40 Compare October 29, 2019 14:38
Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
@ndeloof ndeloof merged commit 6267dc4 into docker:master Oct 29, 2019
@ndeloof ndeloof deleted the APP-294 branch October 29, 2019 16:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants