Skip to content

Support S3-compatible Pulp storage - #418

Open
jakduch wants to merge 1 commit into
theforeman:masterfrom
jakduch:feature/s3-storage
Open

jakduch wants to merge 1 commit into
theforeman:masterfrom
jakduch:feature/s3-storage

Conversation

@jakduch

@jakduch jakduch commented Oct 2, 2026 •

Copy link
Copy Markdown

Summary

Add optional S3-compatible content storage through storage_backend and storage_options. Filesystem content storage remains the default.

Install the stable pulpcore-storage-s3 capability, pass storage options to django-storages, preserve Django's local static-file backend, and omit MEDIA_ROOT and the unused media directory for S3.

Dependency

The prerequisites pulpcore-packaging#3186 and #3189 have merged. The S3 metapackage is now published in the nightly EL9 and EL10 repositories. This branch was rebased onto current master and acceptance CI restarted against that package availability.

Testing

  • Unit coverage checks the filesystem defaults and generated S3 settings.
  • Acceptance coverage applies the S3 configuration twice, verifies the package and settings, and runs pulpcore-manager check. It checks configuration and dependency loading, not an S3 upload/download round trip.
  • The nightly selection uses BEAKER_FACTER_pulpcore_version, matching the CI matrix. It defaults to nightly for local runs.
  • Ruby syntax and git diff --check passed locally. Full unit and acceptance tests run in upstream CI; the local bundle is not installed. Static validation has passed; the acceptance matrix is still queued or running.

AI assistance: Codex. Project signature: GPT_5.6 Sol.

@ekohl ekohl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This reads well. Perhaps you could also add an acceptance test that exercises the new code paths? Or would that be too hard?

@jakduch

jakduch commented Oct 3, 2026

Copy link
Copy Markdown
Author

Added in c882d2a. The acceptance test applies the S3 configuration twice, verifies the package and generated settings, and runs pulpcore-manager check.

It is limited to the nightly repository because pulpcore-storage-s3 is introduced by theforeman/pulpcore-packaging#3189 and is not available in the older stable repositories.

@alexjfisher

Copy link
Copy Markdown
Contributor

I can take a look tomorrow too. I'm using S3, and was doing some bits in my profile code around it, that I was planning to turn into some upstream module patch at some stage.

Comment thread README.md Outdated
Comment thread manifests/init.pp
Comment thread README.md

@alexjfisher alexjfisher left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. I can't see anything I currently have fudged into my profile that couldn't be replaced when this goes into the module.

Comment thread spec/acceptance/storage_spec.rb Outdated
@@ -0,0 +1,38 @@
require 'spec_helper_acceptance'

if fact('pulpcore_version') == 'nightly'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The tests don't fail so I think it isn't being executed now. The fact is probably not set.

@jakduch jakduch Oct 15, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

You're right: the S3 examples were skipped. Fixed in b39d3e3 by reading BEAKER_FACTER_pulpcore_version, which the CI matrix actually exports. The default local value is nightly as well.

I also corrected the installed-package assertion: pulpcore-storage-s3 is an RPM capability, so the test now uses rpm -q --whatprovides rather than treating it as a package name. Nightly acceptance still requires the package from theforeman/pulpcore-packaging#3189 to be published.

Ruby syntax passes; the full acceptance run is in CI.

@jakduch
jakduch force-pushed the feature/s3-storage branch 2 times, most recently from a7ff9e6 to b39d3e3 Compare October 15, 2026 15:29
Install the Pulpcore S3 package capability and configure Django storage while preserving filesystem defaults and local static assets. Select nightly acceptance coverage through the BEAKER_FACTER_pulpcore_version environment value supplied by the test matrix.

AI-Assisted-By: GPT_5.6 Sol (project signature)
AI-Tool: Codex (GPT-6)
@jakduch
jakduch force-pushed the feature/s3-storage branch from b39d3e3 to 18b26e5 Compare October 20, 2026 14:33
@jakduch

jakduch commented Oct 20, 2026

Copy link
Copy Markdown
Author

The prerequisite pulpcore-packaging#3189 is merged, and the S3 metapackage is published in both EL9 and EL10 nightly repositories. Refreshed the branch in 18b26e5 to rerun the acceptance matrix against the available package; the S3 implementation is unchanged.

@ekohl ekohl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I suspect you'll also run into theforeman/pulpcore-packaging#3259 so perhaps you can wait with updating this PR until it's been resolved. That saves some CI resources.

@@ -0,0 +1,38 @@
require 'spec_helper_acceptance'

if ENV.fetch('BEAKER_FACTER_pulpcore_version', 'nightly') == 'nightly'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm still not quite sure about this. It will mean we won't test everything automatically when we add new versions. How about we skip it on known incompatible versions and then list it as a known limitation on https://github.com/theforeman/puppet-pulpcore#support-policy?

Suggested change
if ENV.fetch('BEAKER_FACTER_pulpcore_version', 'nightly') == 'nightly'
# Older versions don't ship s3 support
unless %w[3.63 3.73 3.85 3.105].include?(ENV['BEAKER_FACTER_pulpcore_version'])

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I have prepared this locally: the S3 tests skip only 3.63, 3.73, 3.85 and 3.105, so newly added versions are included automatically. The support policy documents that these Foreman package repositories lack the required pulpcore-storage-s3 capability, rather than implying that Pulp itself cannot use S3. Ruby syntax and the version-selection check passed. I am holding the push until theforeman/pulpcore-packaging#3259 is resolved to avoid spending CI resources on the known packaging blocker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Waiting for review in other projects

Development

Successfully merging this pull request may close these issues.

3 participants