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

Adds nginx test on container restart and makes use of EE_DOCKER methods #315

Closed
wants to merge 1 commit into from
Closed

Adds nginx test on container restart and makes use of EE_DOCKER methods #315

wants to merge 1 commit into from

Conversation

geekodour
Copy link

this PR is tightly related to the changes made in EasyEngine/easyengine#1428

Signed-off-by: Hrishikesh Barman [email protected]

* removed `run_compose_command` and replaced the required with methods added in EasyEngine/easyengine#1428
* also closes https://github.com/rtCamp/sys/issues/96 by adding nginx test before container restart

Signed-off-by: Hrishikesh Barman <[email protected]>
if ( in_array( 'nginx', $containers ) ) {
$nginx_test_command = "sh -c 'nginx -t'";
if ( ! \EE_DOCKER::docker_compose_exec( $this->site_data['site_fs_path'], 'nginx', $nginx_test_command ) ) {
throw new \Exception( 'There was some error in docker-compose exec.' );
Copy link
Member

Choose a reason for hiding this comment

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

A better error message on failure would be: Nginx configuration test failed. Please check the site's nginx config.
There was some error in docker-compose exec is a very generic message and does not convey anything helpful to the end user using EasyEngine.

$this->reload_services( $whitelisted_containers, $reload_commands );
foreach ( $whitelisted_containers as $container ) {
if ( ! \EE_DOCKER::docker_compose_exec( $this->site_data['site_fs_path'], $container, $reload_commands[ $container ] ) ) {
throw new \Exception( 'There was some error in docker-compose exec.' );
Copy link
Member

Choose a reason for hiding this comment

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

Again something useful can be added here like: Reloading service for $container failed.

EE::exec( "docker-compose $action $container", true, true );
$stopped = \EE_DOCKER::docker_compose_stop( $site_fs_path, $all_containers );
$forcermd = \EE_DOCKER::docker_compose_forcerm( $site_fs_path, $all_containers );
if ( ! (stopped && forcermd) ) {
Copy link
Member

Choose a reason for hiding this comment

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

Missing $ in variable names. Also, fix phpcs issues.

@kirtangajjar
Copy link
Contributor

Closing this PR as there are multiple issues and has been left unresolved > 1 year

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants