-
Notifications
You must be signed in to change notification settings - Fork 396
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
Deploy / Update Hook Enhancements and Bugfixes for Drush 9 #3006
Closed
Closed
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
87164a4
Update drush to pinned dev-master to use DRUSH_PATHS_CACHE_DIRECTORY …
lcatlett 7404038
Add cache directory utility to use in update and post install tasks.
lcatlett 4e52dd9
Remove deprecated _ENV and server globals from post install hooks.
lcatlett a83b8e0
Remove redundant cache clears from config update commands.
lcatlett c31df3f
Set default value for drush file based cache dir config.
lcatlett d57600f
Update db update factory hook DRUSH_PATHS_CACHE_DIRECTORY to use per-…
lcatlett dfe1810
Add update hook to refresh factory hook templates.
lcatlett 7da0fbe
Add exception for projects not using Factory hooks.
lcatlett 6c0d171
Style and syntax clean up.
lcatlett b002c4b
Add updated host forwarding, remove globals.
lcatlett 22c2850
Merge branch '9.x' into deploy-update-refactor
lcatlett 83385e9
Merge branch '9.x' into deploy-update-refactor
lcatlett File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
<?php | ||
|
||
/** | ||
* @file | ||
* Creates a unique temporary cache directory given a site, env, and uri. | ||
* This is used for every blt command invocation on site install and update. | ||
*/ | ||
|
||
$site = $argv[1]; | ||
$env = $argv[2]; | ||
$uri = $argv[3]; | ||
|
||
|
||
if (empty($argv[3])) { | ||
echo "Error: Not enough arguments. Site, environment, and uri are required.\n"; | ||
exit(1); | ||
} | ||
|
||
// Create a temporary cache directory for this drush process only. | ||
$cache_directory = sprintf('/mnt/tmp/%s.%s/drush_tmp_cache/%s', $site, $env, md5($uri)); | ||
shell_exec(sprintf('mkdir -p %s', escapeshellarg($cache_directory))); | ||
|
||
if (!file_exists($cache_directory)) { | ||
syslog(LOG_ERR, sprintf('Drush updates could not be executed, as the required cache directory [%s] is missing.', $cache_directory)); | ||
die('Missing or corrupted drush cache for this process.'); | ||
} | ||
else { | ||
echo "$cache_directory"; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
#!/bin/bash | ||
##!/bin/bash | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Extra hash here I had to comment out in order to get bash to run this. |
||
# | ||
# Factory Hook: db-update | ||
# | ||
|
@@ -26,13 +26,16 @@ blt="/var/www/html/$site.$env/vendor/acquia/blt/bin/blt" | |
uri=`/usr/bin/env php /mnt/www/html/$site.$env/hooks/acquia/uri.php $site $env $db_role` | ||
|
||
# Print a statement to the cloud log. | ||
echo "$site.$target_env: Running BLT deploy tasks on $uri domain in $env environment on the $site subscription." | ||
echo "Running BLT deploy tasks on $uri domain in $env environment on the $site subscription." | ||
|
||
IFS='.' read -a name <<< "${uri}" | ||
|
||
# Set Drush cache to local ephemeral storage to avoid race conditions. This is | ||
# done on a per site basis to completely avoid race conditions. | ||
# Create and set Drush cache to local ephemeral storage to avoid race conditions. This is | ||
# done on a per site process basis to completely avoid race conditions. | ||
# @see https://github.com/acquia/blt/pull/2922 | ||
export DRUSH_PATHS_CACHE_DIRECTORY=/tmp/.drush/${db_role} | ||
|
||
$blt drupal:update --environment=$env --site=${name[0]} --define drush.uri=$domain --verbose --yes | ||
cacheDir=`/usr/bin/env php /mnt/www/html/$site.$env/vendor/acquia/blt/scripts/blt/cache.php $site $env $uri` | ||
|
||
DRUSH_PATHS_CACHE_DIRECTORY=$cacheDir $blt drupal:update --environment=$env --site=${name[0]} --define drush.uri=$domain --verbose --yes --no-interaction | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this functionally different from the existing code? It looks like this PR is assigning a cache directory based on site URI, whereas the existing site is assigning it based on db_role. Shouldn't that basically do the same thing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
passing the directory as a cli prefix ensures that the child processes spawned from the current shell inherit the value. Exporting the value in the bash shell could still result in another process changing or overriding what should be a unique value. The db_role alone isnt enough to isolate the calls to cache rebuild in particular because on ACSF the live and update environments share a database, so env differentiates the two. It is more significant that that the cache directly is in /mnt as opposed to /tmp and that this directory is also created and deleted to avoid permissions issues and race conditions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that using
export
makes the variable available to all child / sub-processes. Ref: https://stackoverflow.com/questions/1158091/defining-a-variable-with-or-without-exportThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the issue is that the value can then be modified by exec or other subprocesses of the blt and respective drush commands which does not prevent cache collisions on environments with multiple web servers. if you have suggestions on how to better solve this I am happy to refactor.