add support for oc_service for labels, externalIPs

Add support for `labels` and `externalIPs` in Services
This also adds support for the `labels` parameter of `oadm registry`
and `oadm router` to be a `dict` of values.
This also converts `labels` dict values in the router and registry
classes into a comma delimited list of `key=value` pairs.
The list of `--labels` is sorted for consistency in key pair positioning
in the output (and for consistency in testing) - otherwise, the order
of the list is not guarantee and has actually been observed to be
different from run to run.

13 files changed, 282 insertions, 18 deletions

diff --git a/roles/lib_openshift/src/ansible/oc_adm_registry.py b/roles/lib_openshift/src/ansible/oc_adm_registry.py
index c85973c7d..d669a3488 100644
--- a/roles/lib_openshift/src/ansible/oc_adm_registry.py
+++ b/roles/lib_openshift/src/ansible/oc_adm_registry.py
@@ -17,7 +17,7 @@ def main():
             kubeconfig=dict(default='/etc/origin/master/admin.kubeconfig', type='str'),
             images=dict(default=None, type='str'),
             latest_images=dict(default=False, type='bool'),
-            labels=dict(default=None, type='list'),
+            labels=dict(default=None, type='dict'),
             ports=dict(default=['5000'], type='list'),
             replicas=dict(default=1, type='int'),
             selector=dict(default=None, type='str'),
diff --git a/roles/lib_openshift/src/ansible/oc_adm_router.py b/roles/lib_openshift/src/ansible/oc_adm_router.py
index b6f8e90d0..c6563cc2f 100644
--- a/roles/lib_openshift/src/ansible/oc_adm_router.py
+++ b/roles/lib_openshift/src/ansible/oc_adm_router.py
@@ -21,7 +21,7 @@ def main():
             key_file=dict(default=None, type='str'),
             images=dict(default=None, type='str'), #'openshift3/ose-${component}:${version}'
             latest_images=dict(default=False, type='bool'),
-            labels=dict(default=None, type='list'),
+            labels=dict(default=None, type='dict'),
             ports=dict(default=['80:80', '443:443'], type='list'),
             replicas=dict(default=1, type='int'),
             selector=dict(default=None, type='str'),
diff --git a/roles/lib_openshift/src/ansible/oc_service.py b/roles/lib_openshift/src/ansible/oc_service.py
index 9eb144e9c..b90c08255 100644
--- a/roles/lib_openshift/src/ansible/oc_service.py
+++ b/roles/lib_openshift/src/ansible/oc_service.py
@@ -21,6 +21,7 @@ def main():
             ports=dict(default=None, type='list'),
             session_affinity=dict(default='None', type='str'),
             service_type=dict(default='ClusterIP', type='str'),
+            external_ips=dict(default=None, type='list'),
         ),
         supports_check_mode=True,
     )
diff --git a/roles/lib_openshift/src/class/oc_adm_registry.py b/roles/lib_openshift/src/class/oc_adm_registry.py
index 3c130fe28..ad6869bb6 100644
--- a/roles/lib_openshift/src/class/oc_adm_registry.py
+++ b/roles/lib_openshift/src/class/oc_adm_registry.py
@@ -143,7 +143,7 @@ class Registry(OpenShiftCLI):
 
     def prepare_registry(self):
         ''' prepare a registry for instantiation '''
-        options = self.config.to_option_list()
+        options = self.config.to_option_list(ascommalist='labels')
 
         cmd = ['registry']
         cmd.extend(options)
diff --git a/roles/lib_openshift/src/class/oc_adm_router.py b/roles/lib_openshift/src/class/oc_adm_router.py
index 1a0b94b80..0d50116d1 100644
--- a/roles/lib_openshift/src/class/oc_adm_router.py
+++ b/roles/lib_openshift/src/class/oc_adm_router.py
@@ -222,7 +222,7 @@ class Router(OpenShiftCLI):
             # No certificate was passed to us.  do not pass one to oc adm router
             self.config.config_options['default_cert']['include'] = False
 
-        options = self.config.to_option_list()
+        options = self.config.to_option_list(ascommalist='labels')
 
         cmd = ['router', self.config.name]
         cmd.extend(options)
diff --git a/roles/lib_openshift/src/class/oc_service.py b/roles/lib_openshift/src/class/oc_service.py
index 20cf23df5..7268a0c88 100644
--- a/roles/lib_openshift/src/class/oc_service.py
+++ b/roles/lib_openshift/src/class/oc_service.py
@@ -19,13 +19,15 @@ class OCService(OpenShiftCLI):
                  ports,
                  session_affinity,
                  service_type,
+                 external_ips,
                  kubeconfig='/etc/origin/master/admin.kubeconfig',
                  verbose=False):
         ''' Constructor for OCVolume '''
         super(OCService, self).__init__(namespace, kubeconfig, verbose)
         self.namespace = namespace
         self.config = ServiceConfig(sname, namespace, ports, selector, labels,
-                                    cluster_ip, portal_ip, session_affinity, service_type)
+                                    cluster_ip, portal_ip, session_affinity, service_type,
+                                    external_ips)
         self.user_svc = Service(content=self.config.data)
         self.svc = None
 
@@ -94,6 +96,7 @@ class OCService(OpenShiftCLI):
                            params['ports'],
                            params['session_affinity'],
                            params['service_type'],
+                           params['external_ips'],
                            params['kubeconfig'],
                            params['debug'])
 
diff --git a/roles/lib_openshift/src/doc/service b/roles/lib_openshift/src/doc/service
index 418f91dc5..ba9aa0b38 100644
--- a/roles/lib_openshift/src/doc/service
+++ b/roles/lib_openshift/src/doc/service
@@ -89,6 +89,13 @@ options:
     - LoadBalancer
     - ExternalName
     aliases: []
+  externalips:
+    description:
+    - A list of the external IPs that are exposed for this service.
+    - https://kubernetes.io/docs/concepts/services-networking/service/#external-ips
+    required: false
+    default: None
+    aliases: []
 author:
 - "Kenny Woodson <kwoodson@redhat.com>"
 extends_documentation_fragment: []
diff --git a/roles/lib_openshift/src/lib/base.py b/roles/lib_openshift/src/lib/base.py
index 70755187e..b3f01008b 100644
--- a/roles/lib_openshift/src/lib/base.py
+++ b/roles/lib_openshift/src/lib/base.py
@@ -581,7 +581,6 @@ class Utils(object):
             print('returning true')
         return True
 
-
 class OpenShiftCLIConfig(object):
     '''Generic Config'''
     def __init__(self, rname, namespace, kubeconfig, options):
@@ -595,18 +594,28 @@ class OpenShiftCLIConfig(object):
         ''' return config options '''
         return self._options
 
-    def to_option_list(self):
-        '''return all options as a string'''
-        return self.stringify()
-
-    def stringify(self):
-        ''' return the options hash as cli params in a string '''
+    def to_option_list(self, ascommalist=''):
+        '''return all options as a string
+           if ascommalist is set to the name of a key, and
+           the value of that key is a dict, format the dict
+           as a list of comma delimited key=value pairs'''
+        return self.stringify(ascommalist)
+
+    def stringify(self, ascommalist=''):
+        ''' return the options hash as cli params in a string
+            if ascommalist is set to the name of a key, and
+            the value of that key is a dict, format the dict
+            as a list of comma delimited key=value pairs '''
         rval = []
         for key in sorted(self.config_options.keys()):
             data = self.config_options[key]
             if data['include'] \
                and (data['value'] or isinstance(data['value'], int)):
-                rval.append('--{}={}'.format(key.replace('_', '-'), data['value']))
+                if key == ascommalist:
+                    val = ','.join(['{}={}'.format(kk, vv) for kk, vv in sorted(data['value'].items())])
+                else:
+                    val = data['value']
+                rval.append('--{}={}'.format(key.replace('_', '-'), val))
 
         return rval
 
diff --git a/roles/lib_openshift/src/lib/service.py b/roles/lib_openshift/src/lib/service.py
index eef568779..0e8cc3aa5 100644
--- a/roles/lib_openshift/src/lib/service.py
+++ b/roles/lib_openshift/src/lib/service.py
@@ -15,7 +15,8 @@ class ServiceConfig(object):
                  cluster_ip=None,
                  portal_ip=None,
                  session_affinity=None,
-                 service_type=None):
+                 service_type=None,
+                 external_ips=None):
         ''' constructor for handling service options '''
         self.name = sname
         self.namespace = namespace
@@ -26,6 +27,7 @@ class ServiceConfig(object):
         self.portal_ip = portal_ip
         self.session_affinity = session_affinity
         self.service_type = service_type
+        self.external_ips = external_ips
         self.data = {}
 
         self.create_dict()
@@ -38,8 +40,9 @@ class ServiceConfig(object):
         self.data['metadata']['name'] = self.name
         self.data['metadata']['namespace'] = self.namespace
         if self.labels:
-            for lab, lab_value  in self.labels.items():
-                self.data['metadata'][lab] = lab_value
+            self.data['metadata']['labels'] = {}
+            for lab, lab_value in self.labels.items():
+                self.data['metadata']['labels'][lab] = lab_value
         self.data['spec'] = {}
 
         if self.ports:
@@ -61,6 +64,10 @@ class ServiceConfig(object):
         if self.service_type:
             self.data['spec']['type'] = self.service_type
 
+        if self.external_ips:
+            self.data['spec']['externalIPs'] = self.external_ips
+
+
 # pylint: disable=too-many-instance-attributes,too-many-public-methods
 class Service(Yedit):
     ''' Class to model the oc service object '''
@@ -69,6 +76,7 @@ class Service(Yedit):
     cluster_ip = "spec.clusterIP"
     selector_path = 'spec.selector'
     kind = 'Service'
+    external_ips = "spec.externalIPs"
 
     def __init__(self, content):
         '''Service constructor'''
@@ -129,3 +137,50 @@ class Service(Yedit):
     def add_portal_ip(self, pip):
         '''add cluster ip'''
         self.put(Service.portal_ip, pip)
+
+    def get_external_ips(self):
+        ''' get a list of external_ips '''
+        return self.get(Service.external_ips) or []
+
+    def add_external_ips(self, inc_external_ips):
+        ''' add an external_ip to the external_ips list '''
+        if not isinstance(inc_external_ips, list):
+            inc_external_ips = [inc_external_ips]
+
+        external_ips = self.get_external_ips()
+        if not external_ips:
+            self.put(Service.external_ips, inc_external_ips)
+        else:
+            external_ips.extend(inc_external_ips)
+
+        return True
+
+    def find_external_ips(self, inc_external_ip):
+        ''' find a specific external IP '''
+        val = None
+        try:
+            idx = self.get_external_ips().index(inc_external_ip)
+            val = self.get_external_ips()[idx]
+        except ValueError:
+            pass
+
+        return val
+
+    def delete_external_ips(self, inc_external_ips):
+        ''' remove an external IP from a service '''
+        if not isinstance(inc_external_ips, list):
+            inc_external_ips = [inc_external_ips]
+
+        external_ips = self.get(Service.external_ips) or []
+
+        if not external_ips:
+            return True
+
+        removed = False
+        for inc_external_ip in inc_external_ips:
+            external_ip = self.find_external_ips(inc_external_ip)
+            if external_ip:
+                external_ips.remove(external_ip)
+                removed = True
+
+        return removed
diff --git a/roles/lib_openshift/src/test/integration/oc_service.yml b/roles/lib_openshift/src/test/integration/oc_service.yml
index 3eb6facef..29535f24a 100755
--- a/roles/lib_openshift/src/test/integration/oc_service.yml
+++ b/roles/lib_openshift/src/test/integration/oc_service.yml
@@ -18,6 +18,9 @@
         test-registtry: default
       session_affinity: ClientIP
       service_type: ClusterIP
+      labels:
+        component: test-registry
+        infra: registry
     register: svc_out
   - debug: var=svc_out
 
@@ -25,6 +28,8 @@
       that:
       - "svc_out.results.results[0]['metadata']['name'] == 'test-registry'"
       - svc_out.changed
+      - "svc_out.results.results[0]['metadata']['labels']['component'] == 'test-registry'"
+      - "svc_out.results.results[0]['metadata']['labels']['infra'] == 'registry'"
       msg: service create failed.
 
   # Test idempotent create
diff --git a/roles/lib_openshift/src/test/unit/test_oc_adm_registry.py b/roles/lib_openshift/src/test/unit/test_oc_adm_registry.py
index 97cf86170..77787fe87 100755
--- a/roles/lib_openshift/src/test/unit/test_oc_adm_registry.py
+++ b/roles/lib_openshift/src/test/unit/test_oc_adm_registry.py
@@ -218,7 +218,7 @@ class RegistryTest(unittest.TestCase):
                   'kubeconfig': '/etc/origin/master/admin.kubeconfig',
                   'images': None,
                   'latest_images': None,
-                  'labels': None,
+                  'labels': {"docker-registry": "default", "another-label": "val"},
                   'ports': ['5000'],
                   'replicas': 1,
                   'selector': 'type=infra',
@@ -255,6 +255,7 @@ class RegistryTest(unittest.TestCase):
             mock.call(['oc', 'get', 'dc', 'docker-registry', '-o', 'json', '-n', 'default'], None),
             mock.call(['oc', 'get', 'svc', 'docker-registry', '-o', 'json', '-n', 'default'], None),
             mock.call(['oc', 'adm', 'registry',
+                       "--labels=another-label=val,docker-registry=default",
                        '--ports=5000', '--replicas=1', '--selector=type=infra',
                        '--service-account=registry', '--dry-run=True', '-o', 'json', '-n', 'default'], None),
             mock.call(['oc', 'create', '-f', mock.ANY, '-n', 'default'], None),
diff --git a/roles/lib_openshift/src/test/unit/test_oc_adm_router.py b/roles/lib_openshift/src/test/unit/test_oc_adm_router.py
index 5481ac623..dcf768e08 100755
--- a/roles/lib_openshift/src/test/unit/test_oc_adm_router.py
+++ b/roles/lib_openshift/src/test/unit/test_oc_adm_router.py
@@ -300,7 +300,7 @@ class RouterTest(unittest.TestCase):
                   'cert_file': None,
                   'key_file': None,
                   'cacert_file': None,
-                  'labels': None,
+                  'labels': {"router": "router", "another-label": "val"},
                   'ports': ['80:80', '443:443'],
                   'images': None,
                   'latest_images': None,
@@ -363,6 +363,7 @@ class RouterTest(unittest.TestCase):
             mock.call(['oc', 'get', 'secret', 'router-certs', '-o', 'json', '-n', 'default'], None),
             mock.call(['oc', 'get', 'clusterrolebinding', 'router-router-role', '-o', 'json', '-n', 'default'], None),
             mock.call(['oc', 'adm', 'router', 'router', '--expose-metrics=False', '--external-host-insecure=False',
+                       "--labels=another-label=val,router=router",
                        '--ports=80:80,443:443', '--replicas=2', '--selector=type=infra', '--service-account=router',
                        '--stats-port=1936', '--dry-run=True', '-o', 'json', '-n', 'default'], None),
             mock.call(['oc', 'create', '-f', mock.ANY, '-n', 'default'], None),
diff --git a/roles/lib_openshift/src/test/unit/test_oc_service.py b/roles/lib_openshift/src/test/unit/test_oc_service.py
index e74c66665..9c21a262f 100755
--- a/roles/lib_openshift/src/test/unit/test_oc_service.py
+++ b/roles/lib_openshift/src/test/unit/test_oc_service.py
@@ -39,6 +39,7 @@ class OCServiceTest(unittest.TestCase):
                   'selector': None,
                   'session_affinity': None,
                   'service_type': None,
+                  'external_ips': None,
                   'kubeconfig': '/etc/origin/master/admin.kubeconfig',
                   'debug': False}
 
@@ -124,6 +125,7 @@ class OCServiceTest(unittest.TestCase):
                   'selector': {'router': 'router'},
                   'session_affinity': 'ClientIP',
                   'service_type': 'ClusterIP',
+                  'external_ips': None,
                   'kubeconfig': '/etc/origin/master/admin.kubeconfig',
                   'debug': False}
 
@@ -303,3 +305,183 @@ class OCServiceTest(unittest.TestCase):
         mock_shutil_which.side_effect = lambda _f, path=None: oc_bin
 
         self.assertEqual(locate_oc_binary(), oc_bin)
+
+    @mock.patch('oc_service.Utils.create_tmpfile_copy')
+    @mock.patch('oc_service.OCService._run')
+    def test_create_with_labels(self, mock_cmd, mock_tmpfile_copy):
+        ''' Testing a create service '''
+        params = {'name': 'router',
+                  'namespace': 'default',
+                  'ports': {'name': '9000-tcp',
+                            'port': 9000,
+                            'protocol': 'TCP',
+                            'targetPOrt': 9000},
+                  'state': 'present',
+                  'labels': {'component': 'some_component', 'infra': 'true'},
+                  'clusterip': None,
+                  'portalip': None,
+                  'selector': {'router': 'router'},
+                  'session_affinity': 'ClientIP',
+                  'service_type': 'ClusterIP',
+                  'external_ips': None,
+                  'kubeconfig': '/etc/origin/master/admin.kubeconfig',
+                  'debug': False}
+
+        service = '''{
+            "kind": "Service",
+            "apiVersion": "v1",
+            "metadata": {
+                "name": "router",
+                "namespace": "default",
+                "selfLink": "/api/v1/namespaces/default/services/router",
+                "uid": "fabd2440-e3d8-11e6-951c-0e3dd518cefa",
+                "resourceVersion": "3206",
+                "creationTimestamp": "2017-01-26T15:06:14Z",
+                "labels": {"component": "some_component", "infra": "true"}
+            },
+            "spec": {
+                "ports": [
+                    {
+                        "name": "80-tcp",
+                        "protocol": "TCP",
+                        "port": 80,
+                        "targetPort": 80
+                    },
+                    {
+                        "name": "443-tcp",
+                        "protocol": "TCP",
+                        "port": 443,
+                        "targetPort": 443
+                    },
+                    {
+                        "name": "1936-tcp",
+                        "protocol": "TCP",
+                        "port": 1936,
+                        "targetPort": 1936
+                    },
+                    {
+                        "name": "5000-tcp",
+                        "protocol": "TCP",
+                        "port": 5000,
+                        "targetPort": 5000
+                    }
+                ],
+                "selector": {
+                    "router": "router"
+                },
+                "clusterIP": "172.30.129.161",
+                "type": "ClusterIP",
+                "sessionAffinity": "None"
+            },
+            "status": {
+                "loadBalancer": {}
+            }
+        }'''
+        mock_cmd.side_effect = [
+            (1, '', 'Error from server: services "router" not found'),
+            (1, '', 'Error from server: services "router" not found'),
+            (0, service, ''),
+            (0, service, '')
+        ]
+
+        mock_tmpfile_copy.side_effect = [
+            '/tmp/mocked_kubeconfig',
+        ]
+
+        results = OCService.run_ansible(params, False)
+
+        self.assertTrue(results['changed'])
+        self.assertTrue(results['results']['returncode'] == 0)
+        self.assertEqual(results['results']['results'][0]['metadata']['name'], 'router')
+        self.assertEqual(results['results']['results'][0]['metadata']['labels'], {"component": "some_component", "infra": "true"})
+
+    @mock.patch('oc_service.Utils.create_tmpfile_copy')
+    @mock.patch('oc_service.OCService._run')
+    def test_create_with_external_ips(self, mock_cmd, mock_tmpfile_copy):
+        ''' Testing a create service '''
+        params = {'name': 'router',
+                  'namespace': 'default',
+                  'ports': {'name': '9000-tcp',
+                            'port': 9000,
+                            'protocol': 'TCP',
+                            'targetPOrt': 9000},
+                  'state': 'present',
+                  'labels': {'component': 'some_component', 'infra': 'true'},
+                  'clusterip': None,
+                  'portalip': None,
+                  'selector': {'router': 'router'},
+                  'session_affinity': 'ClientIP',
+                  'service_type': 'ClusterIP',
+                  'external_ips': ['1.2.3.4', '5.6.7.8'],
+                  'kubeconfig': '/etc/origin/master/admin.kubeconfig',
+                  'debug': False}
+
+        service = '''{
+            "kind": "Service",
+            "apiVersion": "v1",
+            "metadata": {
+                "name": "router",
+                "namespace": "default",
+                "selfLink": "/api/v1/namespaces/default/services/router",
+                "uid": "fabd2440-e3d8-11e6-951c-0e3dd518cefa",
+                "resourceVersion": "3206",
+                "creationTimestamp": "2017-01-26T15:06:14Z",
+                "labels": {"component": "some_component", "infra": "true"}
+            },
+            "spec": {
+                "ports": [
+                    {
+                        "name": "80-tcp",
+                        "protocol": "TCP",
+                        "port": 80,
+                        "targetPort": 80
+                    },
+                    {
+                        "name": "443-tcp",
+                        "protocol": "TCP",
+                        "port": 443,
+                        "targetPort": 443
+                    },
+                    {
+                        "name": "1936-tcp",
+                        "protocol": "TCP",
+                        "port": 1936,
+                        "targetPort": 1936
+                    },
+                    {
+                        "name": "5000-tcp",
+                        "protocol": "TCP",
+                        "port": 5000,
+                        "targetPort": 5000
+                    }
+                ],
+                "selector": {
+                    "router": "router"
+                },
+                "clusterIP": "172.30.129.161",
+                "externalIPs": ["1.2.3.4", "5.6.7.8"],
+                "type": "ClusterIP",
+                "sessionAffinity": "None"
+            },
+            "status": {
+                "loadBalancer": {}
+            }
+        }'''
+        mock_cmd.side_effect = [
+            (1, '', 'Error from server: services "router" not found'),
+            (1, '', 'Error from server: services "router" not found'),
+            (0, service, ''),
+            (0, service, '')
+        ]
+
+        mock_tmpfile_copy.side_effect = [
+            '/tmp/mocked_kubeconfig',
+        ]
+
+        results = OCService.run_ansible(params, False)
+
+        self.assertTrue(results['changed'])
+        self.assertTrue(results['results']['returncode'] == 0)
+        self.assertEqual(results['results']['results'][0]['metadata']['name'], 'router')
+        self.assertEqual(results['results']['results'][0]['metadata']['labels'], {"component": "some_component", "infra": "true"})
+        self.assertEqual(results['results']['results'][0]['spec']['externalIPs'], ["1.2.3.4", "5.6.7.8"])