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

docker: Fix web UI container script in basic execution #3607

Merged

Conversation

ilausuch
Copy link
Contributor

@ilausuch ilausuch commented Dec 2, 2020

When web UI is executed in a container executes the script run_openqa.sh
if this is executed in the default mode with all components together
uses the function all_together_apache(). This function has some errors
that avoid the execution of the scheduler, websockets and livehandler
because some parameters for the command start_daemon are missing.

Additionally the same simplification applied here to the gru daemon
is also applied to the function gru() which is used by the load balance
execution mode.

progress.opensuse.org/issues/80534

docker/webui/run_openqa.sh Outdated Show resolved Hide resolved
docker/webui/run_openqa.sh Outdated Show resolved Hide resolved
docker/webui/run_openqa.sh Outdated Show resolved Hide resolved
@ilausuch ilausuch force-pushed the fix_basic_webui_container_execution branch from 78a7324 to 8560836 Compare December 2, 2020 13:26
@ilausuch
Copy link
Contributor Author

ilausuch commented Dec 2, 2020

Done. I tested this and all the services are running

info] Listening at "http://127.0.0.1:9527"
Web application available at http://127.0.0.1:9527
[info] Listening at "http://[::1]:9527"
Web application available at http://[::1]:9527
[info] Listening at "http://127.0.0.1:9528"
Web application available at http://127.0.0.1:9528
[info] Listening at "http://[::1]:9528"
Web application available at http://[::1]:9528
[info] Listening at "http://127.0.0.1:9529"
Web application available at http://127.0.0.1:9529
[info] Listening at "http://[::1]:9529"
Web application available at http://[::1]:9529
[info] Resetting all leftover Gru locks after restart
[info] Listening at "http://127.0.0.1:9526"
Web application available at http://127.0.0.1:9526
[info] Listening at "http://[::1]:9526"
Web application available at http://[::1]:9526

docker/webui/run_openqa.sh Outdated Show resolved Hide resolved
Copy link
Member

@okurz okurz left a comment

Choose a reason for hiding this comment

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

see suggestions from Martchus and please fix typo "livehadler" in git commit message. Please reference a ticket as well

@ilausuch ilausuch force-pushed the fix_basic_webui_container_execution branch 2 times, most recently from 6cd164e to 5069a09 Compare December 3, 2020 08:08
@ilausuch
Copy link
Contributor Author

ilausuch commented Dec 3, 2020

All suggestions done and also I modified the gru() function that is used in the load balance execution mode because of the same reason

@ilausuch ilausuch force-pushed the fix_basic_webui_container_execution branch from 5069a09 to 9c8cda3 Compare December 3, 2020 10:03
Copy link
Member

@okurz okurz left a comment

Choose a reason for hiding this comment

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

in git commit message s/witch/which/, rest is fine, self-approve or merge afterwards

@Martchus
Copy link
Contributor

Martchus commented Dec 3, 2020

When amending the commit message anyways: The grammar could be improved as well, e.g. add at least a "." at the end. And I still prefer "web UI" over "webUI". This is not a camel case variable name and the unabbreviated form ("web user interface") would also have the space.

When web UI is executed in a container executes the script run_openqa.sh
if this is executed in the default mode with all components together
uses the function all_together_apache(). This function has some errors
that avoid the execution of the scheduler, websockets and livehandler
because some parameters for the command start_daemon are missing.

Additionally the same simplification applied here to the gru daemon
is also applied to the function gru() which is used by the load balance
execution mode.

https://progress.opensuse.org/issues/80534
@ilausuch ilausuch force-pushed the fix_basic_webui_container_execution branch from 9c8cda3 to 1314780 Compare December 7, 2020 13:48
@ilausuch ilausuch changed the title docker: Fix webUI container script in basic execution docker: Fix web UI container script in basic execution Dec 7, 2020
@ilausuch
Copy link
Contributor Author

ilausuch commented Dec 7, 2020

Done both things. Thanks

@codecov
Copy link

codecov bot commented Dec 7, 2020

Codecov Report

Merging #3607 (1314780) into master (d86d555) will decrease coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3607      +/-   ##
==========================================
- Coverage   95.68%   95.64%   -0.05%     
==========================================
  Files         367      367              
  Lines       31859    31859              
==========================================
- Hits        30485    30472      -13     
- Misses       1374     1387      +13     
Impacted Files Coverage Δ
t/lib/OpenQA/Test/Utils.pm 65.24% <0.00%> (-3.71%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d86d555...1314780. Read the comment docs.

@mergify mergify bot merged commit ab9d59c into os-autoinst:master Dec 7, 2020
openqabot pushed a commit to openqabot/openQA that referenced this pull request Dec 8, 2020
commit ab9d59c
Merge: 4cd1f43 1314780
Author:     mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
AuthorDate: Mon Dec 7 19:08:20 2020 +0000
Commit:     GitHub <noreply@github.com>
CommitDate: Mon Dec 7 19:08:20 2020 +0000

    Merge pull request os-autoinst#3607 from ilausuch/fix_basic_webui_container_execution

    docker: Fix web UI container script in basic execution
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.

None yet

4 participants