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

[bitnami/discourse] Use unicorn instead of passenger #2355

Closed
wants to merge 1 commit into from
Closed

[bitnami/discourse] Use unicorn instead of passenger #2355

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
2 changes: 1 addition & 1 deletion bitnami/discourse/2/debian-11/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ RUN . /opt/bitnami/scripts/libcomponent.sh && component_unpack "node" "14.20.0-1
RUN . /opt/bitnami/scripts/libcomponent.sh && component_unpack "git" "2.37.1-1" --checksum 39ea3040baa552b4760c1100a3713f86493620c0a74121c1ceba50fe17341878
RUN . /opt/bitnami/scripts/libcomponent.sh && component_unpack "brotli" "1.0.9-151" --checksum ba4c4d26d232f8d91d1143078a633ba7c5ae33f5c4182684400ffb7bbc0ed262
RUN . /opt/bitnami/scripts/libcomponent.sh && component_unpack "discourse" "2.8.7-1" --checksum bd120eab1a7403e691f608f57b574a9782391fe1c4a46ce40752cfa560f922c2
RUN apt-get update && apt-get upgrade -y && \
RUN apt-get update && apt-get upgrade -y && apt-get install sudo runit -y && \
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer not installing runit in a container unless it is strictly necessary.

I have performed some tests using the changes suggested in this review and I was able to use unicorn without runit.

Copy link
Author

Choose a reason for hiding this comment

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

if you change something that supposedly is working, then it would be nice if you just attach it. I don't have time to test a foreign theory. When we tested it didn't work without runit to run as non-root

rm -r /var/lib/apt/lists /var/cache/apt/archives
RUN chmod g+rwX /opt/bitnami
RUN /opt/bitnami/ruby/bin/gem install --force bundler -v '< 2'
Expand Down
3 changes: 3 additions & 0 deletions bitnami/discourse/2/debian-11/rootfs/opt/bitnami/scripts/discourse-env.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export BITNAMI_DEBUG="${BITNAMI_DEBUG:-false}"
# By setting an environment variable matching *_FILE to a file path, the prefixed environment
# variable will be overridden with the value specified in that file
discourse_env_vars=(
DISCOURSE_SERVE_STATIC_ASSETS
DISCOURSE_DATA_TO_PERSIST
DISCOURSE_ENABLE_HTTPS
DISCOURSE_EXTERNAL_HTTP_PORT_NUMBER
Expand Down Expand Up @@ -92,6 +93,8 @@ export DISCOURSE_CONF_FILE="${DISCOURSE_BASE_DIR}/config/discourse.conf"
export PATH="${BITNAMI_ROOT_DIR}/common/bin:${BITNAMI_ROOT_DIR}/brotli/bin:${BITNAMI_ROOT_DIR}/git/bin:${PATH}"

# Discourse persistence configuration
DISCOURSE_SERVE_STATIC_ASSETS="${DISCOURSE_SERVE_STATIC_ASSETS:-true}"
export DISCOURSE_SERVE_STATIC_ASSETS="${DISCOURSE_SERVE_STATIC_ASSETS:-true}"
Comment on lines +96 to +97
Copy link
Member

Choose a reason for hiding this comment

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

Please move these lines under the # Discourse configuration comment

export DISCOURSE_VOLUME_DIR="${BITNAMI_VOLUME_DIR}/discourse"
export DISCOURSE_DATA_TO_PERSIST="${DISCOURSE_DATA_TO_PERSIST:-plugins public/backups public/uploads}"

Copy link
Member

Choose a reason for hiding this comment

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

Unicorn will fail to start if we use the current default value for DISCOURSE_PORT_NUMBER, please change the value from 3000 to 8080.

export DISCOURSE_PORT_NUMBER="${DISCOURSE_PORT_NUMBER:-8080}"

In addition, please replace the ports value at docker-compose.yaml.

Expand Down
9 changes: 4 additions & 5 deletions bitnami/discourse/2/debian-11/rootfs/opt/bitnami/scripts/discourse/run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,10 @@ set -o pipefail
cd "$DISCOURSE_BASE_DIR"

declare -a cmd=(
"bundle" "exec" "passenger" "start"
"--user" "$DISCOURSE_DAEMON_USER"
"-e" "$DISCOURSE_ENV"
chpst -u "$DISCOURSE_DAEMON_USER" -U "$DISCOURSE_DAEMON_USER" "bundle" "exec" "config/unicorn_launcher"
Copy link
Member

Choose a reason for hiding this comment

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

No need to use chpst if runit won't be used

Suggested change
chpst -u "$DISCOURSE_DAEMON_USER" -U "$DISCOURSE_DAEMON_USER" "bundle" "exec" "config/unicorn_launcher"
"bundle" "exec" "config/unicorn_launcher"

"-E" "$DISCOURSE_ENV"
"-p" "$DISCOURSE_PORT_NUMBER"
"--spawn-method" "$DISCOURSE_PASSENGER_SPAWN_METHOD"
"-c" "config/unicorn.conf.rb"
)

# Append extra flags specified via environment variables
Copy link
Member

Choose a reason for hiding this comment

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

Please rename PASSENGER env variables.

Expand All @@ -33,4 +32,4 @@ if [[ -n "$DISCOURSE_PASSENGER_EXTRA_FLAGS" ]]; then
fi

info "** Starting Discourse **"
exec "${cmd[@]}" "$@"
USER=$DISCOURSE_DAEMON_USER exec "${cmd[@]}" "$@"
Copy link
Member

Choose a reason for hiding this comment

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

To avoid running the process as root, use the following instead:

Suggested change
USER=$DISCOURSE_DAEMON_USER exec "${cmd[@]}" "$@"
if am_i_root; then
exec gosu "$DISCOURSE_DAEMON_USER" "${cmd[@]}"
else
exec "${cmd[@]}"
fi

5 changes: 5 additions & 0 deletions bitnami/discourse/2/debian-11/rootfs/opt/bitnami/scripts/libdiscourse.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,11 @@ discourse_create_conf_file() {
discourse_conf_set "smtp_enable_start_tls" "$([[ "$DISCOURSE_SMTP_PROTOCOL" = "tls" ]] && echo "true" || echo "false")"
discourse_conf_set "smtp_authentication" "$DISCOURSE_SMTP_AUTH"
fi

if ! is_empty_value "$DISCOURSE_SERVE_STATIC_ASSETS"; then
discourse_conf_set "serve_static_assets" "$DISCOURSE_SERVE_STATIC_ASSETS"
fi

# Extra configuration
! is_empty_value "$DISCOURSE_EXTRA_CONF_CONTENT" && echo "$DISCOURSE_EXTRA_CONF_CONTENT" >> "$DISCOURSE_CONF_FILE"
}
Expand Down
Loading