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

Small fixes to docker setup (logs, static file routing, healthcheck, dockerignore) #22679

Merged
merged 6 commits into from
Sep 24, 2024

Conversation

KevinMind
Copy link
Contributor

@KevinMind KevinMind commented Sep 18, 2024

Fixes: mozilla/addons#15026

Description

Semi related small commits fixing several known bugs in our local dev setup

Context

I combined these for efficiency as it would be many tiny PRs that should be tested more or less in the same way.

Testing

make up COMPOSE_FILE=docker-compose.yml:docker-compose.ci.yml
  1. running make up should never build.. hard to test but it won't
  2. anonymous volumes are recreated if running make up when containers are already up.
  3. make data_restore works and doesn't fail with invalid index command
  4. make clean_docker works and doesn't perform unnecessary logic

Checklist

  • Add #ISSUENUM at the top of your PR to an existing open issue in the mozilla/addons repository.
  • Successfully verified the change locally.
  • The change is covered by automated tests, or otherwise indicated why doing so is unnecessary/impossible.
  • Add before and after screenshots (Only for changes that impact the UI).
  • Add or update relevant docs reflecting the changes made.

@KevinMind KevinMind marked this pull request as ready for review September 18, 2024 17:25
@KevinMind KevinMind force-pushed the clean-logs branch 4 times, most recently from 1380cdd to 10d1844 Compare September 19, 2024 09:21
@KevinMind KevinMind requested review from a team and eviljeff and removed request for a team September 19, 2024 11:47
@eviljeff
Copy link
Member

Are these all the issues that are being fixed, and can you expand on the description? There are 5 commits, 4 sets of the testing instructions, but only 2 linked issues. (None of which seem to be about cleaning logs - can you update the PR title too)

@KevinMind KevinMind changed the title Clean-logs Small fixes to docker setup (logs, static file routing, healthcheck, dockerignore) Sep 20, 2024
@KevinMind
Copy link
Contributor Author

KevinMind commented Sep 20, 2024

Are these all the issues that are being fixed, and can you expand on the description? There are 5 commits, 4 sets of the testing instructions, but only 2 linked issues. (None of which seem to be about cleaning logs - can you update the PR title too)

Several are indeed about logs. I didn't find any other tickets matching my commits. Added test set for the last commit.

docker-compose.yml Outdated Show resolved Hide resolved
.dockerignore Show resolved Hide resolved
.github/workflows/ci.yml Show resolved Hide resolved
docker/nginx/addons.conf Outdated Show resolved Hide resolved
Copy link
Member

@eviljeff eviljeff left a comment

Choose a reason for hiding this comment

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

I didn't see any affect from changes to start_period - it took ~90 seconds to get to Healthy so I changed the start_period in docker-compose to 180 and it still took ~90 seconds 🤷

@KevinMind KevinMind merged commit 90f997a into master Sep 24, 2024
2 checks passed
@KevinMind KevinMind deleted the clean-logs branch September 24, 2024 17:54
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.

[Bug]: docker compose healthchecks are too aggressive
2 participants