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

CASMCMS-9201 - Fix orphaned artifacts in S3. #155

Merged
merged 2 commits into from
Jan 29, 2025
Merged

Conversation

dlaine-hpe
Copy link
Contributor

Summary and Scope

The 'soft delete' of IMS objects used the S3 'copy_from' command. This command does the copy in a single atomic operation but is limited to artifacts less than 5Gb in size. When the rootfs files got bigger than 5Gb, the copy into the 'deleted objects' location would fail, leaving them behind. Then when the 'deleted image' was deleted, these artifacts were not removed from S3.

The fix is to use the S3 'copy' command which uses a multi-part copy if the object is too larger for the single operation copy. There is an issue with S3/boto3 where the multi-part copy fails if the artifact is owned by a different user than the 'IMS' user. I had to put code in to handle the copy differently if the artifact is owned by 'STS' which is the case if 'cray artifacts create' is used to load the artifact into S3.

Issues and Related PRs

Testing

Tested on:

  • Surtur

Test description:

The new service was upgraded via helm chart and verified that the delete / undelete / hard delete worked as expected with all size artifacts and the 'STS' owner.

  • Were the install/upgrade-based validation checks/tests run (goss tests/install-validation doc)? N
  • Were continuous integration tests run? If not, why? N
  • Was upgrade tested? If not, why? Y
  • Was downgrade tested? If not, why? Y
  • Were new tests (or test issues/Jiras) created for this change? N

Risks and Mitigations

This should be fairly low risk.

Pull Request Checklist

  • Version number(s) incremented, if applicable
  • Copyrights updated
  • License file intact
  • Target branch correct
  • CHANGELOG.md updated
  • Testing is appropriate and complete, if applicable

@dlaine-hpe dlaine-hpe requested a review from a team as a code owner January 27, 2025 21:20
src/server/app.py Outdated Show resolved Hide resolved
src/server/app.py Outdated Show resolved Hide resolved
@dlaine-hpe dlaine-hpe merged commit 6bb507e into develop Jan 29, 2025
4 checks passed
@dlaine-hpe dlaine-hpe deleted the CASMCMS-9201 branch January 29, 2025 20:18
@dlaine-hpe dlaine-hpe mentioned this pull request Jan 29, 2025
6 tasks
S3_STS_ENDPOINT = os.getenv('S3_STS_ENDPOINT')
S3_STS_ACCESS_KEY = os.getenv('S3_STS_ACCESS_KEY')
S3_STS_SECRET_KEY = os.getenv('S3_STS_SECRET_KEY')
S3_STS_SSL_VALIDATE = \
Copy link
Contributor

Choose a reason for hiding this comment

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

You're hashing all of these potential false values to False, but you're not doing the same for, presumably, the true values. Why not? You're just taking whatever is in that environment variable.

# use a multi-part copy operation. This is automatically handled by the Object.copy
# function. The multi-part copy requires greater levels of permissions than the single
# transfer copy. It will currenly not work if the owner of the object is something
# other than IMS. When an object is copied into S3 via 'cray artifacts create ...' it
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# other than IMS. When an object is copied into S3 via 'cray artifacts create ...' it
# other than IMS. When an object is copied into S3 via 'cray artifacts create ...', it

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.

5 participants