Rewrite the role #6

pieterlexis · 2018-02-19T12:21:07Z

Hi!

I had some discussions with you folks at cfgmgmtcamp 2 weeks ago and would love to put pulp 3 into production somewhere this year (especially when the debian module is ready).

In preparation, I looked at this role and the other ansible roles for pulp3. I noticed they were (in my opinion) crossing some role boundaries (like installing postgres in the _db role) and we not configurable.

I've rewritten this role, incorporated the _systemd and _db roles into it and added many configuration options.

By default, this role now uses sqlite3 as a database (so no separate daemon is needed on a test-installation), installs the systemd unitfiles, starts services only when configured to do so, add Debian 9 support, allows enabling the webserver and adds molecule-based tests that are run in Travis.

The role is still Work in Progress, as I did not have a chance to actually deploy pulp3 apart from running the tests. This also why there is no updated README yet.

I open this PR already to allow for feedback on the style, structure, idea, and implementation.

Best regards,

Pieter Lexis

pieterlexis · 2018-02-19T22:28:24Z

I think I had to poke @bmbouter about this 😄

dkliban · 2018-02-20T02:21:00Z

Thanks for the PR. @bmbouter is traveling, but I am looking at this PR. Molecule looks really cool. I am going to try this out.

dkliban · 2018-02-20T03:44:55Z

When I tried to run 'molecule test' I got the following error:

--> Action: 'create'
    
    PLAY [Create the Molecule Test Resources] **************************************
    
    TASK [set_fact] ****************************************************************
    ok: [localhost]
    
    TASK [set_fact] ****************************************************************
    ok: [localhost]
    
    TASK [Create Dockerfiles for the target Platforms] *****************************
    failed: [localhost] (item={'dockerfile_tpl': u'centos-systemd', 'image': u'fedora:27', 'name': u'fedora-27'}) => {"changed": false, "checksum": "1c55240bd9658fc6e73e0c3727097cbedd9f0ea4", "item": {"dockerfile_tpl": "centos-systemd", "image": "fedora:27", "name": "fedora-27"}, "msg": "Aborting, target uses selinux but python bindings (libselinux-python) aren't installed!"}
    failed: [localhost] (item={'dockerfile_tpl': u'debian-systemd', 'image': u'debian:9', 'name': u'debian-9'}) => {"changed": false, "checksum": "388936ff83b3654739b69aeb25b9416e162b9363", "item": {"dockerfile_tpl": "debian-systemd", "image": "debian:9", "name": "debian-9"}, "msg": "Aborting, target uses selinux but python bindings (libselinux-python) aren't installed!"}
    
    PLAY RECAP *********************************************************************
    localhost                  : ok=2    changed=0    unreachable=0    failed=1

dralley · 2018-02-20T04:39:06Z

Ansible requires libselinux-python to run on nodes with selinux enabled.

See: http://docs.ansible.com/ansible/latest/intro_installation.html#managed-node-requirements

If you have SELinux enabled on remote nodes, you will also want to install libselinux-python on them before using any copy/file/template related functions in Ansible. You can of course still use the yum module in Ansible to install this package on remote systems that do not have it.

pieterlexis · 2018-02-20T12:46:32Z

When I tried to run 'molecule test' I got the following error:

I think the failure has to do with the tests being run from an EL system that does not have this package installed, as the creation of the Docker image fails (and that is done locally with Ansible).

I'll install this package via the role anyway, as the targets will indeed need it.

dkliban · 2018-02-20T13:11:26Z

This does not resolve the problem. I am not sure that I completely understand where this failure is occurring. It looks to me like it's the step where a dockerfile is being created from a template. However, I might be not fully understanding what else molecule is doing.

What system are you using to test this?

pieterlexis · 2018-02-20T13:30:19Z

What system are you using to test this?

Arch Linux, and the tests also run on Travis (Ubuntu Trusty)

However, I might be not fully understanding what else molecule is doing.

I think this has to do with the SELinux modules not being in the virtualenv that is created for molecule+ansible. Looks like you have this issue.

dkliban · 2018-02-21T02:28:27Z

This is awesome!

I used the role to install Pulp 3 on Fedora 27. Everything seemed to install correctly. However, when I went to use pulp-manager command I got an error.

source /opt/pulp/bin/activate
pulp-manager
pulp-manager

Type 'pulp-manager help <subcommand>' for help on a specific subcommand.

Available subcommands:

[django]
    check
    compilemessages
    createcachetable
    dbshell
    diffsettings
    dumpdata
    flush
    inspectdb
    loaddata
    makemessages
    makemigrations
    migrate
    runserver
    sendtestemail
    shell
    showmigrations
    sqlflush
    sqlmigrate
    sqlsequencereset
    squashmigrations
    startapp
    startproject
    test
    testserver
Note that only Django core commands are listed as settings are not properly configured (error: The SECRET_KEY setting must not be empty.).

What am I doing wrong? The services seem to be working correctly and /etc/pulp/server.yml has the SECRET_KEY set.

Also, I reran my playbook and set pulp_install_plugins: ['pulp_file']. The plugin got installed but the migrations for it did not get run.

pieterlexis · 2018-02-21T09:29:16Z

This was to be expected, as I did not install pulp myself yet to run into issues with the role.

What am I doing wrong? The services seem to be working correctly and /etc/pulp/server.yml has the SECRET_KEY set.

Are you running this as the pulp user? The /etc/pulp directory (and server.yml file) are set to disallow access from everybody (owned by pulp:pulp and directory permissions are 750).

Also, I reran my playbook and set pulp_install_plugins: ['pulp_file']. The plugin got installed but the migrations for it did not get run.

Will fix.

Ichimonji10 · 2018-02-23T21:26:24Z

Thanks for this PR, @pieterlexis. I'm excited to take a look at this PR. I'm going to give this a test drive and think about its design early next week, on Monday or Tuesday. Please excuse the delay.

Ichimonji10 · 2018-03-01T19:32:07Z

How does one actually use this re-written code? The readme states the following:

Example Playbook
----------------

https://raw.githubusercontent.com/pulp/devel/3.0-dev/ansible/deploy-pulp3.yml

I think the linked-to playbook is out of date, though. After all, this PR merges the functionality in ansible-pulp3_db and ansible-pulp3_systemd into this role, right? I think the readme needs to be re-written.

Squash the commits in this PR down into one big commit, unless each individual change produces a still-valid set of code. I've only spent a little time looking through the commits, but my impression is that this role will break as a result of applying only some commits. And some of the commits change what's done in a previous commit.

EDIT: While I'm at it, please make sure to re-write the readme in its entirety. Document what this role does, the accepted variables and their structure, and so on.

Ichimonji10 · 2018-03-01T19:51:32Z

vars/RedHat.yml

+  - python3-devel
+  - libselinux-python  # For ansible itself
+
+pulp_psycopg_package: 'python{{ ansible_python.version.major }}-psycopg2'


Pulp 3 requires Python 3. Shouldn't python3-psycopg2 always be installed?

On Fedora 26, python-psycopg2 is a proxy for python2-psycopg2.

ah no, we use python3 in a venv. Ansible uses whatever python it can find, so we need the psycopg for that specific version.

If needed, we can add some more magic for other OS releases/versions.

Ansible can be told which version of Python to use with ansible_python_version in a playbook:

- hosts: all vars: ansible_python_version: /usr/bin/python3 roles: "..."

We do so already. Both the deploy-pulp3 and pulp-from-source.yml playbooks require Python 3.5 or newer as the interpreter. Does that make a difference?

Not all users that import this role into their existing playbooks will have this though.

Roles can define requirements, and enforce them through assertions. I'm guessing your response is "I want to support Python 2 for Ansible?"

If you say you absolutely want this, I can type it in, but I prefer to have more compatibility and as far as I can see, nothing will break if we use python 2 or 3 for Ansible and python 3 for the Pulp environment.

As a datapoint Ansible is not fully supported on Python 3 yet

If you say you absolutely want this

No, I'm not set on using Python 3 for Ansible. I'm trying to find out the rationale for the current design choice. Wider compatibility is a fine rationale.

Ichimonji10 · 2018-03-01T19:53:37Z

vars/Debian.yml

+  - python3
+  - python3-venv
+
+pulp_psycopg_package: 'python{% if ansible_python.version.major != 2 %}{{ ansible_python.version.major }}{% endif %}-psycopg2'


Pulp 3 requires Python 3. Shouldn't python3-psycopg2 always be installed?

see above :)

bmbouter · 2018-03-01T20:18:49Z

@Ichimonji10 fwiw, Pulp core source install (as of yesterday) defaults to sqlite3. This was a planned change, but I'm not sure everyone is aware yet. I plan to highlight on pulp-dev for clarity now.

pieterlexis · 2018-03-02T08:32:35Z

How does one actually use this re-written code? The readme states the following:

In my OP I stated the following:

The role is still Work in Progress, as I did not have a chance to actually deploy pulp3 apart from running the tests. This also why there is no updated README yet.

I have installed a pulp server yesterday (although I don't know pulp well enough to know if it useful, but the API works). And I'll update the README once the review has gone through and I've removed/changed whatever was found during review.

Ichimonji10

This looks fantastic. Unfortunately, this role doesn't seem to configure an AMQP broker, like RabbitMQ or Qpid. Is it intended to? I think it'd be a great idea. I love how this PR consolidates much of the functionality from the other Pulp 3 ansible roles into just one.

Also, can we get a readme file?

Ichimonji10 · 2018-03-06T20:55:09Z

I made this role work with Fedora 26 and 27 with the following playbook. I think the firewall and AMQP broker configuration could be rolled into this role. Everything else seems to belong in a playbook.

---
# Install Pulp 3 from PyPI.
- hosts: all
  become: true
  vars:
    ansible_python_interpreter: /usr/bin/python3
    pulp_default_admin_password: admin
    pulp_config_secret_key: aabbccddeeffgghhiijj
  pre_tasks:
    - name: Disable SELinux
      selinux:
        state: disabled

    - name: Disable firewall
      systemd:
        name: firewalld
        state: stopped
        enabled: false

    - name: Install RabbitMQ server
      dnf:
        name: rabbitmq-server
        state: present

    - name: Start and enable RabbitMQ server
      systemd:
        name: rabbitmq-server
        state: started
        enabled: true
  roles:
    - ansible-pulp3
  post_tasks:
    - name: Create directory for pulp_web.service override file
      file:
        dest: /etc/systemd/system/pulp_web.service.d
        state: directory

    - name: Make dev webserver available to hosts beside localhost
      copy:
        dest: /etc/systemd/system/pulp_web.service.d/override.conf
        content: |
          [Service]
          ExecStart=
          ExecStart=/opt/pulp/bin/pulp-manager runserver 0.0.0.0:8000
      register: result

    - name: Restart pulp_web
      systemd:
        name: pulp_web.service
        state: restarted
        daemon_reload: true
      when: result|changed

pieterlexis · 2018-03-07T12:51:16Z

I think installing the message broker and firewall config should be optional (enabled by a variable like pulp_install_rabbitmq) so users can choose to use their own roles for this if they please (and I think firewalld is not default on Debian).

The config for the webserver should be integrated into this role indeed.

Ichimonji10 · 2018-03-07T15:04:44Z

I think installing the message broker and firewall config should be optional

This role can get away with not configuring the firewall, as local usage of Pulp 3 will still function. But a message broker is abolutely required for Pulp 3 to function.

I understand the desire to let users choose their own roles for a message broker. But that's not a good reason for a Pulp 3 installer to leave Pulp 3 in a broken state. Some sane default should be adopted. Similarly, I'd like to be able to choose which web server and database I'm using, but a Pulp 3 installer still needs to provide sane defaults for those components.

bmbouter · 2018-03-07T15:34:18Z

Yes Pulp3 needs a message broker (unfortunately). I recommend having the role default to the same default that Pulp3 has. Currently that is RabbitMQ. I hope it also has the option to install Qpid via a config variable.

At some point pre GA that will switch to Qpid, but we can't use Qpid currently due to some unrelated technical issue. I hope this role will switch it's default when Pulp switches and the variable can then be used to configure rabbitMQ as the "other" option.

dkliban

@pieterlexis Thank you for rewriting this role!

pulp/pulp_installer#6 Mainly the 4 ansible roles have been smooshed into one big role.

pulp/pulp_installer#6

Take vagrant assets from ansible-pulp3

pieterlexis added 4 commits February 7, 2018 14:29

Initial rewrite

9dbba2b

Add tests

304e5f0

Hook up to travis

07b9020

Support Debian 9

953afac

Install libselinux-python on the target nodes when needed

d8813a3

Ensure plugins are installed properly

5b24b86

Ichimonji10 reviewed Mar 1, 2018

View reviewed changes

Ichimonji10 suggested changes Mar 5, 2018

View reviewed changes

dkliban approved these changes Mar 23, 2018

View reviewed changes

dkliban merged commit 65f7453 into pulp:master Mar 23, 2018

pieterlexis deleted the rewrite branch March 23, 2018 15:45

werwty added a commit to werwty/devel that referenced this pull request Mar 27, 2018

Modify playbook to account for changes made in

6e3b7cf

pulp/pulp_installer#6 Mainly the 4 ansible roles have been smooshed into one big role.

werwty added a commit to werwty/pulp-ci that referenced this pull request Mar 27, 2018

Modify job to account for changes made in ansible role in:

904a264

pulp/pulp_installer#6

This was referenced Mar 27, 2018

Modify playbook to account for ansible role changes pulp/devel#140

Merged

Modify job to account for changes made in ansible role pulp/pulp-ci#502

Merged

dkliban pushed a commit to dkliban/pulp_installer that referenced this pull request Mar 2, 2019

Merge pull request pulp#6 from asmacdo/take-vagrant-assets-from-ansible

afb847a

Take vagrant assets from ansible-pulp3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite the role #6

Rewrite the role #6

pieterlexis commented Feb 19, 2018

pieterlexis commented Feb 19, 2018

dkliban commented Feb 20, 2018

dkliban commented Feb 20, 2018

dralley commented Feb 20, 2018 •

edited

Loading

pieterlexis commented Feb 20, 2018

dkliban commented Feb 20, 2018

pieterlexis commented Feb 20, 2018

dkliban commented Feb 21, 2018

pieterlexis commented Feb 21, 2018

Ichimonji10 commented Feb 23, 2018

Ichimonji10 commented Mar 1, 2018 •

edited

Loading

Ichimonji10 Mar 1, 2018 •

edited

Loading

pieterlexis Mar 2, 2018

Ichimonji10 Mar 2, 2018 •

edited

Loading

pieterlexis Mar 3, 2018

Ichimonji10 Mar 5, 2018

pieterlexis Mar 5, 2018

Ichimonji10 Mar 5, 2018 •

edited

Loading

Ichimonji10 Mar 1, 2018

pieterlexis Mar 2, 2018

bmbouter commented Mar 1, 2018

pieterlexis commented Mar 2, 2018

Ichimonji10 left a comment •

edited

Loading

Ichimonji10 commented Mar 6, 2018

pieterlexis commented Mar 7, 2018

Ichimonji10 commented Mar 7, 2018 •

edited

Loading

bmbouter commented Mar 7, 2018

dkliban left a comment

Rewrite the role #6

Rewrite the role #6

Conversation

pieterlexis commented Feb 19, 2018

pieterlexis commented Feb 19, 2018

dkliban commented Feb 20, 2018

dkliban commented Feb 20, 2018

dralley commented Feb 20, 2018 • edited Loading

pieterlexis commented Feb 20, 2018

dkliban commented Feb 20, 2018

pieterlexis commented Feb 20, 2018

dkliban commented Feb 21, 2018

pieterlexis commented Feb 21, 2018

Ichimonji10 commented Feb 23, 2018

Ichimonji10 commented Mar 1, 2018 • edited Loading

Ichimonji10 Mar 1, 2018 • edited Loading

Choose a reason for hiding this comment

pieterlexis Mar 2, 2018

Choose a reason for hiding this comment

Ichimonji10 Mar 2, 2018 • edited Loading

Choose a reason for hiding this comment

pieterlexis Mar 3, 2018

Choose a reason for hiding this comment

Ichimonji10 Mar 5, 2018

Choose a reason for hiding this comment

pieterlexis Mar 5, 2018

Choose a reason for hiding this comment

Ichimonji10 Mar 5, 2018 • edited Loading

Choose a reason for hiding this comment

Ichimonji10 Mar 1, 2018

Choose a reason for hiding this comment

pieterlexis Mar 2, 2018

Choose a reason for hiding this comment

bmbouter commented Mar 1, 2018

pieterlexis commented Mar 2, 2018

Ichimonji10 left a comment • edited Loading

Choose a reason for hiding this comment

Ichimonji10 commented Mar 6, 2018

pieterlexis commented Mar 7, 2018

Ichimonji10 commented Mar 7, 2018 • edited Loading

bmbouter commented Mar 7, 2018

dkliban left a comment

Choose a reason for hiding this comment

dralley commented Feb 20, 2018 •

edited

Loading

Ichimonji10 commented Mar 1, 2018 •

edited

Loading

Ichimonji10 Mar 1, 2018 •

edited

Loading

Ichimonji10 Mar 2, 2018 •

edited

Loading

Ichimonji10 Mar 5, 2018 •

edited

Loading

Ichimonji10 left a comment •

edited

Loading

Ichimonji10 commented Mar 7, 2018 •

edited

Loading