Browse code
Fix layer exclusion bug on multiple tag push
Ensure that layers are not excluded from manifests based on previous pushes.
Continue skipping pushes on layers which were pushed by a previous tag.
Update push multiple tag tests.
Ensure that each tag pushed exists on the registry and is pullable.
Add output comparison on multiple tag push check.
fixes #15536
Signed-off-by: Derek McGowan <derek@mcgstyle.net> (github: dmcgowan)
Derek McGowan authored on 2015/08/13 08:02:12Showing 2 changed files
| ... | ... |
@@ -5,6 +5,7 @@ import ( |
| 5 | 5 |
"io" |
| 6 | 6 |
|
| 7 | 7 |
"github.com/Sirupsen/logrus" |
| 8 |
+ "github.com/docker/distribution/digest" |
|
| 8 | 9 |
"github.com/docker/docker/cliconfig" |
| 9 | 10 |
"github.com/docker/docker/pkg/streamformatter" |
| 10 | 11 |
"github.com/docker/docker/registry" |
| ... | ... |
@@ -44,12 +45,13 @@ func (s *TagStore) NewPusher(endpoint registry.APIEndpoint, localRepo Repository |
| 44 | 44 |
switch endpoint.Version {
|
| 45 | 45 |
case registry.APIVersion2: |
| 46 | 46 |
return &v2Pusher{
|
| 47 |
- TagStore: s, |
|
| 48 |
- endpoint: endpoint, |
|
| 49 |
- localRepo: localRepo, |
|
| 50 |
- repoInfo: repoInfo, |
|
| 51 |
- config: imagePushConfig, |
|
| 52 |
- sf: sf, |
|
| 47 |
+ TagStore: s, |
|
| 48 |
+ endpoint: endpoint, |
|
| 49 |
+ localRepo: localRepo, |
|
| 50 |
+ repoInfo: repoInfo, |
|
| 51 |
+ config: imagePushConfig, |
|
| 52 |
+ sf: sf, |
|
| 53 |
+ layersPushed: make(map[digest.Digest]bool), |
|
| 53 | 54 |
}, nil |
| 54 | 55 |
case registry.APIVersion1: |
| 55 | 56 |
return &v1Pusher{
|
| ... | ... |
@@ -27,6 +27,11 @@ type v2Pusher struct {
|
| 27 | 27 |
config *ImagePushConfig |
| 28 | 28 |
sf *streamformatter.StreamFormatter |
| 29 | 29 |
repo distribution.Repository |
| 30 |
+ |
|
| 31 |
+ // layersPushed is the set of layers known to exist on the remote side. |
|
| 32 |
+ // This avoids redundant queries when pushing multiple tags that |
|
| 33 |
+ // involve the same layers. |
|
| 34 |
+ layersPushed map[digest.Digest]bool |
|
| 30 | 35 |
} |
| 31 | 36 |
|
| 32 | 37 |
func (p *v2Pusher) Push() (fallback bool, err error) {
|
| ... | ... |
@@ -117,6 +122,10 @@ func (p *v2Pusher) pushV2Tag(tag string) error {
|
| 117 | 117 |
return err |
| 118 | 118 |
} |
| 119 | 119 |
|
| 120 |
+ // break early if layer has already been seen in this image, |
|
| 121 |
+ // this prevents infinite loops on layers which loopback, this |
|
| 122 |
+ // cannot be prevented since layer IDs are not merkle hashes |
|
| 123 |
+ // TODO(dmcgowan): throw error if no valid use case is found |
|
| 120 | 124 |
if layersSeen[layer.ID] {
|
| 121 | 125 |
break |
| 122 | 126 |
} |
| ... | ... |
@@ -138,6 +147,13 @@ func (p *v2Pusher) pushV2Tag(tag string) error {
|
| 138 | 138 |
dgst, err := p.graph.GetDigest(layer.ID) |
| 139 | 139 |
switch err {
|
| 140 | 140 |
case nil: |
| 141 |
+ if p.layersPushed[dgst] {
|
|
| 142 |
+ exists = true |
|
| 143 |
+ // break out of switch, it is already known that |
|
| 144 |
+ // the push is not needed and therefore doing a |
|
| 145 |
+ // stat is unnecessary |
|
| 146 |
+ break |
|
| 147 |
+ } |
|
| 141 | 148 |
_, err := p.repo.Blobs(context.Background()).Stat(context.Background(), dgst) |
| 142 | 149 |
switch err {
|
| 143 | 150 |
case nil: |
| ... | ... |
@@ -173,6 +189,7 @@ func (p *v2Pusher) pushV2Tag(tag string) error {
|
| 173 | 173 |
m.History = append(m.History, manifest.History{V1Compatibility: string(jsonData)})
|
| 174 | 174 |
|
| 175 | 175 |
layersSeen[layer.ID] = true |
| 176 |
+ p.layersPushed[dgst] = true |
|
| 176 | 177 |
} |
| 177 | 178 |
|
| 178 | 179 |
logrus.Infof("Signed manifest for %s:%s using daemon's key: %s", p.repo.Name(), tag, p.trustKey.KeyID())
|