-
Notifications
You must be signed in to change notification settings - Fork 36
Conversation
Tested against a vanilla F25 system where:
I tested this code by rebasing on upstream/3.0-dev and executing The playbook succeeded, and |
b7cecf7
to
d3ecaa7
Compare
Replace use of db-reset with pulp-manager, which will create migrations for each plugin and migrate. re: #3012
d3ecaa7
to
257274e
Compare
To me merged with pulp/pulp#3163 |
@@ -20,8 +20,8 @@ | |||
- broker | |||
- lazy | |||
- dev | |||
- plugins | |||
- reset_db | |||
- {role: plugin, plugin_name: 'pulp_file', app_label: 'pulp_file'} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey so I found an issue with this. If you provide the names of the plugins up-front instead of letting them be enumerated, this fails if the plugin isn't in the user's devel directory.
Specifically, I forgot to clone the pulp_file plugin, and the ansible script failed out with
TASK [plugin : Install Plugin packages into core Pulp Python environment] ******
fatal: [pulp3_dev]: FAILED! => {"changed": false, "cmd": "/home/vagrant/.virtualenvs/pulp/bin/pip3 install -e /home/vagrant/devel/pulp_file", "failed": true, "msg": "\n:stderr: /home/vagrant/devel/pulp_file should either be a path to a local project or a VCS url beginning with svn+, git+, hg+, or bzr+\nYou are using pip version 8.1.2, however version 9.0.1 is available.\nYou should consider upgrading via the 'pip install --upgrade pip' command.\n"}
I assume we want to keep the previous workflow where you don't necessarily need to have the plugins cloned - but let me know if that assumption is invalid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have 2 choices. We can use the task to assert that the repo exists:
https://github.com/pulp/devel/blob/3.0-dev/ansible/roles/common/tasks/require_repo.yml
Or we can leave this role commented out as the default. I'm inclined to go that route. Pretty sure you could clone, uncomment and run vagrant provision
.
There was a problem hiding this comment.
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 letting the presence of pulp_file be optional, which would mean having this line of code commented out by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fair, although it means that devs would need to uncomment/list all plugins they want to be installed themselves, which is probably fine at this point in the Pulp 3 lifecycle. I feel like we would eventually want to go back to the old model installing everything it could find, but maybe now isn't the time to worry about it.
Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True enough. I do like the idea of making plugin installation as seamless and easy as possible. We both want the same end goal.
I want to get to a place where we can install pulpcore (with development extras) with a single button press as soon as possible. This will be much easier to do if we start by putting a pulpcore-only playbook in front of the user by default.
EDIT: One specific possible solution is to create a role (or roles) for installing plugins, and to make that role(s) call this role after the plugin(s) is installed. That would place the logic for installing the plugin near the logic for running that plugin's migrations. Regardless of how exactly it happens, the end procedure is the same: get one-button pulpcore installations working now, and get one-button plugin installations working later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Long story short, we were already hardcoding pulp_file into the ansible install, so for now, I think its best to move that out of the role and into the playbook.
Id like to comment out the pulp_file install in the default playbook and use require_repo to fail with an easily readable message. (Probably also a small docs change). That seems like the safest default to me, and we can continue this conversation on the pulp-dev list to align our longer term ideas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me. Let me know when those changes have been made and I will take another look
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Long story short, we were already hardcoding pulp_file into the ansible install, so for now, I think its best to move that out of the role and into the playbook.
This step?
TASK [plugins : Install Plugin packages into core Pulp Python environment] ********************************************
[WARNING]: when statements should not include jinja2 templating delimiters such as {{ }} or {% %}. Found: {{ [item] | issubset(pulp_available_plugins) }}
skipping: [fedora-25-pulp3-pypi] => (item=pulp_file)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ichimonji10, yes, and that part also depends on a hardcoded list in pulp_facts. I prefer that if someone wants to modify their dev environment to install their new plugin, they should only need to change the playbook.
Tested against an almost-vanilla Fedora 25 VM, the only modification being SSH configuration being in place. Installed with:
I had to re-run it several times, whenever Ansible complained about a repository in |
https://pulp.plan.io/issues/3012