Skip to content

Support s3 storage #379

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
+46 −0
Draft

Support s3 storage #379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions manifests/config.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,20 @@
order => '02',
}

if $pulpcore::storage_backend == 's3' {
$storages = {
'default' => {
'BACKEND' => 'storages.backends.s3.S3Storage',
'OPTIONS' => $pulpcore::storage_options,
},
}
concat::fragment { 'storage':
target => 'pulpcore settings',
content => "STORAGES = ${stdlib::to_python($storages)}\n",
order => '03',
}
}

file { $pulpcore::user_home:
ensure => directory,
owner => $pulpcore::user,
Expand Down
8 changes: 8 additions & 0 deletions manifests/init.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,12 @@
# What percentage of available-workers will pulpcore use for import tasks at a time.
# By default, pulpcore will use all available workers.
#
# @param storage_backend
# Which storage backend to use
#
# @param storage_options
# Options to pass to the storage backend
#
# @example Default configuration
# include pulpcore
#
Expand Down Expand Up @@ -268,6 +274,8 @@
Optional[Boolean] $analytics = undef,
Optional[Boolean] $hide_guarded_distributions = undef,
Optional[Integer[1,100]] $import_workers_percent = undef,
Enum['file', 's3'] $storage_backend = 'file',
Hash[String[1], Any] $storage_options = {},
) {
$settings_file = "${config_dir}/settings.py"
$certs_dir = "${config_dir}/certs"
Expand Down
7 changes: 7 additions & 0 deletions manifests/install.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@
}
}

if $pulpcore::storage_backend != 'file' {
# TODO: better virtual for this
package { 'python3.11-django-storages':
Comment on lines +16 to +17
Copy link
Member Author

Choose a reason for hiding this comment

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

@Odilhao any thoughts on how we should tackle this in packaging? I thought about a pulpcore-storages subpackage or multiple to reflect the extras in https://github.com/pulp/pulpcore/blob/528efde666ef49b6a3f0815e6084419750ee2314/pyproject.toml#L74-L77.

Copy link
Member Author

Choose a reason for hiding this comment

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

theforeman/pulpcore-packaging#1760

Copy link
Member

Choose a reason for hiding this comment

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

We can use subpackages as we use for cachecontrol

%description -n python%{python3_pkgversion}-%{srcname}+filecache
This is a metapackage bringing in filecache extra requires for python%{python3_pkgversion}-%{srcname}
It contains no code, just makes sure the dependencies are installed.

%files -n python%{python3_pkgversion}-%{srcname}+filecache
%ghost %{python3_sitelib}/%{srcname}-%{version}.dist-info/

This code will generate the following package:
Provides: python3.12-cachecontrol+filecache = 0.14.2-4.el9 python3.12dist(cachecontrol[filecache]) = 0.14.2

We can use the rich decency package in this case python3.12dist(cachecontrol[filecache]) or in pulpcore logic
python3.12dist(pulpcore[s3])

For pulpcore I see the package like this:

%package -n python%{python3_pkgversion}-%{pypi_name}+s3
Summary: Metapackage for python%{python3_pkgversion}-%{pypi_name}: s3 extra
Requires: python%{python3_pkgversion}-django-storage+s3 >= 3.8.0

%description -n python%{python3_pkgversion}-%{pypi_name}+s3
This is a metapackage bringing in s3 extra requires for python%{python3_pkgversion}-%{pypi_name}
It contains no code, just makes sure the dependencies are installed.

%files -n python%{python3_pkgversion}-%{pypi_name}+s3
%ghost %{python3_sitelib}/%{pypi_name}-%{version}.dist-info/

This will also require us to add the extra package into django-storage.

1 - https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#Extras

Copy link
Member Author

Choose a reason for hiding this comment

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

I like the idea of converting the extras but it still contains the Python version in the name and I'd like to avoid that. Any thought on adding a Provides: pulpcore-storage-s3 on it that packaging could depend on?

Copy link
Member

Choose a reason for hiding this comment

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

I don't see a problem with Provides: pulpcore-storage-s3 we can add that to the metapackage

ensure => present,
}
}

user { $pulpcore::user:
ensure => present,
system => true,
Expand Down
17 changes: 17 additions & 0 deletions spec/classes/pulpcore_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
is_expected.to contain_class('pulpcore::install')
is_expected.to contain_package('pulpcore')
is_expected.to contain_package('pulpcore-selinux')
is_expected.not_to contain_package('python3.11-django-storages')
is_expected.to contain_user('pulp').with_gid('pulp').with_home('/var/lib/pulp')
is_expected.to contain_group('pulp')
end
Expand Down Expand Up @@ -49,6 +50,7 @@
},
}
LOGGING
is_expected.not_to contain_concat__fragment('storage')
is_expected.to contain_file('/etc/pulp')
is_expected.to contain_file('/etc/pulp/certs/database_fields.symmetric.key')
is_expected.to contain_file('/var/lib/pulp')
Expand Down Expand Up @@ -597,6 +599,21 @@
end
end

context 'with s3 storage' do
let :params do
{ storage_backend: 's3' }
end

it { is_expected.to compile.with_all_deps }
it { is_expected.to contain_package('python3.11-django-storages') }

it 'configures pulpcore with setting STORAGES' do
is_expected.to contain_concat__fragment('storage').with_content(<<~STORAGE)
STORAGES = {"default": {"BACKEND": "storages.backends.s3.S3Storage", "OPTIONS": {}}}
STORAGE
end
end

context 'with parameter worker_ttl = 60' do
let :params do
{ worker_ttl: 60 }
Expand Down
Loading