Skip to content

Improve handling of the _installing file #747

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

Merged
merged 4 commits into from
Jun 16, 2025

Conversation

Rade333
Copy link
Contributor

@Rade333 Rade333 commented May 9, 2025

This pull request refactors the handling of the _installing file in the charts/drupal/templates/_helpers.tpl file to improve maintainability and ensure consistent behavior across multiple commands. The changes centralize the definition of the _installing file path, replace hardcoded paths with a reusable template, and add safeguards for file removal operations.

Motivation

  • In some rare cases the feature environment creation is successful but the _installing file get stuck, causing the Drupal pod to never transition to Ready state
  • In other rare cases the _installing file is missing and the rm command errors out and fails the whole deployment

Refactoring _installing File Handling:

  • Introduced a reusable template drupal.installing-file to centralize the definition of the _installing file path, replacing hardcoded paths throughout the file.
  • Updated the drupal.installation-in-progress-test definition to use the new drupal.installing-file template for consistency.

Improvements to File Operations:

  • Added a step to remove the _installing file at the beginning of the drupal.data-pull-command and drupal.post-release-command definitions, ignoring errors if the file does not exist, to clean up state from potential previous failed runs. [1] [2]
  • Replaced hardcoded rm and touch operations with variables referencing the drupal.installing-file template for better maintainability and readability. [1] [2] [3]

@Rade333 Rade333 requested a review from agnis-mateuss May 9, 2025 10:22
@Rade333 Rade333 requested a review from k4lv15 June 2, 2025 07:26
@k4lv15 k4lv15 self-assigned this Jun 4, 2025
Copy link
Contributor

@k4lv15 k4lv15 left a comment

Choose a reason for hiding this comment

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

Nice, looks good 👍 Added a comment tho, please see if it's valid.


# Attempt to remove the _installing file at the very beginning, ignoring errors if it doesn't exist.
# This cleans up state from a potential previous failed install run.
rm -f "$INSTALLING_FILE"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add || true also here to prevent potential erroring out? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@k4lv15 the -f takes care of that, it does not fail even if the file does not exist :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh, okays, just noticed it's added in the other place: https://github.com/wunderio/drupal-project-k8s/pull/747/files#diff-c4f491ae4ef2d83e98019bc3ac28e9ba383d1ec3573c7f25e65e1871f0f13821R436
Was wondering if we need it here as well. If not, all good then!

@Rade333 Rade333 requested a review from k4lv15 June 12, 2025 09:17
@Rade333 Rade333 merged commit b651517 into master Jun 16, 2025
1 check passed
@Rade333 Rade333 deleted the feature/improve-installing-file branch June 16, 2025 11:15
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