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

chore: notify user if docker-compose fails #2064

Closed
1 task
gabrielmer opened this issue Sep 21, 2023 · 3 comments · Fixed by waku-org/nwaku-compose#11
Closed
1 task

chore: notify user if docker-compose fails #2064

gabrielmer opened this issue Sep 21, 2023 · 3 comments · Fixed by waku-org/nwaku-compose#11
Assignees
Labels
E:3.2: Basic DoS protection in production See https://github.com/waku-org/pm/issues/70 for details enhancement New feature or request

Comments

@gabrielmer
Copy link
Contributor

gabrielmer commented Sep 21, 2023

Background

Currently we instruct the users to use nwaku-compose by using the following command: docker-compose up -d.
In addition, we run the docker-compose services with restart: on-failure configuration.
This means that if the run fails during setup because of a missing or invalid input, the user won't see any error in the terminal because of being on detached mode, and the faulty container will keep restarting itself in the background indefinitely as per the restart policy.

Details

This scenario will become usual with the addition of RLN, as the user is required to provide an Ethereum node URL and might try running docker-compose without having set up an address.
Because we instruct to run on detached mode, the terminal will notify that the containers successfully started running but won't show or throw any error during setup because it has already detached.

After some investigation, I found two main ways to approach this problem:
1- Not instruct the users to run on detached mode and possibly use a flag such as --abort-on-container-exit to stop the whole run if a component fails.
2- Create a wrapper script that runs docker-compose on detached mode and checks after a certain amount of seconds that everything is properly up. If everything is ok by then, exits and docker-compose continues its detached run.
In this case, if an error happens mid-run the user won't be notified but it will help the user in common scenarios of missing or invalid inputs.

We can also improve the restart policy to allow a maximum amount of failures in a time window, and prevent the case that the container keeps restarting itself indefinitely because of a wrong input. That would mean that after failing and restarting N amount of times during certain timespan, the container gives up and exits.

cc @Ivansete-status @vpavlin @alrevuelta @rymnc

Acceptance criteria

  • The user is promptly notified if an error occurs when spinning-up their nwaku-compose instance
@gabrielmer gabrielmer self-assigned this Sep 21, 2023
@Ivansete-status
Copy link
Collaborator

Thanks for submitting this!
From my point of view, the best approach is to recommend the users start it in non-detached mode and if a docker container fails, then don't restart it (remove the on-failure.) I think it is essential to notify the user as early as possible when an error occurs.

@vpavlin
Copy link
Member

vpavlin commented Sep 26, 2023

I would recommend using on-failure with some max-retries limit and adding Troubleshooting into README.md to explain how to check that the node is up (JSON RPC or Rest API call) and if it not up, then describe how to use docker logs to get to the nwaku log output to find the actual error.

@gabrielmer
Copy link
Contributor Author

Weekly Update

  • achieved: discussed the issue with colleagues, implemented the solution and closed the issue

@fryorcraken fryorcraken added enhancement New feature or request E:3.2: Basic DoS protection in production See https://github.com/waku-org/pm/issues/70 for details labels Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E:3.2: Basic DoS protection in production See https://github.com/waku-org/pm/issues/70 for details enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants