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

Install object storage support (azure/s3) #571

Merged
merged 1 commit into from Apr 6, 2021
Merged

Install object storage support (azure/s3) #571

merged 1 commit into from Apr 6, 2021

Conversation

fao89
Copy link
Member

@fao89 fao89 commented Mar 30, 2021

@pulpbot
Copy link
Member

pulpbot commented Mar 30, 2021

Attached issue: https://pulp.plan.io/issues/8446

pulp_settings:
secret_key: secret
content_origin: "{{ pulp_webserver_disable_https | default(false) | ternary('http', 'https') }}://{{ ansible_fqdn }}"
aws_access_key_id = 'AKIAIT2Z5TDYPX3ARJBA'
Copy link
Member

Choose a reason for hiding this comment

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

Hope, those aren't real creds.

Copy link
Member Author

Choose a reason for hiding this comment

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

@@ -6,6 +6,7 @@ pulp_config_dir: '/etc/pulp'
# foreseeable need for them to change the filename, only pulp_config_dir.
pulp_settings_file: '{{ pulp_config_dir }}/settings.py'
pulp_install_source: pip
pulp_install_object_storage: filesystem
Copy link
Member

Choose a reason for hiding this comment

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

We could add pretasks with assertions that the selected storage solution is configured.
roughly:

- name: Check storage backend is configured.
  assert:
    that:
      - aws_storage_bucket_name is defined
      - <...>
  when: pulp_install_object_storage == "s3"

Copy link
Contributor

@melcorr melcorr left a comment

Choose a reason for hiding this comment

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

Just one comment!


Storage
-------
For enabling s3/azure storage please refer to [our docs](https://docs.pulpproject.org/pulpcore/installation/storage.html) and the [example playbooks](https://github.com/pulp/pulp_installer/tree/master/playbooks).
Copy link
Contributor

@melcorr melcorr Mar 31, 2021

Choose a reason for hiding this comment

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

Suggested change
For enabling s3/azure storage please refer to [our docs](https://docs.pulpproject.org/pulpcore/installation/storage.html) and the [example playbooks](https://github.com/pulp/pulp_installer/tree/master/playbooks).
To set up and configure Amazon S3 or Azure storage to use with Pulp, follow the storage configuration instructions in [our docs](https://docs.pulpproject.org/pulpcore/installation/storage.html). View the [example playbooks](https://github.com/pulp/pulp_installer/tree/master/playbooks) and note the variables for your chosen storage type. Add an entry for each of the variables for your deployment.

What do they have to do with the example playbooks? This is unclear to me!

Edit: I updated my example, and would say to give an example set, as you've highlighted below. I'm not sure how to finish this sentence "Add these values to your playbook for your storage type", or something?

Comment on lines 21 to 27
azure_account_name = 'Storage account name'
azure_container = 'Container name (as created within the blob service of your storage account)'
azure_account_key = 'Key1 or Key2 from the access keys of your storage account'
azure_url_expiration_secs = 60
azure_overwrite_files = 'True'
azure_location = 'the folder within the container where your pulp objects will be stored'
default_file_storage = 'storages.backends.azure_storage.AzureStorage'
Copy link
Member Author

Choose a reason for hiding this comment

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

@melcorr actually the idea was having the user to see they need to set specific variables to their playbooks

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for clarifying @fao89 - I would suggest telling them specifically what to do. I updated my example, but it's probably slightly incorrect. So, look in the example playbook, understand the information you need to gather to complete the variables, and add that information (where?) to your playbook.

Does that sound better?

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 think I may need to explicitly mention the variables, I'll work on it

Copy link
Contributor

Choose a reason for hiding this comment

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

If I were writing this, I would add a step per variable and tell them what to do. If you want, we can meet and work on this together. It might be quicker.

Comment on lines 22 to 30
aws_secret_access_key = 'qR+vjWPU50fCqQuUWbj9Fain/j2pV+ZtBCiDiieS'
aws_storage_bucket_name = 'pulp3'
aws_default_acl = "@none None"
s3_use_sigv4 = True
aws_s3_signature_version = "s3v4"
aws_s3_addressing_style = "path"
aws_s3_region_name = "eu-central-1"
default_file_storage = 'storages.backends.s3boto3.S3Boto3Storage'
media_root = ''
Copy link
Member Author

Choose a reason for hiding this comment

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

like these variables for S3 for example, but I don't know how to communicate it well on the docs

@fao89 fao89 force-pushed the 8446 branch 4 times, most recently from 6c08ec0 to 58dbdd6 Compare April 5, 2021 20:42
Copy link
Member

@mdellweg mdellweg left a comment

Choose a reason for hiding this comment

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

LGTM

Still it is a bit strange, that we use real looking examples for aws, while there are more descriptive values in the azure example.

@fao89
Copy link
Member Author

fao89 commented Apr 6, 2021

LGTM

Still it is a bit strange, that we use real looking examples for aws, while there are more descriptive values in the azure example.

I just followed the other doc, but I can change it here

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.

None yet

4 participants