Merge pull request #7206 from agiledigital/master

 Allow fully qualified provisioner names

roles/lib_openshift/library/oc_storageclass.py

@@ -1614,13 +1614,27 @@ class OCStorageClass(OpenShiftCLI):
         return False
 
     @staticmethod
+    def provisioner_name_qualified(provisioner_name):
+        pattern = re.compile(r'^[a-z0-9A-Z-_.]+\/[a-z0-9A-Z-_.]+$')
+        return pattern.match(provisioner_name)
+
+    @staticmethod
     # pylint: disable=too-many-return-statements,too-many-branches
     # TODO: This function should be refactored into its individual parts.
     def run_ansible(params, check_mode):
         '''run the ansible idempotent code'''
 
+        # Make sure that the provisioner is fully qualified before using it
+        # E.g. if 'aws-efs' is provided as a provisioner, convert it to 'kubernetes.io/aws-efs'
+        # but if the name is already qualified  (e.g. 'openshift.org/aws-efs') then leave it be.
+        raw_provisioner_name = params['provisioner']
+        if OCStorageClass.provisioner_name_qualified(raw_provisioner_name):
+            qualified_provisioner_name = raw_provisioner_name
+        else:
+            qualified_provisioner_name = "kubernetes.io/{}".format(params['provisioner'])
+
         rconfig = StorageClassConfig(params['name'],
-                                     provisioner="kubernetes.io/{}".format(params['provisioner']),
+                                     provisioner=qualified_provisioner_name,
                                      parameters=params['parameters'],
                                      annotations=params['annotations'],
                                      api_version="storage.k8s.io/{}".format(params['api_version']),

roles/lib_openshift/src/class/oc_storageclass.py

@@ -64,13 +64,27 @@ class OCStorageClass(OpenShiftCLI):
         return False
 
     @staticmethod
+    def provisioner_name_qualified(provisioner_name):
+        pattern = re.compile(r'^[a-z0-9A-Z-_.]+\/[a-z0-9A-Z-_.]+$')
+        return pattern.match(provisioner_name)
+
+    @staticmethod
     # pylint: disable=too-many-return-statements,too-many-branches
     # TODO: This function should be refactored into its individual parts.
     def run_ansible(params, check_mode):
         '''run the ansible idempotent code'''
 
+        # Make sure that the provisioner is fully qualified before using it
+        # E.g. if 'aws-efs' is provided as a provisioner, convert it to 'kubernetes.io/aws-efs'
+        # but if the name is already qualified  (e.g. 'openshift.org/aws-efs') then leave it be.
+        raw_provisioner_name = params['provisioner']
+        if OCStorageClass.provisioner_name_qualified(raw_provisioner_name):
+            qualified_provisioner_name = raw_provisioner_name
+        else:
+            qualified_provisioner_name = "kubernetes.io/{}".format(params['provisioner'])
+
         rconfig = StorageClassConfig(params['name'],
-                                     provisioner="kubernetes.io/{}".format(params['provisioner']),
+                                     provisioner=qualified_provisioner_name,
                                      parameters=params['parameters'],
                                      annotations=params['annotations'],
                                      api_version="storage.k8s.io/{}".format(params['api_version']),

roles/lib_openshift/src/test/unit/test_oc_storageclass.py

@@ -6,6 +6,7 @@ import os
 import sys
 import unittest
 import mock
+import yaml
 
 # Removing invalid variable names for tests so that I can
 # keep them brief
@@ -22,26 +23,113 @@ class OCStorageClassTest(unittest.TestCase):
     '''
      Test class for OCStorageClass
     '''
-    params = {'kubeconfig': '/etc/origin/master/admin.kubeconfig',
-              'state': 'present',
-              'debug': False,
-              'name': 'testsc',
-              'provisioner': 'kubernetes.io/aws-ebs',
-              'annotations': {'storageclass.beta.kubernetes.io/is-default-class': "true"},
-              'parameters': {'type': 'gp2'},
-              'api_version': 'v1',
-              'default_storage_class': 'true'}
 
+    @mock.patch('oc_storageclass.Utils.create_tmpfile')
     @mock.patch('oc_storageclass.locate_oc_binary')
     @mock.patch('oc_storageclass.Utils.create_tmpfile_copy')
     @mock.patch('oc_storageclass.OCStorageClass._run')
-    def test_adding_a_storageclass(self, mock_cmd, mock_tmpfile_copy, mock_oc_binary):
+    def test_adding_a_storageclass_without_qualification(self, mock_cmd, mock_tmpfile_copy, mock_oc_binary, mock_tmpfile_create):
         ''' Testing adding a storageclass '''
 
         # Arrange
 
         # run_ansible input parameters
 
+        params = {
+            'kubeconfig': '/etc/origin/master/admin.kubeconfig',
+            'state': 'present',
+            'debug': False,
+            'name': 'testsc',
+            'provisioner': 'aws-ebs',
+            'annotations': {'storageclass.beta.kubernetes.io/is-default-class': "true"},
+            'parameters': {'type': 'gp2'},
+            'api_version': 'v1',
+            'default_storage_class': 'true'
+        }
+
+        valid_result_json = '''{
+            "kind": "StorageClass",
+            "apiVersion": "v1",
+            "metadata": {
+                "name": "testsc",
+                "selfLink": "/apis/storage.k8s.io/v1/storageclasses/gp2",
+                "uid": "4d8320c9-e66f-11e6-8edc-0eece8f2ce22",
+                "resourceVersion": "2828",
+                "creationTimestamp": "2017-01-29T22:07:19Z",
+                "annotations": {"storageclass.beta.kubernetes.io/is-default-class": "true"}
+            },
+            "provisioner": "kubernetes.io/aws-ebs",
+            "parameters": {"type": "gp2"}
+        }'''
+
+        # Return values of our mocked function call. These get returned once per call.
+        mock_cmd.side_effect = [
+            # First call to mock
+            (1, '', 'Error from server: storageclass "testsc" not found'),
+
+            # Second call to mock
+            (0, 'storageclass "testsc" created', ''),
+
+            # Third call to mock
+            (0, valid_result_json, ''),
+        ]
+
+        mock_oc_binary.side_effect = [
+            'oc'
+        ]
+
+        mock_tmpfile_copy.side_effect = [
+            '/tmp/mocked_kubeconfig',
+        ]
+
+        generated_yaml_spec_file = '/tmp/spec_output_yaml'
+
+        mock_tmpfile_create.side_effect = [
+            generated_yaml_spec_file,
+        ]
+
+        # Act
+        results = OCStorageClass.run_ansible(params, False)
+
+        with open(generated_yaml_spec_file) as json_data:
+            generated_spec = yaml.load(json_data)
+
+        # Assert
+        self.assertTrue(generated_spec['provisioner'], 'kubernetes.io/aws-ebs')
+        self.assertTrue(results['changed'])
+        self.assertEqual(results['results']['returncode'], 0)
+        self.assertEqual(results['state'], 'present')
+
+        # Making sure our mock was called as we expected
+        mock_cmd.assert_has_calls([
+            mock.call(['oc', 'get', 'storageclass', 'testsc', '-o', 'json'], None),
+            mock.call(['oc', 'create', '-f', mock.ANY], None),
+            mock.call(['oc', 'get', 'storageclass', 'testsc', '-o', 'json'], None),
+        ])
+
+    @mock.patch('oc_storageclass.Utils.create_tmpfile')
+    @mock.patch('oc_storageclass.locate_oc_binary')
+    @mock.patch('oc_storageclass.Utils.create_tmpfile_copy')
+    @mock.patch('oc_storageclass.OCStorageClass._run')
+    def test_adding_a_storageclass_with_qualification(self, mock_cmd, mock_tmpfile_copy, mock_oc_binary, mock_tmpfile_create):
+        ''' Testing adding a storageclass '''
+
+        # Arrange
+
+        # run_ansible input parameters
+
+        params = {
+            'kubeconfig': '/etc/origin/master/admin.kubeconfig',
+            'state': 'present',
+            'debug': False,
+            'name': 'testsc',
+            'provisioner': 'kubernetes.io/aws-ebs',
+            'annotations': {'storageclass.beta.kubernetes.io/is-default-class': "true"},
+            'parameters': {'type': 'gp2'},
+            'api_version': 'v1',
+            'default_storage_class': 'true'
+        }
+
         valid_result_json = '''{
             "kind": "StorageClass",
             "apiVersion": "v1",
@@ -77,10 +165,20 @@ class OCStorageClassTest(unittest.TestCase):
             '/tmp/mocked_kubeconfig',
         ]
 
+        generated_yaml_spec_file = '/tmp/spec_output_yaml'
+
+        mock_tmpfile_create.side_effect = [
+            generated_yaml_spec_file,
+        ]
+
         # Act
-        results = OCStorageClass.run_ansible(OCStorageClassTest.params, False)
+        results = OCStorageClass.run_ansible(params, False)
+
+        with open(generated_yaml_spec_file) as json_data:
+            generated_spec = yaml.load(json_data)
 
         # Assert
+        self.assertTrue(generated_spec['provisioner'], 'kubernetes.io/aws-ebs')
         self.assertTrue(results['changed'])
         self.assertEqual(results['results']['returncode'], 0)
         self.assertEqual(results['state'], 'present')