fix(php): fixed php debian recipe to accommodate recent php agent changes #1131
Conversation
shashank-reddy-nr
changed the title
shashank-reddy-nr
force-pushed
the
php-debian-fix
branch
from
f89514f to
6667702
Compare
| if [[ -z "${php_ini_dir}" ]]; then | ||
| if [[ -z "${CLI_DIRS}" ]]; then | ||
| if [[ -d "${ini}/../apache2" ]]; then | ||
| php_ini_dir="${ini}/../apache2" | ||
| fi | ||
| if [[ -d "${ini}/../fpm" ]]; then | ||
| php_ini_dir="${ini}/../fpm" | ||
| fi | ||
| elif [[ -d "${ini}/../cli" ]]; then | ||
| php_ini_dir="${ini}/../cli" | ||
| else | ||
| php_ini_dir=$ini | ||
| fi |
There was a problem hiding this comment.
Changes Explained:
As stated by the PHP Agent team, with the latest PHP Agent v11.3.0.16 released around two weeks ago, they've introduced a change, which would affect the functioning of the PHP tarball installer.
With the debian (package) installer using the etc/php/7.x/mods-available directory (by design) to configure newrelic.ini, customers tending to upgrade using the tarball installer would sometimes run into issues, since the installer would place a duplicate newrelic.ini in etc/php/7.x apache2 (or) fpm (or) cli, leading to duplicates of the newrelic.ini file. To solve this problem, the solution devised by the agent team was to allow for "skipping" placing a newrelic.ini file elsewhere, if a newrelic.ini is already found to have been present in the mods-available directory by the tarball installer.
This affected the recipe owing to the following reason.
The source of ${ini} here is a collection of probable directories where the php.ini file could be present, originating from pi_inidir_dso and/or pi_inidir_cli originating from this place in the agent, which the recipe scrapes.
This list earlier contained a bunch of directories such as apache2 or fpm or cli, which would contain a conf.d and php.ini; however, in order to allow for the tarball installer to check for the presence of a newrelic.ini in mods-available, this has also been in the code linked above, to override these paths when present. This wouldn't cause an issue with the agent, since the intended use is to skip populating a newrelic.ini in any of the aforementioned directories, if there is a mods-available with a newrelic.ini.
However, in the case of this recipe, since we rely on the ${ini} picked later in the code to set the target_ini_dir where the newrelic.ini would need to be moved, the value of ${ini}, which would ideally need to be one of the aforementioned directories and then allow traversing to mods-available to overwrite the newrelic.ini file there, is now mods-available already, which is why it is not able to traverse out of mods-available to mods-available; which is exactly what the error upon guided install currently states.
To avoid this problem, two things needed to be done -
-
firstly, if
${ini}is bound to bemods-availablewhenever anewrelic.iniis present inmods-available, reverse engineer the conditions to fix the effective${ini}we shall use to move thenewrelic.inito, usingphp_ini_dir. In other words, conditionally assign${ini}tophp_ini_dirbased on the path found at../of${ini}, which would in turn, go back to the expectedapache2orfpmorclibased on the conditions written. If none of the conditions are satisfied,php_ini_dirtakes${ini}, which is also likely to happen when${ini}is notmods-available, so either way, this conditional logic preventsphp_ini_dirfrom being assigned to the wrong directory, especially since this is used later in the code. -
the condition to configure
target_ini_dirwas earlier configured based on${ini}which may now be madephp_ini_dir(with one lesser folder to traverse out of,../instead of../../) sincephp_ini_dirnow takes the "effective"${ini}and not the actual${ini}directly, making sure that the traversal would never fail, which would prevent the error we're seeing now
| target_ini_dir="${ini}/../../mods-available/" | ||
| target_ini_dir="${php_ini_dir}/../mods-available/" |
There was a problem hiding this comment.
explained in the second point here
|
|
||
| LICENSE_KEY="{{.NEW_RELIC_LICENSE_KEY}}" | ||
| sed -i -E "s/newrelic.license = \"(REPLACE_WITH_REAL_KEY|)\"/newrelic.license = \"${LICENSE_KEY}\"/" $ini_full_name | ||
|
|
There was a problem hiding this comment.
In continuation to the explanation in the above comment, a different limitation noticed because of the aforementioned change in the agent was that the newrelic.ini file populated in the mods-available directory had no license key populated.
Upon discussing this with the PHP Agent, it was obtained that the cause of this is as follows. Earlier, since a new newrelic.ini would be written to one of apache2 or fpm or cli by the recipe (as provided by the tarball installer, even in the event of a newrelic.ini existing in mods-available), the newrelic.ini file would be written with the license key, and our code in the recipe would move the newrelic.ini from one of these folders into target_ini_dir, i.e. ../../mods-available, thereby making sure that the newrelic.ini in mods-available is overwritten with the newrelic.ini in one of these directories, with the license key, facilitating instrumentation.
However, now that the newrelic.ini file is no longer generated in any of these directories if there already exists a newrelic.ini in mods-available, the newrelic.ini in mods-available is not replaced by any other version of newrelic.ini elsewhere.
Added to this is the fact that the newrelic.ini in mods-available tends to be generated without a license key.
A combination of all of these would mean we would need to sed our way into affixing the license key into the newrelic.ini that is now bound to stay in mods-available, but did not have the license key so far, and hence needs it; this is why this change is needed.
| ini_source_dir=$(dirname "$(realpath "$ini_full_name")") | ||
| ini_destination_dir=$(realpath "$target_ini_dir") | ||
| if [[ "$ini_source_dir" != "$ini_destination_dir" ]]; then | ||
| mv "${ini_full_name}" "${ini_destination_dir}" | ||
| fi |
There was a problem hiding this comment.
Finally, this code block is a consequence of everything else described above. $ini_full_name holds the path to newrelic.ini in ${ini}.
With all the context above, and the model used by the recipe to first install via the package-based installer and then use tarball, with the former leading to placing a newrelic.ini in mods-available, target_ini_dir was always meant to be mods-available, prior to these changes too (to move any updated newrelic.ini to replace the newrelic.ini in mods-available).
However, now with the concept of "updating" newrelic.ini in mods-available vanishing (if it already exists, no other newrelic.ini is added to other directories), and the probability of ${ini} being mods-available when a newrelic.ini is found in mods-available, it is very likely that the path to ini_full_name and target_ini_dir would now be the same; in which case it wouldn't make sense to perform the move operation in the code block. Nevertheless, to make this check and only perform this when both of these paths are unequal, the condition has been added.
There was a problem hiding this comment.
The reason why this piece of code is necessary is that if a user installs the PHP agent manually using a tarball installer, they will have newrelic.ini in fpm or apache2 and cli, but not in mods-available as per agent logic. If, in the future, user want to run the guided install again on same host, the agent will not add newrelic.ini to any locations. However, the agent will inform us that newrelic.ini is already present in cli/conf.d and fpm/conf.d. Therefore, we will move the newrelic.ini to mods-available using this piece of code.
|
Thanks @pranav-new-relic for writing details explanation of code changes |
shashank-reddy-nr
changed the title
Changes:
Recently, php agent team have release few agent changes which broke php guided install. So, inorder to accommodate new agent changes, made required changes in the php recipe and validated.
Results:
1: Installation result of php recipe with this PR changes on top of failing installation host.
2: PHP installation Totally on new host
