Skip to content

Conversation

@jebentier
Copy link
Contributor

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming langauge.

Description of the PR

Added a docker build tag to the circle.yml file so that when the images are pushed, a version tag is pushed as well at latest.

fix #6728

@jebentier jebentier force-pushed the push_image_build_tag branch from 2748ae7 to b075542 Compare October 27, 2017 20:58
environment:
DOCKER_GENERATOR_IMAGE_NAME: swaggerapi/swagger-generator
DOCKER_CODEGEN_CLI_IMAGE_NAME: swaggerapi/swagger-codegen-cli
DOCKER_BUILD_TAG: 2.2.3
Copy link
Contributor

Choose a reason for hiding this comment

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

@jebentier thanks for the enhancement. Should this be 2.3.0-SNAPSHOT instead? (the latest master is 2.3.0)

Copy link
Contributor

Choose a reason for hiding this comment

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

Typically the image tags should match the git tags. To make it dynamic using a git command like such:

tag=$(git tag -l --points-at HEAD) ; if [[ -z "$tag" ]]; then echo "latest" ; else echo "$tag" ; fi

If there is no tag on the current commit, you get back "latest" otherwise you get the tag which would have been v2.2.3 if that commit was the one being built.

Copy link
Contributor

Choose a reason for hiding this comment

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

This could be done more easily in Docker Hub as an automated build. Probably a silly question, but @wing328 do you know if there's a reason it's set up in CI rather than in Docker Hub?

Copy link
Contributor

Choose a reason for hiding this comment

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

@jimschubert sorry I don't have an answer as it's setup by Tony.

cc @fehguy

Copy link
Contributor

Choose a reason for hiding this comment

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

@kenjones-cisco thanks for the explanation. If it's within Docker user's expectation that 2.2.3 can refer to the Docker image built by the latest master (2.3.0), then it's totally fine with me.

@ffledgling
Copy link

Bump, I'd like to see this merged. What needs to be modified to get this PR merged (or have tags on the docker images a different way)? Looks like at the least the tag needs to updated to 2.3.0 (but that seems incorrect-ish?) or alternatively the tag needs to be auto-extracted from git. @wing328 ?

@wing328
Copy link
Contributor

wing328 commented Nov 9, 2017

@ffledgling not that I don't want to merge but I'm busy with other tasks at the moment.

I'll take a deeper look into tags on docker image this coming weekend and merge this PR if I don't have any more question.

@wing328 wing328 added this to the v2.3.0 milestone Nov 9, 2017
@wing328 wing328 merged commit 4e482ee into swagger-api:master Nov 15, 2017
@wing328
Copy link
Contributor

wing328 commented Nov 15, 2017

CircleCI reported the following errors:

Successfully built 123b4eebb286
The push refers to a repository [docker.io/swaggerapi/swagger-codegen-cli] (len: 1)
error getting tags for swaggerapi/swagger-codegen-cli: Tag does not exist for 2.2.3

Ref: https://circleci.com/gh/swagger-api/swagger-codegen/2610

Anyone has any clue about the error?

We will need to roll back the change if we can resolve this.

@jimschubert
Copy link
Contributor

master should be latest, while tags should be triggered based on git ref. As I mentioned before, this would be way easier in Docker Hub. To do via CI, you'd need to build from a release branch unless Travis allows building tags.

@kenjones-cisco
Copy link
Contributor

The error is because the docker tag is using CIRCLE_TAG, but the docker push is using the new environment variable DOCKER_BUILD_TAG. Not sure where or if CIRCLE_TAG is ever set.

To do this via CI, you will need a script that determines if the current commit being built has a tag, if no tag then it is latest which is what most commits to master will be. When a release is tagged, then the commit should have a tag associated it to, and then a tagged version of the image is pushed.

@wing328
Copy link
Contributor

wing328 commented Nov 15, 2017

OK. I'll have another look tomorrow.

wing328 added a commit that referenced this pull request Nov 16, 2017
wing328 added a commit that referenced this pull request Nov 16, 2017
* Revert "[csharp] clean boolean additional properties 6784 (#6899)"

This reverts commit 2c9f98c.

* Revert "[Swift4] Add throw to reserved words (#6952)"

This reverts commit 970de01.

* Revert "add a docker build tag for pushing docker image instead of just latest (#6837)"

This reverts commit 4e482ee.
@wing328
Copy link
Contributor

wing328 commented Nov 20, 2017

I've rolled back this change.

I think what we need to do is to tag the docker image in the next release (in Docker Hub).

@ffledgling
Copy link

So is the final resolution on this that new releases will have manually added version number in the docker releases? If that's the case, it looks like it did not happen for v2.3.0.

@wing328
Copy link
Contributor

wing328 commented Jan 11, 2018

@ffledgling yup, and you're right there's no tagging for v2.3.0. I do not have right to do it in dockerhub.

@webron is very busy these days and we'll do it for v2.3.1 instead, which will be released very soon.

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.

[docker] swagger-codegen-cli docker image - use version tags

5 participants