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

Suite improvements for new lando (bug 1885346) (bug 1904696) (bug 1920987) #59

Merged
merged 18 commits into from
Oct 28, 2024

Conversation

shtrom
Copy link
Member

@shtrom shtrom commented Sep 26, 2024

No description provided.

@shtrom shtrom changed the title Suite fixups (early review) (bug 1921103) (bug 1885346) Suite fixups (early review) (bug 1921103) (bug 1885346) (bug 1904696) Sep 26, 2024
@shtrom shtrom force-pushed the suite-fixups branch 3 times, most recently from df0c3d7 to 23c9272 Compare October 2, 2024 02:03
@shtrom shtrom changed the title Suite fixups (early review) (bug 1921103) (bug 1885346) (bug 1904696) Suite fixups (early review) (bug 1921103) (bug 1885346) (bug 1904696) (bug 1921450) Oct 2, 2024
@shtrom shtrom force-pushed the suite-fixups branch 2 times, most recently from d19531b to 65fe9e4 Compare October 7, 2024 03:19
shtrom added 7 commits October 7, 2024 14:21
* mysqls
* phab and bmo
* phabricator.test

Signed-off-by: Olivier Mehani <[email protected]>
…time

This is useful when the target container has been replaced and has a new
IP address, but the nginx container still points to the old, cached, IP.

Signed-off-by: Olivier Mehani <[email protected]>
@shtrom shtrom force-pushed the suite-fixups branch 2 times, most recently from 7354c25 to 1d80265 Compare October 7, 2024 04:08
@shtrom shtrom marked this pull request as ready for review October 7, 2024 04:08
@shtrom
Copy link
Member Author

shtrom commented Oct 7, 2024

@zzzeid this PR is a combination of

  1. your new-lando PR lando: add lando functionality (bug 1885346) #52
  2. suite improvements suite improvements (bug 1920987) #61
  3. (new) suite improvements for new lando

There may be a couple of conflicts when merging back into main due to changes to the lando- vs. lando-ui+lando-api -related, but it should just be a matter of allowing deletion of the old lando files.

@shtrom shtrom changed the title Suite fixups (early review) (bug 1921103) (bug 1885346) (bug 1904696) (bug 1921450) Suite improvements for new lando (bug 1921103) (bug 1885346) (bug 1904696) (bug 1921450) Oct 7, 2024
@shtrom shtrom changed the title Suite improvements for new lando (bug 1921103) (bug 1885346) (bug 1904696) (bug 1921450) Suite improvements for new lando (bug 1885346) (bug 1904696) (bug 1920987) Oct 7, 2024
@shtrom shtrom requested a review from zzzeid October 10, 2024 06:03
Comment on lines 162 to 177
lando-init:
user: "${UID}:${GID}"
image: mozilla/lando:latest # TODO: this does not exist yet.
command: lando setup_dev
env_file:
- ../lando/.env
environment:
<<: *base-lando-vars
depends_on:
lando.db:
condition: service_healthy
phabricator.test:
condition: service_healthy

Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed, might be better to include the setup_dev command in the main lando container instead, along with the other development specific commands. Also, this could be combined in the main compose file.

@shtrom
Copy link
Member Author

shtrom commented Oct 28, 2024

Ok, I moved things around, and did some cleanup while I was at it. I added a healthcheck, and needed to add a dependency on phabricator.test before starting lando, as it tries (and fails) to talk to it on start and gets into a restart loop.

Starting with the override,

docker-compose -f docker-compose.yml -f docker-compose.bmo.yml -f docker-compose.lando.yml up -d

works reliably (but is a bit slow due to the phabricator dep).

I tested without the override, by using the suite_lando image (after hacking it so it had the version file and static assets already installed) in the base compose file, and that booted fine, too, so we should be OK (:

Copy link
Contributor

@zzzeid zzzeid left a comment

Choose a reason for hiding this comment

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

LGTM. Will merge it now, and if we need to fine tune any of the changes we can do so before we merge #52 in.

@zzzeid zzzeid merged commit 21fc4c0 into mozilla-conduit:zeid/bug-1885346-new-lando Oct 28, 2024
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.

2 participants