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

[Packaging] restore php.ini if something bad happened #116

Merged
merged 2 commits into from Aug 6, 2020

Conversation

v1v
Copy link
Member

@v1v v1v commented Aug 6, 2020

What

Let's restore the php.ini file if something bad happened. The restore should keep the file as used to be and also preserving the permissions.

Tests

$ rm -rf build
$ PHP_VERSION=7.4 make -f .ci/Makefile build
$ PHP_VERSION=7.4 make -C packaging rpm

Then, it will fail if installing php.7.4 in in php7.2:

$ PHP_VERSION=7.2 make -C packaging rpm-install
...

Installing Elastic PHP agent
; THIS IS AN AUTO-GENERATED FILE by the Elastic PHP agent post-install.sh script
extension=/opt/elastic/apm-agent-php/extensions/elastic_apm.so
elastic_apm.bootstrap_php_part_file=/opt/elastic/apm-agent-php/src/bootstrap_php_part.php
; END OF AUTO-GENERATED
PHP Warning:  PHP Startup: Unable to load dynamic library '/opt/elastic/apm-agent-php/extensions/elastic_apm.so' (tried: /opt/elastic/apm-agent-php/extensions/elastic_apm.so (/opt/elastic/apm-agent-php/extensions/elastic_apm.so: undefined symbol: zend_hash_add), /usr/lib64/php/modules//opt/elastic/apm-agent-php/extensions/elastic_apm.so.so (/usr/lib64/php/modules//opt/elastic/apm-agent-php/extensions/elastic_apm.so.so: cannot open shared object file: No such file or directory)) in Unknown on line 0
Failed enabling Elastic PHP agent extension
Reverted changes in the file /etc/php.ini
Set up the Agent manually as explained in:
https://github.com/elastic/apm-agent-php/blob/master/docs/setup.asciidoc

Then it will work when installing it in a php.version 7.4

$ PHP_VERSION=7.4 make -C packaging rpm-install
Installing Elastic PHP agent
; THIS IS AN AUTO-GENERATED FILE by the Elastic PHP agent post-install.sh script
extension=/opt/elastic/apm-agent-php/extensions/elastic_apm.so
elastic_apm.bootstrap_php_part_file=/opt/elastic/apm-agent-php/src/bootstrap_php_part.php
; END OF AUTO-GENERATED
Extension enabled successfully for Elastic PHP agent
+ php -m
+ grep -q elastic
+ composer install
Loading composer repositories with package information
Updating dependencies (including require-dev)

Issues

Closes #112

@v1v v1v self-assigned this Aug 6, 2020
@v1v v1v added the automation label Aug 6, 2020
@v1v v1v requested review from SergeyKleyman and a team August 6, 2020 13:20
Comment on lines +70 to +73
if [ -e "${PHP_INI_FILE_PATH}${BACKUP_EXTENSION}" ] ; then
echo "Reverted changes in the file ${PHP_INI_FILE_PATH}"
mv -f "${PHP_INI_FILE_PATH}${BACKUP_EXTENSION}" "${PHP_INI_FILE_PATH}"
fi
Copy link
Member Author

Choose a reason for hiding this comment

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

For the record, it uses the -f flag, so it forces the move, are we happy with this approach?

@@ -37,7 +38,8 @@ function is_extension_installed() {
################################################################################
#### Function add_extension_configuration_to_file ##############################
function add_extension_configuration_to_file() {
tee -a "$@" <<EOF
cp -fa "$1" "${1}${BACKUP_EXTENSION}"
Copy link
Member Author

@v1v v1v Aug 6, 2020

Choose a reason for hiding this comment

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

-fa flag should help to force if the file already exists and then it will populate the permissions too. Are we happy with this approach?

Copy link
Contributor

Choose a reason for hiding this comment

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

So after restore will the file have the same owner user/group and permissions?

Copy link
Member Author

@v1v v1v Aug 6, 2020

Choose a reason for hiding this comment

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

IIUC, that's the meaning for those flags, see below:

     -a    Same as -pPR options. Preserves structure and attributes of files but not directory structure.

     -f    If the destination file cannot be opened, remove it and create a new file, without prompting for confirmation regardless of its permissions.  (The -f option overrides any
           previous -n option.)

           The target file is not unlinked before the copy.  Thus, any existing access rights will be retained.
     -p    Cause cp to preserve the following attributes of each source file in the copy: modification time, access time, file flags, file mode, user ID, and group ID, as allowed by
           permissions.  Access Control Lists (ACLs) and Extended Attributes (EAs), including resource forks, will also be preserved.

           If the user ID and group ID cannot be preserved, no error message is displayed and the exit value is not altered.

           If the source file has its set-user-ID bit on and the user ID cannot be preserved, the set-user-ID bit is not preserved in the copy's permissions.  If the source file has
           its set-group-ID bit on and the group ID cannot be preserved, the set-group-ID bit is not preserved in the copy's permissions.  If the source file has both its set-user-ID
           and set-group-ID bits on, and either the user ID or group ID cannot be preserved, neither the set-user-ID nor set-group-ID bits are preserved in the copy's permissions.

@apmmachine
Copy link
Collaborator

apmmachine commented Aug 6, 2020

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [Pull request #116 updated]

  • Start Time: 2020-08-06T13:31:35.304+0000

  • Duration: 9 min 56 sec

Test stats 🧪

Test Results
Failed 0
Passed 108
Skipped 0
Total 108

@v1v v1v marked this pull request as ready for review August 6, 2020 14:16
@v1v v1v merged commit fce8009 into elastic:master Aug 6, 2020
v1v added a commit to v1v/apm-agent-php that referenced this pull request Aug 18, 2020
* upstream/master:
  Add ENVIRONMENT configuration option
  Change composer installation in docker (elastic#128)
  Add tests/APM_Agents_shared directory
  [packaging] support multiple PHP API (elastic#121)
  [Packaging] restore php.ini if something bad happened (elastic#116)
  [CI] Cosmetic changes in the stage names (elastic#115)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Packaging] Revert installation if something bad happened.
3 participants