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

Podman support #995

Merged
merged 2 commits into from
Aug 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions start-alertmanager.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ if [ "$1" = "-e" ]; then
else
. versions.sh
fi
is_podman="$(docker --help | grep -o podman)"
VERSIONS=$DEFAULT_VERSION
RULE_FILE=$PWD/prometheus/rule_config.yml
ALERT_MANAGER_VERSION="v0.20.0"
Expand Down Expand Up @@ -85,4 +86,8 @@ then
fi

AM_ADDRESS="$(docker inspect --format '{{ .NetworkSettings.IPAddress }}' $ALERTMANAGER_NAME):9093"
if [ ! -z "$is_podman" ] && [ "$AM_ADDRESS" = ":9093" ]; then
HOST_IP=`hostname -I | awk '{print $1}'`
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, I don't understand the meaning of taking the first response in "hostname -I", which is a list of IP address whose order is undefined.

AM_ADDRESS="$HOST_IP:9093"
fi
echo $AM_ADDRESS
13 changes: 13 additions & 0 deletions start-all.sh
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@ else
GROUPID=`id -g`
USER_PERMISSIONS="-u $UID:$GROUPID"
fi

group_args=()
is_podman="$(docker --help | grep -o podman)"
if [ ! -z "$is_podman" ]; then
group_args+=(--userns=keep-id)
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I'm happy to report that this change (plus the one line below using group_args) is what solves the problem I had with the "-d" option, where the container didn't have permision to actual use the directory I gave it because it belonged to my user. Please split this as a separate patch, explain why it's needed, and then I'll commit this.
But, to be honest, while this works, I don't understand what that "group-add" is supposed to be doing... Isn't "--userns=keep-id" all the magic we need? What is this group-add thing doing?



PROMETHEUS_RULES="$PWD/prometheus/prometheus.rules.yml"
VERSIONS=$DEFAULT_VERSION
usage="$(basename "$0") [-h] [--version] [-e] [-d Prometheus data-dir] [-L resolve the servers from the manger running on the given address] [-G path to grafana data-dir] [-s scylla-target-file] [-n node-target-file] [-l] [-v comma separated versions] [-j additional dashboard to load to Grafana, multiple params are supported] [-c grafana environment variable, multiple params are supported] [-b Prometheus command line options] [-g grafana port ] [ -p prometheus port ] [-a admin password] [-m alertmanager port] [ -M scylla-manager version ] [-D encapsulate docker param] [-r alert-manager-config] [-R prometheus-alert-file] [-N manager target file] [-A bind-to-ip-address] [-C alertmanager commands] [-Q Grafana anonymous role (Admin/Editor/Viewer)] [-S start with a system specific dashboard set] -- starts Grafana and Prometheus Docker instances"
Expand Down Expand Up @@ -223,6 +231,7 @@ fi

docker run -d $DOCKER_PARAM $USER_PERMISSIONS \
$DATA_DIR \
"${group_args[@]}" \
-v $PWD/prometheus/build/prometheus.yml:/etc/prometheus/prometheus.yml:Z \
-v $PROMETHEUS_RULES:/etc/prometheus/prometheus.rules.yml:Z \
$SCYLLA_TARGET_FILE \
Expand Down Expand Up @@ -266,6 +275,10 @@ fi
# Can't use localhost here, because the monitoring may be running remotely.
# Also note that the port to which we need to connect is 9090, regardless of which port we bind to at localhost.
DB_ADDRESS="$(docker inspect --format '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}' $PROMETHEUS_NAME):9090"
if [ ! -z "$is_podman" ] && [ "$DB_ADDRESS" = ":9090" ]; then
HOST_IP=`hostname -I | awk '{print $1}'`
DB_ADDRESS="$HOST_IP:9090"
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this change fix? It needs to be explained either in the code as a comment, or in a commit message.
Also, what does "hostname -I | awk '{print $1'}" even trying to do? The hostname manual page clearly says: "Do not make any assumptions about the order of the output" - so what's the meaning of taking the first one?


for val in "${GRAFANA_ENV_ARRAY[@]}"; do
GRAFANA_ENV_COMMAND="$GRAFANA_ENV_COMMAND -c $val"
Expand Down
7 changes: 7 additions & 0 deletions start-grafana.sh
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,12 @@ if [ $? -eq 0 ]; then
exit 1
fi

group_args=()
is_podman="$(docker --help | grep -o podman)"
if [ ! -z "$is_podman" ]; then
group_args+=(--userns=keep-id)
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

start-grafana.sh worked well for me even without this patch. When is this change needed? Perhaps for the "-G" option to work? Or something else?
And again I understand the userns thing, but not the purpose of the groups thing.


if [ "`id -u`" -ne 0 ]; then
GROUPID=`id -g`
USER_PERMISSIONS="-u $UID:$GROUPID"
Expand Down Expand Up @@ -127,6 +133,7 @@ docker run -d $DOCKER_PARAM -i $USER_PERMISSIONS $PORT_MAPPING \
-e "GF_AUTH_ANONYMOUS_ENABLED=$GRAFANA_AUTH_ANONYMOUS" \
-e "GF_AUTH_ANONYMOUS_ORG_ROLE=$ANONYMOUS_ROLE" \
-e "GF_PANELS_DISABLE_SANITIZE_HTML=true" \
"${group_args[@]}" \
-v $PWD/grafana/build:/var/lib/grafana/dashboards:z \
-v $PWD/grafana/plugins:/var/lib/grafana/plugins:z \
-v $PWD/grafana/provisioning:/var/lib/grafana/provisioning:z $EXTERNAL_VOLUME \
Expand Down