Moving docker service digest pinning to client side #32388
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| package client | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "net/url" | ||
|
|
||
| registrytypes "github.com/docker/docker/api/types/registry" | ||
| "golang.org/x/net/context" | ||
| ) | ||
|
|
||
| // DistributionInspect returns the image digest with full Manifest | ||
| func (cli *Client) DistributionInspect(ctx context.Context, image, encodedRegistryAuth string) (registrytypes.DistributionInspect, error) { | ||
| var headers map[string][]string | ||
|
|
||
| if encodedRegistryAuth != "" { | ||
| headers = map[string][]string{ | ||
| "X-Registry-Auth": {encodedRegistryAuth}, | ||
| } | ||
| } | ||
|
|
||
| // Contact the registry to retrieve digest and platform information | ||
| var distributionInspect registrytypes.DistributionInspect | ||
| resp, err := cli.get(ctx, "/distribution/"+image+"/json", url.Values{}, headers) | ||
| if err != nil { | ||
| return distributionInspect, err | ||
| } | ||
|
|
||
| err = json.NewDecoder(resp.body).Decode(&distributionInspect) | ||
| ensureReaderClosed(resp) | ||
| return distributionInspect, err | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,29 +2,78 @@ package client | |
|
|
||
| import ( | ||
| "encoding/json" | ||
| "fmt" | ||
|
|
||
| "github.com/docker/distribution/reference" | ||
| "github.com/docker/docker/api/types" | ||
| "github.com/docker/docker/api/types/swarm" | ||
| "github.com/opencontainers/go-digest" | ||
| "golang.org/x/net/context" | ||
| ) | ||
|
|
||
| // ServiceCreate creates a new Service. | ||
| func (cli *Client) ServiceCreate(ctx context.Context, service swarm.ServiceSpec, options types.ServiceCreateOptions) (types.ServiceCreateResponse, error) { | ||
| var distErr error | ||
|
|
||
| headers := map[string][]string{ | ||
| "version": {cli.version}, | ||
| } | ||
|
|
||
| if options.EncodedRegistryAuth != "" { | ||
| headers["X-Registry-Auth"] = []string{options.EncodedRegistryAuth} | ||
| } | ||
|
|
||
| // Contact the registry to retrieve digest and platform information | ||
| if options.QueryRegistry { | ||
| distributionInspect, err := cli.DistributionInspect(ctx, service.TaskTemplate.ContainerSpec.Image, options.EncodedRegistryAuth) | ||
| distErr = err | ||
| if err == nil { | ||
| // now pin by digest if the image doesn't already contain a digest | ||
| img := imageWithDigestString(service.TaskTemplate.ContainerSpec.Image, distributionInspect.Descriptor.Digest) | ||
| if img != "" { | ||
| service.TaskTemplate.ContainerSpec.Image = img | ||
| } | ||
| } | ||
| } | ||
| var response types.ServiceCreateResponse | ||
| resp, err := cli.post(ctx, "/services/create", nil, service, headers) | ||
| if err != nil { | ||
| return response, err | ||
| } | ||
|
|
||
| err = json.NewDecoder(resp.body).Decode(&response) | ||
|
|
||
| if distErr != nil { | ||
| response.Warnings = append(response.Warnings, digestWarning(service.TaskTemplate.ContainerSpec.Image)) | ||
| } | ||
|
|
||
| ensureReaderClosed(resp) | ||
| return response, err | ||
| } | ||
|
|
||
| // imageWithDigestString takes an image string and a digest, and updates | ||
| // the image string if it didn't originally contain a digest. It assumes | ||
| // that the image string is not an image ID | ||
| func imageWithDigestString(image string, dgst digest.Digest) string { | ||
| isCanonical := false | ||
| ref, err := reference.ParseAnyReference(image) | ||
| if err == nil { | ||
| _, isCanonical = ref.(reference.Canonical) | ||
|
|
||
| if !isCanonical { | ||
| namedRef, _ := ref.(reference.Named) | ||
| img, err := reference.WithDigest(namedRef, dgst) | ||
| if err == nil { | ||
| return img.String() | ||
| } | ||
| } | ||
| } | ||
| return "" | ||
| } | ||
|
|
||
| // digestWarning constructs a formatted warning string using the | ||
| // image name that could not be pinned by digest. The formatting | ||
| // is hardcoded, but could me made smarter in the future | ||
| func digestWarning(image string) string { | ||
| return fmt.Sprintf("image %s could not be accessed on a registry to record\nits digest. Each node will access %s independently,\npossibly leading to different nodes running different\nversions of the image.\n", image, image) | ||
|
There was a problem hiding this comment. the There was a problem hiding this comment. @tiborvass this is to add line breaks, since the text is big. This is how it is also in the daemon right now. |
||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nishanttotla I'm wondering if the default shouldn't be the current behavior.
SkipRegistryQuery?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tiborvass the corresponding CLI PR is docker/cli#30.
The idea is that
QueryRegistryshould be set on allservice createcommands unless the user explicitly provides a flag to prevent it, and for allservice updatecommands that update the image, using the--imageflag (also unless the user explicitly asks to not do it with the flag).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tiborvass: The current behavior is that the client does not perform this query.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aaronlehmann my bad i thought this was server side.