Security groups refactor

vvaldez commented 8 years ago

What does this PR do?

Refactor OpenStack Security Group creation using Ansible 2.x modules.

Also:

Adds new security_groups.yml playbook called from openstack-create
Addresses Issue #211 by adding default to each instance type security group variable
Uses the defined security groups data structure to build a comma-separated list to pass to nova_compute representing the list of requested security groups.

Default Security Group variables:

default_openshift_openstack_master_security_groups:
  - name: default
    rules: []
  - name: ose3_master
    rules:
    - name: ssh
      from_port: 22
      to_port: 22
      protocol: tcp
      cidr: 0.0.0.0/0
    - name: http
      from_port: 80
      to_port: 80
      protocol: tcp
      cidr: 0.0.0.0/0
    - name: https
      from_port: 443
      to_port: 443
      protocol: tcp
      cidr: 0.0.0.0/0
    - name: https-8443
      from_port: 8443
      to_port: 8443
      protocol: tcp
      cidr: 0.0.0.0/0
default_openshift_openstack_node_security_groups:
  - name: default
    rules: []
  - name: ose3_nodes
    rules:
    - name: ssh
      from_port: 22
      to_port: 22
      protocol: tcp
      cidr: 0.0.0.0/0
    - name: http
      from_port: 80
      to_port: 80
      protocol: tcp
      cidr: 0.0.0.0/0
default_openshift_openstack_dns_security_groups:
  - name: default
    rules: []
  - name: dns
    rules:
    - name: dns
      from_port: 53
      to_port: 53
      protocol: udp
      cidr: 0.0.0.0/0
default_openshift_openstack_nfs_security_groups:
  - name: default
    rules: []

How should this be manually tested?

Several scenarios are possible, call provisioning with each:

No OpenStack security groups present in environment
Some security groups present, some not
All security groups present with no rules defined
All security groups present with some rules defined and not others
All security groups present with some rules all rules defined
Custom security group names and rules
Is there a relevant Issue open for this?

After discussing on our call I modified this PR to use separate variables for each instance type. This playbook now does not care what type of instance is being created, it merely creates security groups and rules as supplied in the given data structure.

Fixes Issue #211 by including the default security group in the list of groups defined by default. I chose to implement it this way so any changes in the future can be made in the variables, rather than hard coding default into the playbook

The only issue is that the key rules must be present in any defined security group, even if no rules will be defined. In this case the user must use supply a blank list for the rule key which will be checked for and skipped on security group rule creation. Otherwise, the with_subelements task will fail if it cannot find the key rules. See the default variables for an example.

Who would you like to review this?

/cc @etsauer @oybed @sabre1041 @JaredBurck @JayKayy

vvaldez commented 8 years ago

Modified PR as discussed today. I removed the type key in the data structure and instead split it out into individual variables per type. I also moved them back into openshift-common.

vvaldez commented 8 years ago

Resolved merge conflicts on this PR.

Also added detection of non-Neutron environments. In these cases the playbook will not attempt to create the security groups or rules (as @sabre1041 discovered these modules do not work on non-Neutron). Therefore these would need to be manually created. As discussed, this is an acceptable tradeoff in order to use the 2.x modules. However, all other steps in this PR still work: the variables are read and a list constructed to be used for the nova_compute create task.

Here is sample output from a run against a Nova Network environment without the required security groups:

TASK [openstack-create : Provision OpenStack master] ***************************
failed: [localhost] (item=1) => {"failed": true, "item": "1", "msg": "Error in creating instance: Security group ose3_master not found for project 0f79ed005f974125bccd44a1cceed9a8. "}

vvaldez commented 8 years ago

Added UDP 8053 to default master security group.

mwitzenm commented 8 years ago

After trying the provision install I got this

TASK [openstack-create : Check for Neutron services - a failure assumes Legacy Networking (Nova Network)] ***
ok: [localhost]

TASK [openstack-create : Create Security Groups if required] *******************
failed: [localhost] (item={u'rules': [], u'name': u'default'}) => {"failed": true, "item": {"name": "default", "rules": []}, "msg": "shade is required for this module"}
failed: [localhost] (item={u'rules': [{u'to_port': 22, u'cidr': u'0.0.0.0/0', u'from_port': 22, u'protocol': u'tcp', u'name': u'ssh'}, {u'to_port': 80, u'cidr': u'0.0.0.0/0', u'from_port': 80, u'protocol': u'tcp', u'name': u'http'}, {u'to_port': 443, u'cidr': u'0.0.0.0/0', u'from_port': 443, u'protocol': u'tcp', u'name': u'https'}, {u'to_port': 8443, u'cidr': u'0.0.0.0/0', u'from_port': 8443, u'protocol': u'tcp', u'name': u'https-8443'}, {u'to_port': 8053, u'cidr': u'0.0.0.0/0', u'from_port': 8053, u'protocol': u'udp', u'name': u'udp-8053'}], u'name': u'ose3_master'}) => {"failed": true, "item": {"name": "ose3_master", "rules": [{"cidr": "0.0.0.0/0", "from_port": 22, "name": "ssh", "protocol": "tcp", "to_port": 22}, {"cidr": "0.0.0.0/0", "from_port": 80, "name": "http", "protocol": "tcp", "to_port": 80}, {"cidr": "0.0.0.0/0", "from_port": 443, "name": "https", "protocol": "tcp", "to_port": 443}, {"cidr": "0.0.0.0/0", "from_port": 8443, "name": "https-8443", "protocol": "tcp", "to_port": 8443}, {"cidr": "0.0.0.0/0", "from_port": 8053, "name": "udp-8053", "protocol": "udp", "to_port": 8053}]}, "msg": "shade is required for this module"}

NO MORE HOSTS LEFT *************************************************************
    to retry, use: --limit @/root/repository/rhc-ose-mwitzenm/rhc-ose-ansible/ose-provision.retry

PLAY RECAP *********************************************************************
localhost                  : ok=14   changed=3    unreachable=0    failed=1

oybed commented 8 years ago

@mwitzenm how are you running this? Are you using one of the docker images?

mwitzenm commented 8 years ago

[root@d9942be33b85 repository]# rhc-ose-mwitzenm/rhc-ose-ansible/provision.sh -i=/root/repository/ose-provision.d2.etl -p=/root/repository/openshift-ansible/

mwitzenm commented 8 years ago

after some effort with git ...I did correctly get his repo

[root@d9942be33b85 rhc-ose-mwitzenm]# git branch openshift-enterprise-3

vvaldez-secgroups

vvaldez commented 8 years ago

@mwitzenm 'shade' is a requirement of the os_security_group module: http://docs.ansible.com/ansible/os_security_group_module.html

If that error is not sufficient, how do you recommend detecting and reporting this missing requirement?

oybed commented 8 years ago

@vvaldez @mwitzenm I think this PR is fine as-is, but we need to update the Dockerfile for the images to include 'shade' - separate PR IMHO

mwitzenm commented 8 years ago

I completely agree. Being the new guy still learning the ropes I defer to @etsauer

vvaldez commented 8 years ago

@mwitzenm np, I'm using a Fedora 23 VM, not a container so my system has the shade library installed at /usr/lib/python2.7/site-packages/shade after installing the OpenStack clients.

etsauer commented 8 years ago

@oybed @vvaldez yeah IMO the rule here should be that introducing new dependencies to get our work done is fine, but the PR should also include those dependencies in the container we ship to run this.

oybed commented 8 years ago

Tested and working - note: Need to get PR #231 merged for it all to be successful.

rhtconsulting / rhc-ose