Skip to content
This repository was archived by the owner on Aug 23, 2023. It is now read-only.

debug build of metrictank and debug docker/k8s images #1187

Merged
merged 7 commits into from
Apr 10, 2019
Merged

Conversation

robert-milan
Copy link
Contributor

add docker-dev-debug for running local metrictank with dlv in container
add -debug flag for use with build scripts
add Dockerfiles for local and kubernetes versions
add .vscode to .gitignore
add metrictank-debug image for public and private repositories

@Dieterbe
Copy link
Contributor

need to update docs/docker.md

COPY config/schema-store-cassandra.toml /etc/metrictank/schema-store-cassandra.toml
COPY config/schema-idx-cassandra.toml /etc/metrictank/schema-idx-cassandra.toml
COPY config/schema-store-scylladb.toml /usr/share/metrictank/examples/schema-store-scylladb.toml
COPY config/schema-idx-scylladb.toml /usr/share/metrictank/examples/schema-idx-scylladb.toml
Copy link
Contributor

Choose a reason for hiding this comment

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

can't we do this without repeating all the stuff we did for the regular image?
maybe use FROM metrictank ? (obviously, requires that image to be built before building this one)

}

trap 'kill_metrictank' SIGTERM
trap 'kill_metrictank' SIGINT
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need all this stuff because when terminating delve, it doesn't terminate metrictank?
you need to describe the problem and the solution in a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it doesn't properly terminate Metrictank and then the container hangs. I will document it in a comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

it's unfortunate, all the complexity we have to add to deal with this..
can we solve this in a more clean way, e.g. add support to delve to properly kill its child process when it is terminated? or is it a design goal to keep the child alive?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm looking into other possible options or flags to use that might accomplish this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't been able to find another way to get this done.

@Dieterbe
Copy link
Contributor

please keep commit messages more highlevel. there's not much value to them if they are as detailed and verbose as the actual diff is.
also https://chris.beams.io/posts/git-commit/#separate

@robert-milan robert-milan changed the title [WIP] add build for debug version of metrictank add build for debug version of metrictank Feb 15, 2019
@robert-milan robert-milan requested a review from replay February 15, 2019 15:35
@Dieterbe
Copy link
Contributor

@robert-milan is this ready to merge in your opinion?

@robert-milan
Copy link
Contributor Author

@robert-milan is this ready to merge in your opinion?

I believe so, I would like to have a day or two more just to look over it again.

robert-milan and others added 7 commits April 10, 2019 14:45
add docker-dev-debug for running local metrictank with dlv in container
add -debug flag for use with build scripts
add Dockerfiles for local and kubernetes versions
add metrictank-debug image for public and private repositories
update Makefile to include qa for debug builds
remove metrictank-debug versions of docker images
add debug tags to existing metrictank docker images to differentiate them from normal builds
add debug after version name of metrictank
remove exec command from startup scripts in order to trap signals and gracefully exit metrictank and dlv
minor diff with stock docker config, may as well use it.
Copy link
Contributor

@Dieterbe Dieterbe left a comment

Choose a reason for hiding this comment

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

LGTM

@Dieterbe Dieterbe changed the title add build for debug version of metrictank debug build of metrictank and debug docker/k8s images Apr 10, 2019
@Dieterbe Dieterbe merged commit 9871607 into master Apr 10, 2019
@robert-milan robert-milan deleted the remote-debug branch June 22, 2019 16:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants