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.

Sign up for GitHub

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

Jump to bottom

Adds a docker-compose.test.yml to do a health check of built container. #2808

Merged
merged 6 commits into from
Sep 19, 2019
Merged

Adds a docker-compose.test.yml to do a health check of built container. #2808

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e4d429f
Add docker test and health check.
anuraaga Sep 18, 2019
0f2216c
pre_build instead of build since it sets up builds.
anuraaga Sep 18, 2019
1595070
Merge pull request #1 from anuraaga/docker-test
anuraaga Sep 18, 2019
433330c
Update docker-compose.test.yml
anuraaga Sep 18, 2019
cb7cdca
Update docker-compose.test.yml
anuraaga Sep 18, 2019
9199199
Document and tighten health check
anuraaga Sep 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .dockerignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
**/\.*
!.git
!**/.eslintrc
**/*.test.yml
Copy link
Member

Choose a reason for hiding this comment

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

fuzz?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is since building a docker image doesn't require the compsoe file in the build context.


**/target
**/node_modules
Expand Down
2 changes: 2 additions & 0 deletions docker/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,6 @@ USER zipkin

EXPOSE 9410 9411

HEALTHCHECK --interval=10s --start-period=30s --timeout=5s CMD wget --quiet http://localhost:9411/health || exit 1
Copy link
Member

Choose a reason for hiding this comment

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

Cool, I didn't realize we were using the debug image

Copy link
Member

Choose a reason for hiding this comment

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

does this imply any blocking? the start period is really long

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't block, it probes every interval regardless - went ahead and lowered interval to 5s. Any failures during the start-period aren't counted as "unhealthy", they're just skipped, but they still happen.

Copy link
Member

Choose a reason for hiding this comment

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

@devinsba I didn't notice the debug image until now also. In fact my hotel can't even pull the zipkin image it is so big. I opened this up for follow-up in case there's a way to reduce size again openzipkin-attic/docker-zipkin#226

Copy link
Member

Choose a reason for hiding this comment

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

cc also @jcarres-mdsol in case you are using health checking in your deployment. This isn't the release dockerfile, but I suspect it will soon converge.


ENTRYPOINT ["/busybox/sh", "run.sh"]
30 changes: 30 additions & 0 deletions docker/docker-compose.test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#
# Copyright 2015-2019 The OpenZipkin Authors
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
# in compliance with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software distributed under the License
# is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
# or implied. See the License for the specific language governing permissions and limitations under
# the License.
#
version: "3.6"
services:
zipkin:
build:
context: ..
dockerfile: docker/Dockerfile
networks:
default:
aliases:
- zipkin
sut:
Copy link
Member

Choose a reason for hiding this comment

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

perhaps healthchecker would be a better name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That name is hard coded as the name that docker hub checks for the test status - added a link to the docs so it's easier to follow.

Copy link
Member

Choose a reason for hiding this comment

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

👍 seems like an odd name but works for me

image: appropriate/curl
networks:
- default
command: -v --connect-timeout 5 --retry 30 --retry-connrefused --retry-delay 5 http://zipkin:9411/health
depends_on:
- zipkin
2 changes: 0 additions & 2 deletions hooks/build → hooks/pre_build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,3 @@ case "$DOCKER_REPO" in
*)
;;
esac

docker build -f $DOCKERFILE_PATH -t $IMAGE_NAME .