drainpipe
drainpipe copied to clipboard
Lullabot
Include binaries during ddev start #172
https://github.com/Lullabot/drainpipe/issues/164 came up on two separate projects again within a day. As well, we noticed that the error still happens even when not running parallel jobs, which we had previously thought was a workaround.
I know we had left #172 at "needs discussion", but I had a good idea from other work how to approach this, so I figured a PR gave us something to talk about instead of something theoretical.
This PR adds a composer config to disable the current behaviour of downloading task and local-php-security-checker. That way, there's no immediate change for current sites, and they can migrate any existing CI or build scripts one tool at a time.
composer config --json extra.drainpipe.global-binaries.task "true"
composer config --json extra.drainpipe.global-binaries.local-php-security-checker "true"
This PR should also fix:
- https://github.com/Lullabot/drainpipe/issues/164
- https://github.com/Lullabot/drainpipe/issues/135
- https://github.com/Lullabot/drainpipe/issues/129
As well, it fixes users accidentally running composer on the host, getting the host OS binary, and then being confused with the binary not working inside of ddev.
Finally, it upgrades local-php-security-checker because version 1 isn't available for ARM macs.
Talked about this today:
- "We agreed this is a great idea"
- "Andrew is going to fix some little bits"
- "lets ship it"
@alexis-saransig-lullabot confirmed this is now working on Lullabot.com https://github.com/Lullabot/lullabot.com-d8/pull/2696 https://github.com/Lullabot/lullabot.com-d8/pull/2696/files#diff-d2ab9925cad7eac58e0ff4cc0d251a937ecf49e4b6bf57f8b95aab76648a9d34R177
@deviantintegral I added a test for this and it appears to have broken ddev drush somehow :thinking:
@justafish my guess is this is erroring because settings.php isn't including settings.ddev.php, because we run composer install after ddev is configured. Can you pick it up from here?
@deviantintegral
I attempted to merge main branch into 172--global-binaries and I got a Permission denied error. I need the latest main changes on this branch so that I can upgrade a site that uses drainpipe to Drupal 10.
@elvism-lullabot I added you as a collaborator to this project. Feel free to push to this branch.
@deviantintegral Thank you! I guess I should see all checks to be successful before upgrading the reference on the site I need, right?
Yeah, I would get things working here first, especially the test production build one.
@justafish this is ready for a review again.
The one failing test appears to be unrelated, as its failing on main and has to do with yarn and nightwatch which this PR doesn't touch: https://github.com/Lullabot/drainpipe/actions/runs/6338460550/job/17215706080
I ran the failed test jobs locally until the point where the environment was being pushed to pantheon.
The job failed with:
547
> In SiteInstallCommands.php line 181:
548
>
549
> Existing configuration directory not found or does not contain a core.exten
550
> sion.yml file.".
I verified that file existed in /tmp/release/config/core.extension.yml. @justafish is there any chance the multidev environment is broken outside of this PR's changes?
@deviantintegral I deleted the environment and branch directly in the Pantheon dashboard, updated this branch, then rebuilt - seems to have worked!
The GitHub actions are assuming that global binaries are turned on. We need to make it so they check if task lives in the global path or vendor/bin.
@justafish I made the improvement in https://github.com/Lullabot/drainpipe/pull/207/commits/ca13661b86730a680608ed9e3a012bb66ef6283f and builds pass now. Back to you!
Based on https://github.com/Lullabot/drainpipe/pull/355 I believe our tests on main are broken.
I've marked this as high because I know of at least 3 client projects the bugs this solves has affected.
I believe we've hit our multidev limit or similar as we're getting errors running wipe that are unrelated to this PR: https://github.com/Lullabot/drainpipe/actions/runs/8297688089/job/22709329644?pr=207
I've noticed that on other PRs as well. I've been going in and manually deleting the oldest MultiDevs and re-running jobs for Renovate PRs. I just merged a Renovate pull request that all checks passed and then re-ran the action for this one.
@deviantintegral I've asked Pantheon support chat to bump us to 15 MultiDev and they said we'd have to have a Gold level account. I told them we were a Pantheon partner and they've done this for Lullabot.com which is on a Small tier, they still said they wouldn't do it. Maybe there's someone else we can ask?
Looks like this is now failing because of a real error during the deploy to Pantheon.
https://github.com/Lullabot/drainpipe/actions/runs/8297688089/job/22710408917?pr=207#step:15:544
[warning] Waited '180' seconds, giving up waiting for workflow to finish
[notice] Fetching site information to build Drush aliases...
[notice] 1 sites found.
[notice] Writing Drush 8 alias file to ~/.drush/pantheon.aliases.drushrc.php
[notice] Writing Drush 9 alias files to ~/.drush/sites/pantheon
> Warning: Permanently added '[appserver.pr-207.22fcd7e9-8dd3-4bab-8fe0-7c856b1ee202.drush.in]:2222,[34.34.44.43]:2222' (RSA) to the list of known hosts.
You are about to:
* DROP all tables in your 'pantheon' database.
// Do you want to continue?: yes.
>
> In SiteInstallCommands.php line 181:
>
> Existing configuration directory not found or does not contain a core.exten
> sion.yml file.".
>
>
In SiteProcess.php line 214:
The command "ssh -p 2222 -o "AddressFamily inet" pr-207.22fcd7e9-8dd3-4bab-
8fe0-7c856b1ee202@appserver.pr-207.22fcd7e9-8dd3-4bab-8fe0-7c856b1ee202.dru
sh.in 'drush --yes site:install --existing-config --uri=pr-207-***.pa
ntheonsite.io'" failed.
Exit Code: 1(General error)
Working directory:
Output:
================
You are about to:
* DROP all tables in your 'pantheon' database.
// Do you want to continue?: yes.
Error Output:
================
Warning: Permanently added '[appserver.pr-207.22fcd7e9-8dd3-4bab-8fe0-7c856
b1ee202.drush.in]:2222,[34.34.44.43]:2222' (RSA) to the list of known hosts
.
In SiteInstallCommands.php line 181:
Existing configuration directory not found or does not contain a core.ext
en
sion.yml file.".
Not sure how the test failure is related. Sadly the drush command doesn't give more information (that message has been fixed in HEAD) https://github.com/drush-ops/drush/pull/5881
I've re-tested and now getting a different error (terminus missing a sitename argument).
@justafish @deviantintegral @davereid Any reason to not update this branch to the latest in main? Or the latest in the 3.7.0 release?? https://github.com/Lullabot/drainpipe/releases
I thought of this, cause we went to use https://github.com/Lullabot/drainpipe/pull/470 on a project that we are pulling in like "lullabot/drainpipe": "dev-172--global-binaries as 3.7.0", ... and I wanted to use https://github.com/Lullabot/drainpipe/pull/470 which went into v3.6.0. cc @leonel-lullabot
@YesCT hopefully this branch won't be needed anymore with the last round of bug fixes in 3.7.1 and adding composer install to your ddev start hook
With 3.7.1, on tugboat, I ran into:
In Filesystem.php line 370:
copy(/var/lib/tugboat/vendor/lullabot/drainpipe/scaffold/ddev/web-build/):
Failed to open stream: No such file or directory
Though we aren't using the new tugboat drainpipe integration.
@justafish For the filesystem.php error, @leonel-lullabot was able to bypass the error. (By running the init steps again on the preview I think.) So, no known concerns from me about closing this.
@YesCT It could be an issue on CI, where you don't have the option for "rerun again". I have https://lullabot.slack.com/archives/C019JP1EN1Y/p1712262592896549?thread_ts=1712245179.161029&cid=C019JP1EN1Y on my pile for testing if that would work, but more than happy if you beat me to do it.
Following up from https://github.com/Lullabot/drainpipe/pull/207#issuecomment-2056790567, we think this is solved by adding composer install as a ddev post-start hook.
I don't know if that makes sense for us to ship automatically, or to just add as documentation. Regardless, I'm going to close this PR - but I'll leave the branch in place in case anyone is pointing to it. Thanks all for your efforts on this one!