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

cqfd: add support for the 'docker' option #157

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

cqfd: add support for the 'docker' option #157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,12 @@ For example, to attempt to pull newer version of the image, it can be set like:
docker_build_args='--pull=true'
```

``docker`` (optional): program used to invoke `docker` client.
Copy link
Member

Choose a reason for hiding this comment

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

You could add a test for this feature. It could pass a dummy command as docker, and check that it was executed.

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 wonder if I should also add CQFD_DOCKER variable.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds great, maybe even better than a docker option? We can expect users to have a custom docker installation, so they want to pass it through the variable. Passing it through the configuration file doesn't make a whole lot of sense with the many extra_run/build_args options we already have? A configuration would be useful if we want to distinguish which command to use per flavors or project, but it doesn't seem to be the use case you are addressing.

Copy link
Member

Choose a reason for hiding this comment

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

Since the use case rather depends on the user's docker installation than the cqfd project, I think it would be best if this feature was an environment variable rather than in the project file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fact, I like both: environment CQFD_DOCKER and property docker=.

Also, prepending sudo to the docker command could be easy with that change:

diff --git a/cqfd b/cqfd
index 6d4a4ab..9748afd 100755
--- a/cqfd
+++ b/cqfd
@@ -27,6 +27,7 @@ cqfd_user='builder'
 cqfd_user_home='/home/builder'
 cqfd_user_cwd="$cqfd_user_home/src"
 cqfd_shell=${CQFD_SHELL:-/bin/bash}
+cqfd_docker=('docker')
 cqfd_docker_gid="${CQFD_DOCKER_GID:-0}"
 
 ## usage() - print usage on stdout
@@ -166,14 +167,14 @@ docker_build() {
 		args+=("$project_build_context" -f "$dockerfile")
 	fi
 
-	debug executing: docker build "${args[@]}"
-	docker build "${args[@]}"
+	debug executing: "${cqfd_docker[@]}" build "${args[@]}"
+	"${cqfd_docker[@]}" build "${args[@]}"
 }
 
 # image_exists_locally(): checks if image exists in the local image store
 # arg$1: the image name to check
 image_exists_locally() {
-	docker image inspect "$1" &> /dev/null
+	"${cqfd_docker[@]}" image inspect "$1" &> /dev/null
 }
 
 # docker_run() - run command in configured container
@@ -197,7 +198,7 @@ docker_run() {
 	if ! image_exists_locally "$docker_img_name"; then
 		# If custom image name is used, try to pull it before dying
 		if [ "$project_custom_img_name" ]; then
-			if ! docker pull "$docker_img_name" >& /dev/null; then
+			if ! "${cqfd_docker[@]}" pull "$docker_img_name" >& /dev/null; then
 				die "Custom image couldn't be pulled, please build/upload it first"
 			fi
 		else
@@ -255,6 +256,11 @@ docker_run() {
 		fi
 	fi
 
+	# Use sudo if user is not in the docker group
+	if [ "$cqfd_docker_gid" -eq 0 ]; then
+		cqfd_docker=(sudo "${cqfd_docker[@]}")
+	fi
+
 	# Display a warning message if using no more supported options
 	if [ -n "$CQFD_EXTRA_VOLUMES" ]; then
 		die 'Warning: CQFD_EXTRA_VOLUMES is no more supported, use
@@ -332,8 +338,8 @@ docker_run() {
 
 	args+=("$docker_img_name" cqfd_launch "$1")
 
-	debug executing: docker run "${args[@]}"
-	docker run "${args[@]}"
+	debug executing: "${cqfd_docker[@]}" run "${args[@]}"
+	"${cqfd_docker[@]}" run "${args[@]}"
 }
 
 # make_archive(): Create a release package.

For example, to use docker if not in the docker group, it can be set like:
```
docker='sudo docker'
```

``docker_run_args`` (optional): arguments used to invoke `docker run`.
For example, to share networking with the host, it can be set like:
```
Expand Down
17 changes: 11 additions & 6 deletions cqfd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ cqfd_user='builder'
cqfd_user_home='/home/builder'
cqfd_user_cwd="$cqfd_user_home/src"
cqfd_shell=${CQFD_SHELL:-/bin/bash}
cqfd_docker="${CQFD_DOCKER:-docker}"

## usage() - print usage on stdout
usage() {
Expand Down Expand Up @@ -165,14 +166,14 @@ docker_build() {
args+=("$project_build_context" -f "$dockerfile")
fi

debug executing: docker build "${args[@]}"
docker build "${args[@]}"
debug executing: "${cqfd_docker[@]:-docker}" build "${args[@]}"
"${cqfd_docker[@]:-docker}" build "${args[@]}"
}

# image_exists_locally(): checks if image exists in the local image store
# arg$1: the image name to check
image_exists_locally() {
docker image inspect "$1" &> /dev/null
"${cqfd_docker[@]:-docker}" image inspect "$1" &> /dev/null
}

# docker_run() - run command in configured container
Expand All @@ -196,7 +197,7 @@ docker_run() {
if ! image_exists_locally "$docker_img_name"; then
# If custom image name is used, try to pull it before dying
if [ "$project_custom_img_name" ]; then
if ! docker pull "$docker_img_name" >& /dev/null; then
if ! "${cqfd_docker[@]:-docker}" pull "$docker_img_name" >& /dev/null; then
die "Custom image couldn't be pulled, please build/upload it first"
fi
else
Expand Down Expand Up @@ -313,8 +314,8 @@ docker_run() {

args+=("$docker_img_name" cqfd_launch "$1")

debug executing: docker run "${args[@]}"
docker run "${args[@]}"
debug executing: "${cqfd_docker[@]:-docker}" run "${args[@]}"
"${cqfd_docker[@]:-docker}" run "${args[@]}"
}

# make_archive(): Create a release package.
Expand Down Expand Up @@ -560,6 +561,10 @@ config_load() {
# shellcheck disable=SC2154
release_tar_opts="$tar_options"

if [ "$docker" ]; then
# shellcheck disable=SC2162
read -a cqfd_docker <<<"$docker"
fi
dockerfile="${cqfd_project_dir}/${cqfddir}/${distro:-docker}/Dockerfile"

if [ -z "$project_org" ] || [ -z "$project_name" ]; then
Expand Down
Loading