Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(main.go,actions): create a docker image tagging command #89

Merged
merged 4 commits into from
Jun 29, 2016

Conversation

arschles
Copy link
Member

@arschles arschles commented Jun 8, 2016

Fixes #57
Fixes #6
Fixes #58

Associated Documentation PR(s)

deis/workflow#355

@arschles
Copy link
Member Author

Pulling missing images & re-tagging them is tested and working (see output below). Pushing images is not yet tested.

ENG000656:deisrel aaronschlesinger$ time ./deisrel docker tag v2.0.2 --prompt-tags=false --push=false --force-retag=true
0. Using SHA '43a66c9f209f52ca5c54f9ab089bce1ab8338fa4' for repo builder
1. Using SHA '0e6473b250883f1a696daf246927abadc2cdce1b' for repo controller
2. Using SHA 'bce1fd717ebb142e8714eb1899829d300c736920' for repo dockerbuilder
3. Using SHA '2ab194a7ccf9c523eeeefeb43e05da9d1fa55544' for repo fluentd
4. Using SHA 'a9934d4425085e331008a2d1c94f31284fb61299' for repo logger
5. Using SHA '958ccd7af60354e6263278ba6f592a5449e25ed8' for repo minio
6. Using SHA '99028fa4327997d0101ec061caf6ba555b0c00e1' for repo monitor
7. Using SHA 'ebd3e1c0f8d7a2580d5c097a924ae3a55553f772' for repo postgres
8. Using SHA '7a0fb9715ca5d8b2c0fe847164247b74a33facb5' for repo registry
9. Using SHA 'eb6b05c55873805c87cf69297c8ae32bf3f532cd' for repo router
10. Using SHA 'e95687ee174ed0a3a991aab7333fae9243a83890' for repo slugbuilder
11. Using SHA '9d16ae7237f9e938385d91912b15365a164314c4' for repo slugrunner
12. Using SHA 'c37e3ec5390222e5f773ad2a7b4fc19cd0ca23f2' for repo stdout-metrics
13. Using SHA 'f21e0a81a4092685dee195766db778bb4592468a' for repo workflow-e2e
14. Using SHA '6e9100ccea52648ec44b95ab8be350e8a45086a6' for repo workflow-manager
Re-tagging images...
starting tag operation from quay.io/deisci/builder:git-43a66c9 to quay.io/deisci/builder:v2.0.2
starting tag operation from quay.io/deisci/workflow-e2e:git-f21e0a8 to quay.io/deisci/workflow-e2e:v2.0.2
starting tag operation from quay.io/deisci/logger:git-a9934d4 to quay.io/deisci/logger:v2.0.2
starting tag operation from quay.io/deisci/workflow-manager:git-6e9100c to quay.io/deisci/workflow-manager:v2.0.2
starting tag operation from quay.io/deisci/monitor:git-99028fa to quay.io/deisci/monitor:v2.0.2
starting tag operation from quay.io/deisci/controller:git-0e6473b to quay.io/deisci/controller:v2.0.2
starting tag operation from quay.io/deisci/router:git-eb6b05c to quay.io/deisci/router:v2.0.2
starting tag operation from quay.io/deisci/registry:git-7a0fb97 to quay.io/deisci/registry:v2.0.2
starting tag operation from quay.io/deisci/minio:git-958ccd7 to quay.io/deisci/minio:v2.0.2
starting tag operation from quay.io/deisci/slugbuilder:git-e95687e to quay.io/deisci/slugbuilder:v2.0.2
starting tag operation from quay.io/deisci/dockerbuilder:git-bce1fd7 to quay.io/deisci/dockerbuilder:v2.0.2
starting tag operation from quay.io/deisci/stdout-metrics:git-c37e3ec to quay.io/deisci/stdout-metrics:v2.0.2
starting tag operation from quay.io/deisci/slugrunner:git-9d16ae7 to quay.io/deisci/slugrunner:v2.0.2
starting tag operation from quay.io/deisci/postgres:git-ebd3e1c to quay.io/deisci/postgres:v2.0.2
starting tag operation from quay.io/deisci/fluentd:git-2ab194a to quay.io/deisci/fluentd:v2.0.2
finished tag operation from quay.io/deisci/fluentd:git-2ab194a to quay.io/deisci/fluentd:v2.0.2
starting pull operation on image quay.io/deisci/monitor:git-99028fa
finished tag operation from quay.io/deisci/slugrunner:git-9d16ae7 to quay.io/deisci/slugrunner:v2.0.2
finished tag operation from quay.io/deisci/postgres:git-ebd3e1c to quay.io/deisci/postgres:v2.0.2
finished tag operation from quay.io/deisci/workflow-manager:git-6e9100c to quay.io/deisci/workflow-manager:v2.0.2
finished tag operation from quay.io/deisci/dockerbuilder:git-bce1fd7 to quay.io/deisci/dockerbuilder:v2.0.2
finished tag operation from quay.io/deisci/registry:git-7a0fb97 to quay.io/deisci/registry:v2.0.2
finished tag operation from quay.io/deisci/builder:git-43a66c9 to quay.io/deisci/builder:v2.0.2
finished tag operation from quay.io/deisci/controller:git-0e6473b to quay.io/deisci/controller:v2.0.2
finished tag operation from quay.io/deisci/logger:git-a9934d4 to quay.io/deisci/logger:v2.0.2
finished tag operation from quay.io/deisci/stdout-metrics:git-c37e3ec to quay.io/deisci/stdout-metrics:v2.0.2
finished tag operation from quay.io/deisci/workflow-e2e:git-f21e0a8 to quay.io/deisci/workflow-e2e:v2.0.2
finished tag operation from quay.io/deisci/minio:git-958ccd7 to quay.io/deisci/minio:v2.0.2
finished tag operation from quay.io/deisci/router:git-eb6b05c to quay.io/deisci/router:v2.0.2
finished tag operation from quay.io/deisci/slugbuilder:git-e95687e to quay.io/deisci/slugbuilder:v2.0.2
Tag git-99028fa not found in repository quay.io/deisci/monitor
2016/06/17 16:15:36 Failed to re-tag every image. See errors below

real    0m31.869s
user    0m0.263s
sys 0m0.055s

@vdice
Copy link
Member

vdice commented Jun 17, 2016

@arschles one comment while I think of it (have yet to actually review this PR):

Would like to standardize on one approach or the other for all the deisrel commands that potentially require such safeguards as docker tag here uses. For instance, the existing deisrel git tag command utilizes the prompt w/ override approach while this PR introduces the --push=false approach for preventing actual remote api calls.

If we are good with following the precedent established by the deisrel git tag command, I'd propose we use the same for this command (prompt w/ override). Otherwise, if consensus on the topic has yet to be achieved, I am definitely fine with creating a separate issue, non-blocking for this PR, to establish such standardization.

@@ -24,12 +26,17 @@ func main() {
ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: ghTkn})
cl := oauth2.NewClient(oauth2.NoContext, ts)
ghClient := github.NewClient(cl)
dockerCl, err := dlib.NewClientFromEnv()
Copy link
Member

@vdice vdice Jun 20, 2016

Choose a reason for hiding this comment

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

Will this work with Docker For Mac? (probably not as it's looking for the docker-machine env vars, right?)

We may consider explicitly mentioning the need for docker-machine now that multiple Mac options are available...

Copy link
Member Author

Choose a reason for hiding this comment

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

This should. The code for this func uses the NewVersionedClientFromEnv function under the covers, which in turn uses getDockerEnv(), which knows what to do if the DOCKER_HOST and related env vars are missing (as they would be in the case of docker for mac). I've tested the same command as above (./deisrel docker tag v2.0.2 --prompt-tags=false --push=false --force-retag=true) against a machine running Docker for Mac and saw it successfully connect to the docker daemon

@vdice
Copy link
Member

vdice commented Jun 20, 2016

@arschles pending a couple questions, this looks great! I understand unit tests are in progress, so let me know when I can revisit to review.

@arschles
Copy link
Member Author

Latest command & debug output:

ENG000656:deisrel aaronschlesinger$ ./deisrel docker retag v2.0.1 --yes=false
Re-tagging images...
tagging quay.io/deisci/postgres:git-ebd3e1c to quay.io/deisci/postgres:v2.0.1
tagging quay.io/deisci/builder:git-938cdf2 to quay.io/deisci/builder:v2.0.1
tagging quay.io/deisci/fluentd:git-2ab194a to quay.io/deisci/fluentd:v2.0.1
tagging quay.io/deisci/slugrunner:git-9d16ae7 to quay.io/deisci/slugrunner:v2.0.1
tagging quay.io/deisci/registry:git-7a0fb97 to quay.io/deisci/registry:v2.0.1
tagging quay.io/deisci/router:git-eb6b05c to quay.io/deisci/router:v2.0.1
tagging quay.io/deisci/stdout-metrics:git-c37e3ec to quay.io/deisci/stdout-metrics:v2.0.1
tagging quay.io/deisci/logger:git-a9934d4 to quay.io/deisci/logger:v2.0.1
tagging quay.io/deisci/workflow-e2e:git-f21e0a8 to quay.io/deisci/workflow-e2e:v2.0.1
tagging quay.io/deisci/controller:git-0e6473b to quay.io/deisci/controller:v2.0.1
tagging quay.io/deisci/dockerbuilder:git-bce1fd7 to quay.io/deisci/dockerbuilder:v2.0.1
tagging quay.io/deisci/monitor:git-99028fa to quay.io/deisci/monitor:v2.0.1
tagging quay.io/deisci/workflow-manager:git-6e9100c to quay.io/deisci/workflow-manager:v2.0.1
tagging quay.io/deisci/minio:git-958ccd7 to quay.io/deisci/minio:v2.0.1
tagging quay.io/deisci/slugbuilder:git-e95687e to quay.io/deisci/slugbuilder:v2.0.1
pulling quay.io/deisci/monitor:git-99028fa
2016/06/20 11:46:12 Failed to re-tag every image. See errors below
2016/06/20 11:46:12 tagging image quay.io/deisci/workflow-e2e:git-f21e0a8 to new tag v2.0.1 (API error (500): {"message":"invalid reference format"})
2016/06/20 11:46:12 tagging image quay.io/deisci/logger:git-a9934d4 to new tag v2.0.1 (API error (500): {"message":"invalid reference format"})
2016/06/20 11:46:12 tagging image quay.io/deisci/slugrunner:git-9d16ae7 to new tag v2.0.1 (API error (500): {"message":"invalid reference format"})
2016/06/20 11:46:12 tagging image quay.io/deisci/dockerbuilder:git-bce1fd7 to new tag v2.0.1 (API error (500): {"message":"invalid reference format"})
2016/06/20 11:46:12 tagging image quay.io/deisci/stdout-metrics:git-c37e3ec to new tag v2.0.1 (API error (500): {"message":"invalid reference format"})
2016/06/20 11:46:12 tagging image quay.io/deisci/router:git-eb6b05c to new tag v2.0.1 (API error (500): {"message":"invalid reference format"})
2016/06/20 11:46:12 tagging image quay.io/deisci/fluentd:git-2ab194a to new tag v2.0.1 (API error (500): {"message":"invalid reference format"})
2016/06/20 11:46:12 tagging image quay.io/deisci/workflow-manager:git-6e9100c to new tag v2.0.1 (API error (500): {"message":"invalid reference format"})
2016/06/20 11:46:12 tagging image quay.io/deisci/registry:git-7a0fb97 to new tag v2.0.1 (API error (500): {"message":"invalid reference format"})
2016/06/20 11:46:12 tagging image quay.io/deisci/builder:git-938cdf2 to new tag v2.0.1 (API error (500): {"message":"invalid reference format"})
2016/06/20 11:46:12 tagging image quay.io/deisci/postgres:git-ebd3e1c to new tag v2.0.1 (API error (500): {"message":"invalid reference format"})
2016/06/20 11:46:12 tagging image quay.io/deisci/slugbuilder:git-e95687e to new tag v2.0.1 (API error (500): {"message":"invalid reference format"})
2016/06/20 11:46:12 tagging image quay.io/deisci/controller:git-0e6473b to new tag v2.0.1 (API error (500): {"message":"invalid reference format"})
2016/06/20 11:46:12 tagging image quay.io/deisci/minio:git-958ccd7 to new tag v2.0.1 (API error (500): {"message":"invalid reference format"})
2016/06/20 11:46:12 pulling image quay.io/deisci/monitor:git-99028fa (Tag git-99028fa not found in repository quay.io/deisci/monitor)

Notice the changed flags. Also, the latest version of the Docker client library isn't compatible with Docker 1.12 😢

@vdice
Copy link
Member

vdice commented Jun 20, 2016

@arschles:

tagging quay.io/deisci/postgres:git-ebd3e1c to quay.io/deisci/postgres:v2.0.1

I believe we want the retagged "pretty" img to be under the deis org as opposed to deisci

@arschles
Copy link
Member Author

@vdice good call - thanks! Fixed in 53c98b8

@arschles
Copy link
Member Author

re-labelling as in progress again because of docker client incompatibilities with docker daemon 1.12

Name: "retag",
Description: "This command pulls specific Docker images for each corresponding repository's Git SHA, then retags each one to a uniform release tag",
Usage: "Retag each specific Docker image from a repo-specific Git SHA to a uniform release tag",
Flags: []cli.Flag{
Copy link
Member

@vdice vdice Jun 20, 2016

Choose a reason for hiding this comment

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

@arschles can we also allow use of the --ref <ref> flag for this command so that the shas can be pulled from HEAD of a branch other than master? addressed in latest changeset.


// ParseImageFromRepoAndSha attempts to convert ras into a docker image, using dockerRegistryOrg as the docker registry
func ParseImageFromRepoAndSha(dockerRegistryOrg string, ras git.RepoAndSha) (*Image, error) {
str := fmt.Sprintf("quay.io/%s/%s:git-%s", dockerRegistryOrg, ras.Name, ras.ShortSHA())
Copy link
Member

@vdice vdice Jun 28, 2016

Choose a reason for hiding this comment

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

Does hard-coding quay.io prevent us from retagging/pushing release images to the DockerHub registry? It doesn't appear that we're set up to push release images to both quay.io and DockerHub registries, but please correct me if I'm wrong. Otherwise, definitely okay with covering in follow-up ticket.

Copy link
Member Author

Choose a reason for hiding this comment

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

ah, yes it certainly does! are you cool with me covering it immediately after merging this PR? if so, I'll create a ticket for that work

return nil, ErrListTags{Img: img, Err: err}
}
return ret, nil
}
Copy link
Member

Choose a reason for hiding this comment

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

Is this file being used anymore?

Copy link
Member Author

Choose a reason for hiding this comment

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

@vdice no, good catch. I'll remove


func (s *sdkClient) Retag(src *Image, tar *Image) error {
return errNotYetImplemented
}
Copy link
Member

Choose a reason for hiding this comment

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

Nice, a placeholder to revisit the sdk client when/if it gets updated to work w/ latest docker client? If so, wondering if we should create a specific ticket in the backlog for this?

Copy link
Member Author

Choose a reason for hiding this comment

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

@vdice yea, it's a placeholder that I'd love to use but, as you said, it's incompatible with 1.12 afaict. Created #108 to track

@vdice
Copy link
Member

vdice commented Jun 28, 2016

@arschles besides a couple of non-blocking questions, code LGTM. Also, a reminder to create the ticket for pushing release images to the DockerHub registry (in addition to pushing to Quay.io as this PR adds), so we can prioritize that next...

@arschles
Copy link
Member Author

thx @vdice - created #107 to track pushing to dockerhub

"strings"
)

// GetShasFromFilepath opens the file at path and reads all the git repos and SHAs in the file. Returns a slice of all the RepoAndShas that correspond to each entry in the file, or an empty slice and non-nil error if there was any error
Copy link
Member

Choose a reason for hiding this comment

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

Could you format the docstrings at 99 chars / line or less?

Copy link
Member Author

Choose a reason for hiding this comment

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

@mboersma fixed in 79bf288

@bacongobbler
Copy link
Member

bacongobbler commented Jun 29, 2016

barring the one comment from @vdice in deis/workflow#355 (comment) this LGTM leaving it to @mboersma to LGTM

return &sdkClient{cl: cl}
}

func (s *sdkClient) Pull(img *Image) error {
Copy link
Member

Choose a reason for hiding this comment

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

Add documentation strings for these exported funcs?

Or make that a TODO later if this needs to merge now--deisrel is essentially an internal tool. 🙈

Copy link
Member Author

Choose a reason for hiding this comment

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

@mboersma good catch - thanks. Fixed in f89ec9a

@mboersma
Copy link
Member

Code looks good, and I just had a couple nitpicky comments. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants