Skip to content

[Develop] Add data integrity checks before removing backup directories #2727

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
May 28, 2024
Merged

[Develop] Add data integrity checks before removing backup directories #2727

merged 13 commits into from
May 28, 2024

Conversation

@hehe7318
Copy link
Contributor

@hehe7318 hehe7318 commented May 20, 2024 •
edited
Loading

Description of changes

  • Updated restore_home_shared_data.rb to include a diff check before removing /tmp/home/ directory.

    • Ensures data in the new home directory is the same as the original before deleting the temporary backup.

  • Updated restore_internal_use_shared_data.rb to include a diff check before removing /tmp directories for each internal shared directory.

    • Ensures data in the new directory is the same as the original before deleting the temporary backup.

  • Updated config_default_user_home.rb to include data integrity checks during the process of moving the cluster user's default home directory.

  • Added detailed comments to explain the new data integrity checks.

  • Added new Spec tests.

  • This update prevents potential data loss or inconsistency during the restoration process.

Tests

  • Added Spec tests
  • Spec tests passed successfully
  • Kitchen tests passed successfully
  • Manual Test for changes in config_default_user_home.rb:
    • Conducted manual tests to verify the new integrity check functionality.
    • Happy Path: Manually run config_default_user_home recipe. Ensured the home directory is moved correctly when no data integrity issues are present.
    • Unhappy Path: Simulated a failure in the data integrity check by creating expected node['cluster']['cluster_user_local_home'] and modifying files in the expected directory to differ from the original. Manually run config_default_user_home recipe. Verified that the script fails and outputs the correct error message in chef-client.log.
    • Results:
      • Happy path: Passed.
      • Unhappy path: Passed, with expected failure and error message.

Checklist

  • Make sure you are pointing to the right branch.
  • If you're creating a patch for a branch other than develop add the branch name as prefix in the PR title (e.g. [release-3.6]).
  • Check all commits' messages are clear, describing what and why vs how.
  • Make sure to have added unit tests or integration tests to cover the new/modified code.
  • Check if documentation is impacted by this change.

Please review the guidelines for contributing and Pull Request Instructions.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@hehe7318
feat: Add data integrity checks before removing backup directories
e837707 
- Updated restore_home_shared_data.rb to include a diff check before removing /tmp/home/ directory.
  - Ensures data in the new home directory is the same as the original before deleting the temporary backup.
- Updated restore_internal_use_shared_data.rb to include a diff check before removing /tmp directories for each internal shared directory.
  - Ensures data in the new directory is the same as the original before deleting the temporary backup.
- Added detailed comments to explain the new data integrity checks.

This update prevents potential data loss or inconsistency during the restoration process.
@hehe7318 hehe7318 requested review from a team as code owners May 20, 2024 05:24
@hehe7318 hehe7318 added the skip-recursive-deletion-check Skip the checks regarding the use of recursive deletion. label May 20, 2024
gmarciani
gmarciani requested changes May 20, 2024
View reviewed changes
hehe7318 and others added 2 commits May 20, 2024 12:38
@hehe7318
Merge branch 'develop' into wip/codebase-deletion-review
9d070b6
@hehe7318
Add data integrity check when moving the default home directory, upda…
f0dc2ad 
…te error messages and update spec tests

- Modified 'config_default_user_home.rb' to include data integrity checks during the process of moving the cluster user's default home directory. This ensures the data in the new directory matches the original before deletion.
- Updated error message when data integrity checks fail to show the difference.
- Enhanced tests in 'config_default_user_home_spec.rb' to verify the data integrity check logic in the 'config_default_user_home.rb' recipe.
- Enhanced tests in 'mount_internal_use_efs_spec.rb' to verify the data integrity check logic in the 'restore_internal_use_shared_data.rb' recipe.
hehe7318 added 2 commits May 21, 2024 22:56
@hehe7318
Add the manual test of the unhappy case step and result in the descri…
d9844d0 
…ption comments. Output error messages to stderr to ensure we can see it in chef-client.log. Add restart sshd service in config_default_user_home.rb, or after failed, user will can not ssh into the instance.
@hehe7318
Modify the diff command to compare cluster_user_home and cluster_user…
366af3a 
…_local_home
@hehe7318 hehe7318 force-pushed the wip/codebase-deletion-review branch from e8980d1 to 366af3a Compare May 22, 2024 15:01
@hehe7318 hehe7318 enabled auto-merge (squash) May 28, 2024 14:21
dreambeyondorange
dreambeyondorange approved these changes May 28, 2024
View reviewed changes
Copy link
Contributor

@dreambeyondorange dreambeyondorange left a comment

Choose a reason for hiding this comment

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

LGTM!

@hehe7318 hehe7318 merged commit a252e53 into aws:develop May 28, 2024
@hehe7318 hehe7318 mentioned this pull request Jun 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gmarciani gmarciani gmarciani approved these changes

+1 more reviewer

@dreambeyondorange dreambeyondorange dreambeyondorange approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

3.x skip-changelog-update skip-recursive-deletion-check Skip the checks regarding the use of recursive deletion.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hehe7318 @gmarciani @dreambeyondorange