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

fixes #28655 - support fetching files via /pulp/isos with pulp3 #228

Merged
merged 1 commit into from
Jan 17, 2020

Conversation

wbclark
Copy link
Contributor

@wbclark wbclark commented Jan 10, 2020

This is a WIP. Currently it should disable isos for pulp2 when $enable_pulpcore_file but it does not yet add the vhost entry for proxying /pulp/isos to the pulpcore content app

@wbclark wbclark changed the title [WIP] fixes #28655 - support fetching files via /pulp/isos with pulp3 fixes #28655 - support fetching files via /pulp/isos with pulp3 Jan 13, 2020
@wbclark
Copy link
Contributor Author

wbclark commented Jan 13, 2020

Looking for feedback on the right place to set $enable_pulpcore_file to true -- whether that should just be default here or explicitly called in another repo.

@ehelms
Copy link
Member

ehelms commented Jan 13, 2020

The default should be true going forward, in a Katello upgrade scenario we will need this value t be false. However, that will be confusing to users of this module potentially. As we need to support:

  • Pulp 2 and Pulp 3 running with File support
  • Apache configured to proxy /pulp/isos to Pulp 3 content app instead of sending /pulp/isos to Pulp 2

I think the second bullet will need a specific parameter to do what your code is doing here and add an entry to the vhost for proxying

@wbclark
Copy link
Contributor Author

wbclark commented Jan 13, 2020

Thanks @ehelms . I think the second bullet should be adequately addressed by theforeman/puppet-pulpcore#55 which should also be enabled once $enable_pulpcore_file is set to true here.

About the first bullet point, I think it's adequately solved as well if I understand correctly the existing $enable_iso and $enable_file params. The new $enable_pulpcore_file won't affect the existing $enable_file but it will override $enable_iso to be always false. So IIUC, this leaves the possibility for the pulp2 file module to be enabled but not ISOs specifically. Please let me know if I got that correct.

@wbclark wbclark force-pushed the pulp_isos_path branch 2 times, most recently from 6b62261 to a3916eb Compare January 13, 2020 21:13
@wbclark
Copy link
Contributor Author

wbclark commented Jan 13, 2020

The latest iteration changes the name of the new parameter to $proxy_pulp_isos_to_pulpcore to make it clear that this isn't responsible for whether the file plugin is enabled in pulpcore.

Additionally, the actual ProxyPass logic is moved into this module since the /pulp/isos path is a Katello-ism that doesn't exist in standalone pulp{,core}

@wbclark wbclark force-pushed the pulp_isos_path branch 8 times, most recently from ae42946 to c42d15c Compare January 14, 2020 03:01
@wbclark
Copy link
Contributor Author

wbclark commented Jan 14, 2020

Added tests and I believe those should be working as expected after the latest push.

manifests/init.pp Outdated Show resolved Hide resolved
@wbclark wbclark force-pushed the pulp_isos_path branch 2 times, most recently from a2cd9d0 to 9c3dffb Compare January 16, 2020 19:34
Adds ProxyPass{,Reverse} delcarations for /pulp/isos -> /pulp/content to Foreman HTTPS vhost
@ehelms ehelms merged commit 1761805 into theforeman:master Jan 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants