Add initial config.yml #47
Conversation
Just basic config, used for WIP testing
|
From the resulting config I must admit that placing every new Docker version in a dir doesn't look like ideal way for a long term. There are better ways to do this:
|
|
@armab Thanks for your feedback. I agree, current WIP is not ideal long term. To date, I've been using this PR as a playground. Caching works really well - builds on CircleCI are at least 8x faster than on Docker Hub. Additionally, we're not currently building the other images (stackstorm-1ppc, stackstorm-all) when there's a new ST2 version. IMHO, we must handle this case and the ones you've mentioned before I'd consider this PR ready for final review. |
|
Remaining Tasks:
|
$latest is prefixed with v, but $tag is not. Perform the correct comparison.
|
Please include the build badge in |
.circleci/config.yml
Outdated
| name: Install bash | ||
| command: apk add --no-cache bash | ||
| - deploy: | ||
| name: Build and Push image to Docker Hub |
There was a problem hiding this comment.
Ideally is to divide that single Build & Push into 2 steps: build & push where the build happens as "normal" CircleCI step, while push happens on deploy CircleCI step.
This way it will be more clear what's going on in CircleCI: when the CI just builds an image and when it's actually uploading it to the Hub.
bin/build-deploy.sh
Outdated
| exit 1 | ||
| fi | ||
|
|
||
| docker login --username ${DOCKER_USER} --password ${DOCKER_PASSWORD} |
There was a problem hiding this comment.
This line will fail the build in some cases.
Secret env vars set in CircleCI DOCKER_USER and DOCKER_PASSWORD are not available for forked PRs, eg. cases with external contributions (and we want those contributions).
This perfectly works with the previous message when build & deploy steps are separated.
So docker login + docker push should be part of the deployment step, which is triggered only on post-merge or new git tag (GitHub release), while we also build the image to verify Dockerfile corectness on every PR, where docker login is not needed.
images/stackstorm/bin/install.sh
Outdated
|
|
||
| MANIFEST="/st2-manifest.txt" | ||
|
|
||
| echo "Image built at $(date) using st2-docker:${ST2_DOCKER_SHA1}" > $MANIFEST |
There was a problem hiding this comment.
st2-docker:${ST2_DOCKER_SHA1}
Can we use the full GitHub link with the SHA1 instead? Could be easier to copy-paste.
At the same time, would be good to include CircleCI build URL as well, like we do in st2 repo.
This way user has all the information: where/how the build happened and which commit it refers to.
There was a problem hiding this comment.
Agreed. Good idea!
.circleci/config.yml
Outdated
| name: Deploy image to Docker Hub | ||
| branches: | ||
| only: | ||
| - feature/circleci |
There was a problem hiding this comment.
Seems master branch is needed as well.
Also will this work when creating a git tag (GitHub release)?
There was a problem hiding this comment.
I plan to replace feature/circleci with master before merging and deleting feature/circleci branch.
I've yet to confirm adding a git tag (GitHub release). I'd hoped to do this already, but I'm blocked by CircleCI infra issue. I'll try again tonight. I thought it was a recent commit that broke it, but rebuild of previously successful builds are failing too...
Change the logic that writes /st2-manifest.txt
branches:
only:
- master
Even with the above, it deployed with CIRCLE_BRANCH=feature/circleci
Enable builds using CircleCI.
Just basic config, used for WIP testing