[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user asfgit closed the pull request at: https://github.com/apache/cloudstack/pull/1615 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user jburwell commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r81019103 --- Diff: test/integration/smoke/test_ssvm.py --- @@ -42,12 +43,19 @@ class TestSSVMs(cloudstackTestCase): def setUp(self): + test_case = super(TestSSVMs, self) self.apiclient = self.testClient.getApiClient() self.hypervisor = self.testClient.getHypervisorInfo() self.cleanup = [] + self.config = test_case.getClsConfig() self.services = self.testClient.getParsedTestDataConfig() self.zone = get_zone(self.apiclient, self.testClient.getZoneForTests()) +self.logger = logging.getLogger('TestSSVMs') +self.stream_handler = logging.StreamHandler() +self.logger.setLevel(logging.DEBUG) +self.logger.addHandler(self.stream_handler) + # Default sleep is set to 90 seconds, which is too long if the SSVM takes up to 2min to start. # Second sleep in the loop will waste test time. self.services["sleep"] = 30 --- End diff -- Thanks for the information. Good to know to know that it has been researched and that it is as low as possible. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user nvazquez commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r80986927 --- Diff: test/integration/smoke/test_ssvm.py --- @@ -42,12 +43,19 @@ class TestSSVMs(cloudstackTestCase): def setUp(self): + test_case = super(TestSSVMs, self) self.apiclient = self.testClient.getApiClient() self.hypervisor = self.testClient.getHypervisorInfo() self.cleanup = [] + self.config = test_case.getClsConfig() self.services = self.testClient.getParsedTestDataConfig() self.zone = get_zone(self.apiclient, self.testClient.getZoneForTests()) +self.logger = logging.getLogger('TestSSVMs') +self.stream_handler = logging.StreamHandler() +self.logger.setLevel(logging.DEBUG) +self.logger.addHandler(self.stream_handler) + # Default sleep is set to 90 seconds, which is too long if the SSVM takes up to 2min to start. # Second sleep in the loop will waste test time. self.services["sleep"] = 30 --- End diff -- We tested changing it to 5 secs and caused much tests to fail, at least in out environment --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user nvazquez commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r80951612 --- Diff: test/integration/smoke/test_ssvm.py --- @@ -1197,3 +1205,148 @@ def test_10_destroy_cpvm(self): # Call to verify cloud process is running self.test_04_cpvm_internals() return + +@attr( +tags=[ +"advanced", +"advancedns", +"smoke", +"basic", +"sg"], +required_hardware="true") +def test_11_ss_nfs_version_on_ssvm(self): +"""Test NFS Version on Secondary Storage mounted properly on SSVM +""" + + # 1) List SSVM in zone + # 2) Get id and url from mounted nfs store + # 3) Update NFS version for previous image store + # 4) Stop SSVM + # 5) Check NFS version of mounted nfs store after SSVM starts + + nfs_version = self.config.nfsVersion + if nfs_version == None: + self.skipTest('No NFS version provided in test data') + + #List SSVM for zone id +list_ssvm_response = list_ssvms( +self.apiclient, +systemvmtype='secondarystoragevm', +state='Running', +zoneid=self.zone.id +) +self.assertEqual( +isinstance(list_ssvm_response, list), +True, +"Check list response returns a valid list" +) +self.assertEqual( +len(list_ssvm_response), +1, +"Check list System VMs response" +) + +ssvm = list_ssvm_response[0] +image_stores_response = ImageStore.list(self.apiclient,zoneid=self.zone.id) + + if self.hypervisor.lower() in ('vmware', 'hyperv'): +# SSH into SSVMs is done via management server for Vmware and Hyper-V +result = get_process_status( +self.apiclient.connection.mgtSvr, +22, +self.apiclient.connection.user, +self.apiclient.connection.passwd, +ssvm.privateip, +"mount | grep 'type nfs'", +hypervisor=self.hypervisor) + + for res in result: +split_res = res.split(" on ") +mounted_img_store_url = split_res[0] +for img_store in image_stores_response: + img_store_url = str(img_store.url) + if img_store_url.startswith("nfs://"): + img_store_url = img_store_url[6:] +#Add colon after ip address to match output from mount command +first_slash = img_store_url.find('/') +img_store_url = img_store_url[0:first_slash] + ':' + img_store_url[first_slash:] +if img_store_url == mounted_img_store_url: +img_store_id = img_store.id +break + +self.assertNotEqual( +img_store_id, +None, +"Check image store id mounted on SSVM" +) + + #Update NFS version for image store mounted on SSVM +updateConfigurationCmd = updateConfiguration.updateConfigurationCmd() +updateConfigurationCmd.name = "secstorage.nfs.version" +updateConfigurationCmd.value = nfs_version +updateConfigurationCmd.imagestoreuuid = img_store_id + +updateConfigurationResponse = self.apiclient.updateConfiguration(updateConfigurationCmd) +self.logger.debug("updated the parameter %s with value %s"%(updateConfigurationResponse.name, updateConfigurationResponse.value)) + + #Stop SSVM + self.debug("Stopping SSVM: %s" % ssvm.id) +cmd = stopSystemVm.stopSystemVmCmd() +cmd.id = ssvm.id +self.apiclient.stopSystemVm(cmd) + +timeout = self.services["timeout"] +while True: +list_ssvm_response = list_ssvms( +self.apiclient, +id=ssvm.id +) +if isinstance(list_ssvm_response, list): +if list_ssvm_response[0].state == 'Running': +break +if timeout == 0: +raise Exception("List SSVM call failed!") + +time.sleep(self.services["sleep"]) +timeout = timeout - 1 + + self.assertEqual( +isinstance(list_ssvm_response, list), +True, +"Check list response returns a valid list" +) +ssvm = list_ssvm_response[0] +self.debug("SSVM state after debug: %s" % ssvm.state) +self.assertEqual(
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user nvazquez commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r80951528 --- Diff: test/integration/smoke/test_ssvm.py --- @@ -1197,3 +1205,148 @@ def test_10_destroy_cpvm(self): # Call to verify cloud process is running self.test_04_cpvm_internals() return + +@attr( +tags=[ +"advanced", +"advancedns", +"smoke", +"basic", +"sg"], +required_hardware="true") +def test_11_ss_nfs_version_on_ssvm(self): +"""Test NFS Version on Secondary Storage mounted properly on SSVM +""" + + # 1) List SSVM in zone + # 2) Get id and url from mounted nfs store + # 3) Update NFS version for previous image store + # 4) Stop SSVM + # 5) Check NFS version of mounted nfs store after SSVM starts + + nfs_version = self.config.nfsVersion + if nfs_version == None: + self.skipTest('No NFS version provided in test data') + + #List SSVM for zone id +list_ssvm_response = list_ssvms( +self.apiclient, +systemvmtype='secondarystoragevm', +state='Running', +zoneid=self.zone.id +) +self.assertEqual( +isinstance(list_ssvm_response, list), +True, +"Check list response returns a valid list" +) +self.assertEqual( +len(list_ssvm_response), +1, +"Check list System VMs response" +) + +ssvm = list_ssvm_response[0] +image_stores_response = ImageStore.list(self.apiclient,zoneid=self.zone.id) + + if self.hypervisor.lower() in ('vmware', 'hyperv'): +# SSH into SSVMs is done via management server for Vmware and Hyper-V +result = get_process_status( +self.apiclient.connection.mgtSvr, +22, +self.apiclient.connection.user, +self.apiclient.connection.passwd, +ssvm.privateip, +"mount | grep 'type nfs'", +hypervisor=self.hypervisor) + + for res in result: +split_res = res.split(" on ") +mounted_img_store_url = split_res[0] +for img_store in image_stores_response: + img_store_url = str(img_store.url) + if img_store_url.startswith("nfs://"): + img_store_url = img_store_url[6:] +#Add colon after ip address to match output from mount command +first_slash = img_store_url.find('/') +img_store_url = img_store_url[0:first_slash] + ':' + img_store_url[first_slash:] +if img_store_url == mounted_img_store_url: +img_store_id = img_store.id +break + +self.assertNotEqual( +img_store_id, +None, +"Check image store id mounted on SSVM" +) + + #Update NFS version for image store mounted on SSVM +updateConfigurationCmd = updateConfiguration.updateConfigurationCmd() +updateConfigurationCmd.name = "secstorage.nfs.version" +updateConfigurationCmd.value = nfs_version +updateConfigurationCmd.imagestoreuuid = img_store_id + +updateConfigurationResponse = self.apiclient.updateConfiguration(updateConfigurationCmd) +self.logger.debug("updated the parameter %s with value %s"%(updateConfigurationResponse.name, updateConfigurationResponse.value)) + + #Stop SSVM + self.debug("Stopping SSVM: %s" % ssvm.id) +cmd = stopSystemVm.stopSystemVmCmd() +cmd.id = ssvm.id +self.apiclient.stopSystemVm(cmd) + +timeout = self.services["timeout"] +while True: +list_ssvm_response = list_ssvms( +self.apiclient, +id=ssvm.id +) +if isinstance(list_ssvm_response, list): +if list_ssvm_response[0].state == 'Running': +break +if timeout == 0: +raise Exception("List SSVM call failed!") + +time.sleep(self.services["sleep"]) +timeout = timeout - 1 --- End diff -- Done, thanks --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user nvazquez commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r80951509 --- Diff: test/integration/smoke/test_ssvm.py --- @@ -1197,3 +1205,148 @@ def test_10_destroy_cpvm(self): # Call to verify cloud process is running self.test_04_cpvm_internals() return + +@attr( +tags=[ +"advanced", +"advancedns", +"smoke", +"basic", +"sg"], +required_hardware="true") +def test_11_ss_nfs_version_on_ssvm(self): +"""Test NFS Version on Secondary Storage mounted properly on SSVM +""" + + # 1) List SSVM in zone + # 2) Get id and url from mounted nfs store + # 3) Update NFS version for previous image store + # 4) Stop SSVM + # 5) Check NFS version of mounted nfs store after SSVM starts + + nfs_version = self.config.nfsVersion + if nfs_version == None: + self.skipTest('No NFS version provided in test data') + + #List SSVM for zone id +list_ssvm_response = list_ssvms( +self.apiclient, +systemvmtype='secondarystoragevm', +state='Running', +zoneid=self.zone.id +) +self.assertEqual( +isinstance(list_ssvm_response, list), +True, +"Check list response returns a valid list" +) +self.assertEqual( +len(list_ssvm_response), +1, +"Check list System VMs response" +) + +ssvm = list_ssvm_response[0] +image_stores_response = ImageStore.list(self.apiclient,zoneid=self.zone.id) + + if self.hypervisor.lower() in ('vmware', 'hyperv'): +# SSH into SSVMs is done via management server for Vmware and Hyper-V +result = get_process_status( +self.apiclient.connection.mgtSvr, +22, +self.apiclient.connection.user, +self.apiclient.connection.passwd, +ssvm.privateip, +"mount | grep 'type nfs'", +hypervisor=self.hypervisor) + + for res in result: +split_res = res.split(" on ") +mounted_img_store_url = split_res[0] +for img_store in image_stores_response: + img_store_url = str(img_store.url) + if img_store_url.startswith("nfs://"): + img_store_url = img_store_url[6:] +#Add colon after ip address to match output from mount command +first_slash = img_store_url.find('/') +img_store_url = img_store_url[0:first_slash] + ':' + img_store_url[first_slash:] +if img_store_url == mounted_img_store_url: +img_store_id = img_store.id +break + +self.assertNotEqual( +img_store_id, +None, +"Check image store id mounted on SSVM" +) + + #Update NFS version for image store mounted on SSVM +updateConfigurationCmd = updateConfiguration.updateConfigurationCmd() +updateConfigurationCmd.name = "secstorage.nfs.version" +updateConfigurationCmd.value = nfs_version +updateConfigurationCmd.imagestoreuuid = img_store_id + +updateConfigurationResponse = self.apiclient.updateConfiguration(updateConfigurationCmd) +self.logger.debug("updated the parameter %s with value %s"%(updateConfigurationResponse.name, updateConfigurationResponse.value)) + + #Stop SSVM + self.debug("Stopping SSVM: %s" % ssvm.id) +cmd = stopSystemVm.stopSystemVmCmd() +cmd.id = ssvm.id +self.apiclient.stopSystemVm(cmd) + +timeout = self.services["timeout"] +while True: +list_ssvm_response = list_ssvms( +self.apiclient, +id=ssvm.id +) +if isinstance(list_ssvm_response, list): +if list_ssvm_response[0].state == 'Running': +break +if timeout == 0: +raise Exception("List SSVM call failed!") + +time.sleep(self.services["sleep"]) +timeout = timeout - 1 + + self.assertEqual( +isinstance(list_ssvm_response, list), +True, +"Check list response returns a valid list" +) +ssvm = list_ssvm_response[0] +self.debug("SSVM state after debug: %s" % ssvm.state) +self.assertEqual(
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user nvazquez commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r80937979 --- Diff: test/integration/smoke/test_ssvm.py --- @@ -1197,3 +1205,148 @@ def test_10_destroy_cpvm(self): # Call to verify cloud process is running self.test_04_cpvm_internals() return + +@attr( +tags=[ +"advanced", +"advancedns", +"smoke", +"basic", +"sg"], +required_hardware="true") +def test_11_ss_nfs_version_on_ssvm(self): +"""Test NFS Version on Secondary Storage mounted properly on SSVM +""" + + # 1) List SSVM in zone + # 2) Get id and url from mounted nfs store + # 3) Update NFS version for previous image store + # 4) Stop SSVM + # 5) Check NFS version of mounted nfs store after SSVM starts + + nfs_version = self.config.nfsVersion + if nfs_version == None: + self.skipTest('No NFS version provided in test data') + + #List SSVM for zone id +list_ssvm_response = list_ssvms( +self.apiclient, +systemvmtype='secondarystoragevm', +state='Running', +zoneid=self.zone.id +) --- End diff -- Done, thanks --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user jburwell commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r80701705 --- Diff: test/integration/smoke/test_ssvm.py --- @@ -1197,3 +1205,148 @@ def test_10_destroy_cpvm(self): # Call to verify cloud process is running self.test_04_cpvm_internals() return + +@attr( +tags=[ +"advanced", +"advancedns", +"smoke", +"basic", +"sg"], +required_hardware="true") +def test_11_ss_nfs_version_on_ssvm(self): +"""Test NFS Version on Secondary Storage mounted properly on SSVM +""" + + # 1) List SSVM in zone + # 2) Get id and url from mounted nfs store + # 3) Update NFS version for previous image store + # 4) Stop SSVM + # 5) Check NFS version of mounted nfs store after SSVM starts + + nfs_version = self.config.nfsVersion + if nfs_version == None: + self.skipTest('No NFS version provided in test data') + + #List SSVM for zone id +list_ssvm_response = list_ssvms( +self.apiclient, +systemvmtype='secondarystoragevm', +state='Running', +zoneid=self.zone.id +) +self.assertEqual( +isinstance(list_ssvm_response, list), +True, +"Check list response returns a valid list" +) +self.assertEqual( +len(list_ssvm_response), +1, +"Check list System VMs response" +) + +ssvm = list_ssvm_response[0] +image_stores_response = ImageStore.list(self.apiclient,zoneid=self.zone.id) + + if self.hypervisor.lower() in ('vmware', 'hyperv'): +# SSH into SSVMs is done via management server for Vmware and Hyper-V +result = get_process_status( +self.apiclient.connection.mgtSvr, +22, +self.apiclient.connection.user, +self.apiclient.connection.passwd, +ssvm.privateip, +"mount | grep 'type nfs'", +hypervisor=self.hypervisor) + + for res in result: +split_res = res.split(" on ") --- End diff -- Relying on the result having only one space seems brittle. Please consider splitting on "on" and trimming the results or using a regular expression. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user jburwell commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r80702685 --- Diff: test/integration/smoke/test_ssvm.py --- @@ -1197,3 +1205,148 @@ def test_10_destroy_cpvm(self): # Call to verify cloud process is running self.test_04_cpvm_internals() return + +@attr( +tags=[ +"advanced", +"advancedns", +"smoke", +"basic", +"sg"], +required_hardware="true") +def test_11_ss_nfs_version_on_ssvm(self): +"""Test NFS Version on Secondary Storage mounted properly on SSVM +""" + + # 1) List SSVM in zone + # 2) Get id and url from mounted nfs store + # 3) Update NFS version for previous image store + # 4) Stop SSVM + # 5) Check NFS version of mounted nfs store after SSVM starts + + nfs_version = self.config.nfsVersion + if nfs_version == None: + self.skipTest('No NFS version provided in test data') + + #List SSVM for zone id +list_ssvm_response = list_ssvms( +self.apiclient, +systemvmtype='secondarystoragevm', +state='Running', +zoneid=self.zone.id +) +self.assertEqual( +isinstance(list_ssvm_response, list), +True, +"Check list response returns a valid list" +) +self.assertEqual( +len(list_ssvm_response), +1, +"Check list System VMs response" +) + +ssvm = list_ssvm_response[0] +image_stores_response = ImageStore.list(self.apiclient,zoneid=self.zone.id) + + if self.hypervisor.lower() in ('vmware', 'hyperv'): +# SSH into SSVMs is done via management server for Vmware and Hyper-V +result = get_process_status( +self.apiclient.connection.mgtSvr, +22, +self.apiclient.connection.user, +self.apiclient.connection.passwd, +ssvm.privateip, +"mount | grep 'type nfs'", +hypervisor=self.hypervisor) + + for res in result: +split_res = res.split(" on ") +mounted_img_store_url = split_res[0] +for img_store in image_stores_response: + img_store_url = str(img_store.url) + if img_store_url.startswith("nfs://"): + img_store_url = img_store_url[6:] +#Add colon after ip address to match output from mount command +first_slash = img_store_url.find('/') +img_store_url = img_store_url[0:first_slash] + ':' + img_store_url[first_slash:] +if img_store_url == mounted_img_store_url: +img_store_id = img_store.id +break --- End diff -- What is this `for` accomplishing? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user jburwell commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r80704271 --- Diff: test/integration/smoke/test_ssvm.py --- @@ -1197,3 +1205,148 @@ def test_10_destroy_cpvm(self): # Call to verify cloud process is running self.test_04_cpvm_internals() return + +@attr( +tags=[ +"advanced", +"advancedns", +"smoke", +"basic", +"sg"], +required_hardware="true") +def test_11_ss_nfs_version_on_ssvm(self): +"""Test NFS Version on Secondary Storage mounted properly on SSVM +""" + + # 1) List SSVM in zone + # 2) Get id and url from mounted nfs store + # 3) Update NFS version for previous image store + # 4) Stop SSVM + # 5) Check NFS version of mounted nfs store after SSVM starts + + nfs_version = self.config.nfsVersion + if nfs_version == None: + self.skipTest('No NFS version provided in test data') + + #List SSVM for zone id +list_ssvm_response = list_ssvms( +self.apiclient, +systemvmtype='secondarystoragevm', +state='Running', +zoneid=self.zone.id +) +self.assertEqual( +isinstance(list_ssvm_response, list), +True, +"Check list response returns a valid list" +) +self.assertEqual( +len(list_ssvm_response), +1, +"Check list System VMs response" +) + +ssvm = list_ssvm_response[0] +image_stores_response = ImageStore.list(self.apiclient,zoneid=self.zone.id) + + if self.hypervisor.lower() in ('vmware', 'hyperv'): +# SSH into SSVMs is done via management server for Vmware and Hyper-V +result = get_process_status( +self.apiclient.connection.mgtSvr, +22, +self.apiclient.connection.user, +self.apiclient.connection.passwd, +ssvm.privateip, +"mount | grep 'type nfs'", +hypervisor=self.hypervisor) + + for res in result: +split_res = res.split(" on ") +mounted_img_store_url = split_res[0] +for img_store in image_stores_response: + img_store_url = str(img_store.url) + if img_store_url.startswith("nfs://"): + img_store_url = img_store_url[6:] +#Add colon after ip address to match output from mount command +first_slash = img_store_url.find('/') +img_store_url = img_store_url[0:first_slash] + ':' + img_store_url[first_slash:] +if img_store_url == mounted_img_store_url: +img_store_id = img_store.id +break + +self.assertNotEqual( +img_store_id, +None, +"Check image store id mounted on SSVM" +) + + #Update NFS version for image store mounted on SSVM +updateConfigurationCmd = updateConfiguration.updateConfigurationCmd() +updateConfigurationCmd.name = "secstorage.nfs.version" +updateConfigurationCmd.value = nfs_version +updateConfigurationCmd.imagestoreuuid = img_store_id + +updateConfigurationResponse = self.apiclient.updateConfiguration(updateConfigurationCmd) +self.logger.debug("updated the parameter %s with value %s"%(updateConfigurationResponse.name, updateConfigurationResponse.value)) + + #Stop SSVM + self.debug("Stopping SSVM: %s" % ssvm.id) +cmd = stopSystemVm.stopSystemVmCmd() +cmd.id = ssvm.id +self.apiclient.stopSystemVm(cmd) + +timeout = self.services["timeout"] +while True: +list_ssvm_response = list_ssvms( +self.apiclient, +id=ssvm.id +) +if isinstance(list_ssvm_response, list): +if list_ssvm_response[0].state == 'Running': +break +if timeout == 0: +raise Exception("List SSVM call failed!") + +time.sleep(self.services["sleep"]) +timeout = timeout - 1 + + self.assertEqual( +isinstance(list_ssvm_response, list), +True, +"Check list response returns a valid list" +) +ssvm = list_ssvm_response[0] +self.debug("SSVM state after debug: %s" % ssvm.state) +self.assertEqual(
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user jburwell commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r80701833 --- Diff: test/integration/smoke/test_ssvm.py --- @@ -1197,3 +1205,148 @@ def test_10_destroy_cpvm(self): # Call to verify cloud process is running self.test_04_cpvm_internals() return + +@attr( +tags=[ +"advanced", +"advancedns", +"smoke", +"basic", +"sg"], +required_hardware="true") +def test_11_ss_nfs_version_on_ssvm(self): +"""Test NFS Version on Secondary Storage mounted properly on SSVM +""" + + # 1) List SSVM in zone + # 2) Get id and url from mounted nfs store + # 3) Update NFS version for previous image store + # 4) Stop SSVM + # 5) Check NFS version of mounted nfs store after SSVM starts + + nfs_version = self.config.nfsVersion + if nfs_version == None: + self.skipTest('No NFS version provided in test data') + + #List SSVM for zone id +list_ssvm_response = list_ssvms( +self.apiclient, +systemvmtype='secondarystoragevm', +state='Running', +zoneid=self.zone.id +) --- End diff -- Please add an assertion that `list_ssvm_response` is not `None`. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user jburwell commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r80702827 --- Diff: test/integration/smoke/test_ssvm.py --- @@ -1197,3 +1205,148 @@ def test_10_destroy_cpvm(self): # Call to verify cloud process is running self.test_04_cpvm_internals() return + +@attr( +tags=[ +"advanced", +"advancedns", +"smoke", +"basic", +"sg"], +required_hardware="true") +def test_11_ss_nfs_version_on_ssvm(self): +"""Test NFS Version on Secondary Storage mounted properly on SSVM +""" + + # 1) List SSVM in zone + # 2) Get id and url from mounted nfs store + # 3) Update NFS version for previous image store + # 4) Stop SSVM + # 5) Check NFS version of mounted nfs store after SSVM starts + + nfs_version = self.config.nfsVersion + if nfs_version == None: + self.skipTest('No NFS version provided in test data') + + #List SSVM for zone id +list_ssvm_response = list_ssvms( +self.apiclient, +systemvmtype='secondarystoragevm', +state='Running', +zoneid=self.zone.id +) +self.assertEqual( +isinstance(list_ssvm_response, list), +True, +"Check list response returns a valid list" +) +self.assertEqual( +len(list_ssvm_response), +1, +"Check list System VMs response" +) + +ssvm = list_ssvm_response[0] +image_stores_response = ImageStore.list(self.apiclient,zoneid=self.zone.id) + + if self.hypervisor.lower() in ('vmware', 'hyperv'): +# SSH into SSVMs is done via management server for Vmware and Hyper-V +result = get_process_status( +self.apiclient.connection.mgtSvr, +22, +self.apiclient.connection.user, +self.apiclient.connection.passwd, +ssvm.privateip, +"mount | grep 'type nfs'", +hypervisor=self.hypervisor) + + for res in result: +split_res = res.split(" on ") +mounted_img_store_url = split_res[0] +for img_store in image_stores_response: + img_store_url = str(img_store.url) + if img_store_url.startswith("nfs://"): + img_store_url = img_store_url[6:] +#Add colon after ip address to match output from mount command +first_slash = img_store_url.find('/') +img_store_url = img_store_url[0:first_slash] + ':' + img_store_url[first_slash:] +if img_store_url == mounted_img_store_url: +img_store_id = img_store.id +break + +self.assertNotEqual( +img_store_id, +None, +"Check image store id mounted on SSVM" +) + + #Update NFS version for image store mounted on SSVM +updateConfigurationCmd = updateConfiguration.updateConfigurationCmd() +updateConfigurationCmd.name = "secstorage.nfs.version" +updateConfigurationCmd.value = nfs_version +updateConfigurationCmd.imagestoreuuid = img_store_id + +updateConfigurationResponse = self.apiclient.updateConfiguration(updateConfigurationCmd) +self.logger.debug("updated the parameter %s with value %s"%(updateConfigurationResponse.name, updateConfigurationResponse.value)) + + #Stop SSVM + self.debug("Stopping SSVM: %s" % ssvm.id) +cmd = stopSystemVm.stopSystemVmCmd() +cmd.id = ssvm.id +self.apiclient.stopSystemVm(cmd) + +timeout = self.services["timeout"] +while True: +list_ssvm_response = list_ssvms( +self.apiclient, +id=ssvm.id +) --- End diff -- Please add an assertions to check the following: * `list_ssvm_response` is not `None` * `list_ssvm_response` is an instance of `list_ssvm_response` * `list_ssvm_response` contains one element with an ID equal to `ssvm.id` Also, why is `list_ssvm_response` being re-used in multiple contexts across this method? Please consider splitting into multiple variables to more clearly express the intent of how each result should be used and avoid overwriting values in a manner that causes inconsistent behavior. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user jburwell commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r80700964 --- Diff: test/integration/smoke/test_ssvm.py --- @@ -1197,3 +1205,148 @@ def test_10_destroy_cpvm(self): # Call to verify cloud process is running self.test_04_cpvm_internals() return + +@attr( +tags=[ +"advanced", +"advancedns", +"smoke", +"basic", +"sg"], +required_hardware="true") +def test_11_ss_nfs_version_on_ssvm(self): +"""Test NFS Version on Secondary Storage mounted properly on SSVM +""" + + # 1) List SSVM in zone + # 2) Get id and url from mounted nfs store + # 3) Update NFS version for previous image store + # 4) Stop SSVM + # 5) Check NFS version of mounted nfs store after SSVM starts + + nfs_version = self.config.nfsVersion + if nfs_version == None: + self.skipTest('No NFS version provided in test data') + + #List SSVM for zone id +list_ssvm_response = list_ssvms( +self.apiclient, +systemvmtype='secondarystoragevm', +state='Running', +zoneid=self.zone.id +) +self.assertEqual( +isinstance(list_ssvm_response, list), +True, +"Check list response returns a valid list" +) +self.assertEqual( +len(list_ssvm_response), +1, +"Check list System VMs response" +) + +ssvm = list_ssvm_response[0] +image_stores_response = ImageStore.list(self.apiclient,zoneid=self.zone.id) + + if self.hypervisor.lower() in ('vmware', 'hyperv'): +# SSH into SSVMs is done via management server for Vmware and Hyper-V +result = get_process_status( +self.apiclient.connection.mgtSvr, +22, +self.apiclient.connection.user, +self.apiclient.connection.passwd, +ssvm.privateip, +"mount | grep 'type nfs'", +hypervisor=self.hypervisor) + + for res in result: +split_res = res.split(" on ") +mounted_img_store_url = split_res[0] +for img_store in image_stores_response: + img_store_url = str(img_store.url) + if img_store_url.startswith("nfs://"): + img_store_url = img_store_url[6:] +#Add colon after ip address to match output from mount command +first_slash = img_store_url.find('/') +img_store_url = img_store_url[0:first_slash] + ':' + img_store_url[first_slash:] +if img_store_url == mounted_img_store_url: +img_store_id = img_store.id +break + +self.assertNotEqual( +img_store_id, +None, +"Check image store id mounted on SSVM" +) + + #Update NFS version for image store mounted on SSVM +updateConfigurationCmd = updateConfiguration.updateConfigurationCmd() +updateConfigurationCmd.name = "secstorage.nfs.version" +updateConfigurationCmd.value = nfs_version +updateConfigurationCmd.imagestoreuuid = img_store_id + +updateConfigurationResponse = self.apiclient.updateConfiguration(updateConfigurationCmd) +self.logger.debug("updated the parameter %s with value %s"%(updateConfigurationResponse.name, updateConfigurationResponse.value)) + + #Stop SSVM + self.debug("Stopping SSVM: %s" % ssvm.id) +cmd = stopSystemVm.stopSystemVmCmd() +cmd.id = ssvm.id +self.apiclient.stopSystemVm(cmd) + +timeout = self.services["timeout"] +while True: +list_ssvm_response = list_ssvms( +self.apiclient, +id=ssvm.id +) +if isinstance(list_ssvm_response, list): +if list_ssvm_response[0].state == 'Running': +break +if timeout == 0: +raise Exception("List SSVM call failed!") + +time.sleep(self.services["sleep"]) +timeout = timeout - 1 --- End diff -- Please consider replacing this `while` loop (lines 1299-1311) with `utils.waitUntil` as it provides a Pythonic/tested implementation of waiting for a condition to complete. It would also clarify the intent of this section. --- If your project is set up for it, you can reply to this email
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user jburwell commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r80701499 --- Diff: test/integration/smoke/test_ssvm.py --- @@ -1197,3 +1205,148 @@ def test_10_destroy_cpvm(self): # Call to verify cloud process is running self.test_04_cpvm_internals() return + +@attr( +tags=[ +"advanced", +"advancedns", +"smoke", +"basic", +"sg"], +required_hardware="true") +def test_11_ss_nfs_version_on_ssvm(self): +"""Test NFS Version on Secondary Storage mounted properly on SSVM +""" + + # 1) List SSVM in zone + # 2) Get id and url from mounted nfs store + # 3) Update NFS version for previous image store + # 4) Stop SSVM + # 5) Check NFS version of mounted nfs store after SSVM starts + + nfs_version = self.config.nfsVersion + if nfs_version == None: + self.skipTest('No NFS version provided in test data') + + #List SSVM for zone id +list_ssvm_response = list_ssvms( +self.apiclient, +systemvmtype='secondarystoragevm', +state='Running', +zoneid=self.zone.id +) +self.assertEqual( +isinstance(list_ssvm_response, list), +True, +"Check list response returns a valid list" +) +self.assertEqual( +len(list_ssvm_response), +1, +"Check list System VMs response" +) + +ssvm = list_ssvm_response[0] +image_stores_response = ImageStore.list(self.apiclient,zoneid=self.zone.id) + + if self.hypervisor.lower() in ('vmware', 'hyperv'): +# SSH into SSVMs is done via management server for Vmware and Hyper-V +result = get_process_status( +self.apiclient.connection.mgtSvr, +22, +self.apiclient.connection.user, +self.apiclient.connection.passwd, +ssvm.privateip, +"mount | grep 'type nfs'", +hypervisor=self.hypervisor) + + for res in result: +split_res = res.split(" on ") +mounted_img_store_url = split_res[0] +for img_store in image_stores_response: + img_store_url = str(img_store.url) + if img_store_url.startswith("nfs://"): + img_store_url = img_store_url[6:] +#Add colon after ip address to match output from mount command +first_slash = img_store_url.find('/') +img_store_url = img_store_url[0:first_slash] + ':' + img_store_url[first_slash:] +if img_store_url == mounted_img_store_url: +img_store_id = img_store.id +break + +self.assertNotEqual( +img_store_id, +None, +"Check image store id mounted on SSVM" +) + + #Update NFS version for image store mounted on SSVM +updateConfigurationCmd = updateConfiguration.updateConfigurationCmd() +updateConfigurationCmd.name = "secstorage.nfs.version" +updateConfigurationCmd.value = nfs_version +updateConfigurationCmd.imagestoreuuid = img_store_id + +updateConfigurationResponse = self.apiclient.updateConfiguration(updateConfigurationCmd) +self.logger.debug("updated the parameter %s with value %s"%(updateConfigurationResponse.name, updateConfigurationResponse.value)) + + #Stop SSVM + self.debug("Stopping SSVM: %s" % ssvm.id) --- End diff -- Please consider changing the log level to `INFO` as this information seems like it would be useful for monitoring the progress of the test. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user jburwell commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r80702139 --- Diff: test/integration/smoke/test_ssvm.py --- @@ -42,12 +43,19 @@ class TestSSVMs(cloudstackTestCase): def setUp(self): + test_case = super(TestSSVMs, self) self.apiclient = self.testClient.getApiClient() self.hypervisor = self.testClient.getHypervisorInfo() self.cleanup = [] + self.config = test_case.getClsConfig() self.services = self.testClient.getParsedTestDataConfig() self.zone = get_zone(self.apiclient, self.testClient.getZoneForTests()) +self.logger = logging.getLogger('TestSSVMs') +self.stream_handler = logging.StreamHandler() +self.logger.setLevel(logging.DEBUG) +self.logger.addHandler(self.stream_handler) + # Default sleep is set to 90 seconds, which is too long if the SSVM takes up to 2min to start. # Second sleep in the loop will waste test time. self.services["sleep"] = 30 --- End diff -- I recognize that this value is out of the scope of this change. However, would it make sense to reduce this value to 5 seconds? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user nvazquez commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r79419754 --- Diff: test/integration/smoke/test_ssvm.py --- @@ -1197,3 +1205,148 @@ def test_10_destroy_cpvm(self): # Call to verify cloud process is running self.test_04_cpvm_internals() return + +@attr( +tags=[ +"advanced", +"advancedns", +"smoke", +"basic", +"sg"], +required_hardware="true") --- End diff -- Done, thanks @koushik-das --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user nvazquez commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r79389208 --- Diff: test/integration/smoke/test_ssvm.py --- @@ -1197,3 +1205,148 @@ def test_10_destroy_cpvm(self): # Call to verify cloud process is running self.test_04_cpvm_internals() return + +@attr( +tags=[ +"advanced", +"advancedns", +"smoke", +"basic", +"sg"], +required_hardware="false") --- End diff -- Done, thanks! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user koushik-das commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r79357165 --- Diff: test/integration/smoke/test_ssvm.py --- @@ -1197,3 +1205,148 @@ def test_10_destroy_cpvm(self): # Call to verify cloud process is running self.test_04_cpvm_internals() return + +@attr( +tags=[ +"advanced", +"advancedns", +"smoke", +"basic", +"sg"], +required_hardware="false") --- End diff -- Is it possible to run this test on Simulator? If not please change required_hardware to true. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user jburwell commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r74079012 --- Diff: server/src/com/cloud/storage/ImageStoreDetailsUtil.java --- @@ -20,48 +20,68 @@ import javax.inject.Inject; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.cloudstack.framework.config.impl.ConfigurationVO; import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; +import com.cloud.capacity.CapacityManager; + public class ImageStoreDetailsUtil { @Inject protected ImageStoreDao imageStoreDao; @Inject protected ImageStoreDetailsDao imageStoreDetailsDao; +@Inject +protected ConfigurationDao configurationDao; /** - * Obtain NFS protocol version (if provided) for a store id. - * It can be set by adding an entry in {@code image_store_details} table, providing {@code name=nfs.version} and {@code value=X} (e.g. 3) + * Retrieve global secondary storage NFS version default value + * @return global default value + */ +protected Integer getGlobalDefaultNfsVersion(){ +ConfigurationVO globalNfsVersion = configurationDao.findByName(CapacityManager.ImageStoreNFSVersion.key()); +String value = globalNfsVersion.getValue(); --- End diff -- @nvazquez that seems like a good solution. Also, please use ``Preconditions.checkState(globalNfsVersion !=null, "Unable to find global NFS version for version key" + CapacityManager.ImageStoreNFSVersion.key())`` to help someone understand why it happened in the unlikely event it occurs. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user nvazquez commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r74068727 --- Diff: server/src/com/cloud/storage/ImageStoreDetailsUtil.java --- @@ -20,48 +20,68 @@ import javax.inject.Inject; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.cloudstack.framework.config.impl.ConfigurationVO; import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; +import com.cloud.capacity.CapacityManager; + public class ImageStoreDetailsUtil { @Inject protected ImageStoreDao imageStoreDao; @Inject protected ImageStoreDetailsDao imageStoreDetailsDao; +@Inject +protected ConfigurationDao configurationDao; /** - * Obtain NFS protocol version (if provided) for a store id. - * It can be set by adding an entry in {@code image_store_details} table, providing {@code name=nfs.version} and {@code value=X} (e.g. 3) + * Retrieve global secondary storage NFS version default value + * @return global default value + */ +protected Integer getGlobalDefaultNfsVersion(){ +ConfigurationVO globalNfsVersion = configurationDao.findByName(CapacityManager.ImageStoreNFSVersion.key()); +String value = globalNfsVersion.getValue(); --- End diff -- @jburwell I didn't consider it because configuration will be added and it will be present unless it is explicitly deleted from db. Anyways, I can add `Preconditions.checkState(globalNfsVersion != null)` do you agree? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user nvazquez commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r74067494 --- Diff: server/src/com/cloud/storage/ImageStoreDetailsUtil.java --- @@ -20,48 +20,68 @@ import javax.inject.Inject; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.cloudstack.framework.config.impl.ConfigurationVO; import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; +import com.cloud.capacity.CapacityManager; + public class ImageStoreDetailsUtil { @Inject protected ImageStoreDao imageStoreDao; @Inject protected ImageStoreDetailsDao imageStoreDetailsDao; +@Inject +protected ConfigurationDao configurationDao; /** - * Obtain NFS protocol version (if provided) for a store id. - * It can be set by adding an entry in {@code image_store_details} table, providing {@code name=nfs.version} and {@code value=X} (e.g. 3) + * Retrieve global secondary storage NFS version default value + * @return global default value + */ +protected Integer getGlobalDefaultNfsVersion(){ +ConfigurationVO globalNfsVersion = configurationDao.findByName(CapacityManager.ImageStoreNFSVersion.key()); +String value = globalNfsVersion.getValue(); +return (value != null ? Integer.valueOf(value) : null); +} +/** + * Obtain NFS protocol version (if provided) for a store id, if not use default config value * @param storeId image store id - * @return {@code null} if {@code nfs.version} is not found for storeId - * {@code X} if {@code nfs.version} is found found for storeId + * @return {@code null} if {@code secstorage.nfs.version} is not found for storeId + * {@code X} if {@code secstorage.nfs.version} is found found for storeId */ public Integer getNfsVersion(long storeId) throws NumberFormatException { -String nfsVersion = null; +Integer nfsVersion; if (imageStoreDetailsDao.getDetails(storeId) != null){ MapstoreDetails = imageStoreDetailsDao.getDetails(storeId); --- End diff -- Done --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user nvazquez commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r74067440 --- Diff: server/src/com/cloud/configuration/ConfigurationManagerImpl.java --- @@ -520,6 +528,16 @@ public String updateConfiguration(final long userId, final String name, final St _accountDetailsDao.update(accountDetailVO.getId(), accountDetailVO); } break; + +case ImageStore: +final ImageStoreVO imgStore = _imageStoreDao.findById(resourceId); +if (imgStore == null) { +throw new InvalidParameterValueException("unable to find image store by id " + resourceId); --- End diff -- Thanks, I'll take it into account for next time --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user jburwell commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r73766528 --- Diff: server/src/com/cloud/storage/ImageStoreDetailsUtil.java --- @@ -20,48 +20,68 @@ import javax.inject.Inject; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.cloudstack.framework.config.impl.ConfigurationVO; import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; +import com.cloud.capacity.CapacityManager; + public class ImageStoreDetailsUtil { @Inject protected ImageStoreDao imageStoreDao; @Inject protected ImageStoreDetailsDao imageStoreDetailsDao; +@Inject +protected ConfigurationDao configurationDao; /** - * Obtain NFS protocol version (if provided) for a store id. - * It can be set by adding an entry in {@code image_store_details} table, providing {@code name=nfs.version} and {@code value=X} (e.g. 3) + * Retrieve global secondary storage NFS version default value + * @return global default value + */ +protected Integer getGlobalDefaultNfsVersion(){ +ConfigurationVO globalNfsVersion = configurationDao.findByName(CapacityManager.ImageStoreNFSVersion.key()); +String value = globalNfsVersion.getValue(); +return (value != null ? Integer.valueOf(value) : null); +} +/** + * Obtain NFS protocol version (if provided) for a store id, if not use default config value * @param storeId image store id - * @return {@code null} if {@code nfs.version} is not found for storeId - * {@code X} if {@code nfs.version} is found found for storeId + * @return {@code null} if {@code secstorage.nfs.version} is not found for storeId + * {@code X} if {@code secstorage.nfs.version} is found found for storeId */ public Integer getNfsVersion(long storeId) throws NumberFormatException { -String nfsVersion = null; +Integer nfsVersion; if (imageStoreDetailsDao.getDetails(storeId) != null){ MapstoreDetails = imageStoreDetailsDao.getDetails(storeId); --- End diff -- I recognize that this code predates this patch, but it is a double API call. Consider the following refactoring: ``` public Integer getNfsVersion(long storeId) throws NumberFormatException { final Map storeDetails = imageStoreDetailsDao.getDetails(storeId); if (storeDetails != null && storeDetails.containsKey(CapacityManager.ImageStoreNFSVersion.key())) { final String version = storeDetails.get(CapacityManager.ImageStoreNFSVersion.key()); return (val != null ? Integer.valueOf(val) : null); } return getGlobalDefaultNfsVersion(); } ``` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user jburwell commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r73765825 --- Diff: server/src/com/cloud/storage/ImageStoreDetailsUtil.java --- @@ -20,48 +20,68 @@ import javax.inject.Inject; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.cloudstack.framework.config.impl.ConfigurationVO; import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; +import com.cloud.capacity.CapacityManager; + public class ImageStoreDetailsUtil { @Inject protected ImageStoreDao imageStoreDao; @Inject protected ImageStoreDetailsDao imageStoreDetailsDao; +@Inject +protected ConfigurationDao configurationDao; /** - * Obtain NFS protocol version (if provided) for a store id. - * It can be set by adding an entry in {@code image_store_details} table, providing {@code name=nfs.version} and {@code value=X} (e.g. 3) + * Retrieve global secondary storage NFS version default value + * @return global default value + */ +protected Integer getGlobalDefaultNfsVersion(){ +ConfigurationVO globalNfsVersion = configurationDao.findByName(CapacityManager.ImageStoreNFSVersion.key()); +String value = globalNfsVersion.getValue(); +return (value != null ? Integer.valueOf(value) : null); +} +/** + * Obtain NFS protocol version (if provided) for a store id, if not use default config value * @param storeId image store id - * @return {@code null} if {@code nfs.version} is not found for storeId - * {@code X} if {@code nfs.version} is found found for storeId + * @return {@code null} if {@code secstorage.nfs.version} is not found for storeId + * {@code X} if {@code secstorage.nfs.version} is found found for storeId */ public Integer getNfsVersion(long storeId) throws NumberFormatException { -String nfsVersion = null; +Integer nfsVersion; --- End diff -- Variables should always be initialized. Consider initializing to ``null``. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user jburwell commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r73765615 --- Diff: server/src/com/cloud/storage/ImageStoreDetailsUtil.java --- @@ -20,48 +20,68 @@ import javax.inject.Inject; +import org.apache.cloudstack.framework.config.dao.ConfigurationDao; +import org.apache.cloudstack.framework.config.impl.ConfigurationVO; import org.apache.cloudstack.storage.datastore.db.ImageStoreDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreDetailsDao; import org.apache.cloudstack.storage.datastore.db.ImageStoreVO; +import com.cloud.capacity.CapacityManager; + public class ImageStoreDetailsUtil { @Inject protected ImageStoreDao imageStoreDao; @Inject protected ImageStoreDetailsDao imageStoreDetailsDao; +@Inject +protected ConfigurationDao configurationDao; /** - * Obtain NFS protocol version (if provided) for a store id. - * It can be set by adding an entry in {@code image_store_details} table, providing {@code name=nfs.version} and {@code value=X} (e.g. 3) + * Retrieve global secondary storage NFS version default value + * @return global default value + */ +protected Integer getGlobalDefaultNfsVersion(){ +ConfigurationVO globalNfsVersion = configurationDao.findByName(CapacityManager.ImageStoreNFSVersion.key()); +String value = globalNfsVersion.getValue(); --- End diff -- What if ``globalNfsVersion`` is null? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user jburwell commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r73765448 --- Diff: server/src/com/cloud/configuration/ConfigurationManagerImpl.java --- @@ -520,6 +528,16 @@ public String updateConfiguration(final long userId, final String name, final St _accountDetailsDao.update(accountDetailVO.getId(), accountDetailVO); } break; + +case ImageStore: +final ImageStoreVO imgStore = _imageStoreDao.findById(resourceId); +if (imgStore == null) { +throw new InvalidParameterValueException("unable to find image store by id " + resourceId); --- End diff -- ``InvalidParameterValueException`` is used to indicate that an invalid parameter was passed by a caller to a method. Since ``imgStore`` was not passed into this method, it does not seem appropriate to use this exception type. ``IllegalStateException`` seems more appropriate. Also, consider using ``Preconditions.checkState`` to replace the ``if`` block with a single line of code for greater concession. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user jburwell commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r73764835 --- Diff: api/src/org/apache/cloudstack/api/command/admin/config/UpdateCfgCmd.java --- @@ -145,6 +156,9 @@ public void execute() { if (getAccountId() != null) { response.setScope("account"); } +if (getImageStoreId() != null) { +response.setScope("imagestore"); +} --- End diff -- Please remove this check and adding ``validations = PositiveNumber`` to the ``imageStoreId @Parameter`` annotation. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user koushik-das commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r73283060 --- Diff: setup/db/db/schema-481to490.sql --- @@ -545,3 +545,6 @@ INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervis INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created, is_user_defined) VALUES (UUID(), 'VMware', '5.0', 'centos64Guest', 228, now(), 0); INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created, is_user_defined) VALUES (UUID(), 'VMware', '5.1', 'centos64Guest', 228, now(), 0); INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created, is_user_defined) VALUES (UUID(), 'VMware', '5.5', 'centos64Guest', 228, now(), 0); + +ALTER TABLE `cloud`.`image_store_details` CHANGE COLUMN `value` `value` VARCHAR(255) NULL DEFAULT NULL COMMENT 'value of the detail', ADD COLUMN `display` tinyint(1) NOT --- End diff -- About the display field, I didn't see that in cluster details. So looks like detail VOs implementing ResourceDetail have it, others don't. So for this PR it is fine but will be good to make them consistent across all detail VOs. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user koushik-das commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r73281219 --- Diff: engine/schema/src/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java --- @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.cloudstack.storage.image.db; --- End diff -- Missed that out previously, thanks --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user nvazquez commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r73156984 --- Diff: engine/schema/src/org/apache/cloudstack/storage/datastore/db/ImageStoreDetailVO.java --- @@ -23,61 +23,61 @@ import javax.persistence.Id; import javax.persistence.Table; -import org.apache.cloudstack.api.InternalIdentity; +import org.apache.cloudstack.api.ResourceDetail; @Entity @Table(name = "image_store_details") -public class ImageStoreDetailVO implements InternalIdentity { +public class ImageStoreDetailVO implements ResourceDetail { @Id @GeneratedValue(strategy = GenerationType.IDENTITY) @Column(name = "id") long id; @Column(name = "store_id") -long storeId; +long resourceId; @Column(name = "name") String name; @Column(name = "value") String value; +@Column(name = "display") --- End diff -- We added it to make it consistent with `ResourceDetail` as we made `ImageStoreDetailVO` implement `ResourceDetail`, which has the method `isDisplay`. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user nvazquez commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r73156809 --- Diff: setup/db/db/schema-481to490.sql --- @@ -545,3 +545,6 @@ INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervis INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created, is_user_defined) VALUES (UUID(), 'VMware', '5.0', 'centos64Guest', 228, now(), 0); INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created, is_user_defined) VALUES (UUID(), 'VMware', '5.1', 'centos64Guest', 228, now(), 0); INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created, is_user_defined) VALUES (UUID(), 'VMware', '5.5', 'centos64Guest', 228, now(), 0); + +ALTER TABLE `cloud`.`image_store_details` CHANGE COLUMN `value` `value` VARCHAR(255) NULL DEFAULT NULL COMMENT 'value of the detail', ADD COLUMN `display` tinyint(1) NOT --- End diff -- We decided to make the default value as null to not impact deployments which didn't specify a NFS version for their secondary storage operations, so they work as they were before. About *display* column, we added it to make it consistent with `ResourceDetail` as we made `ImageStoreDetailVO` implement `ResourceDetail`, which has the method `isDisplay`. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user nvazquez commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r73154622 --- Diff: engine/schema/src/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java --- @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.cloudstack.storage.image.db; --- End diff -- Hi @koushik-das, the reason was to avoid a dependency cycle on Maven poms, I tried explaining it under *Important* section in this PR's description --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user koushik-das commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r73118447 --- Diff: engine/schema/src/org/apache/cloudstack/storage/datastore/db/ImageStoreDetailVO.java --- @@ -23,61 +23,61 @@ import javax.persistence.Id; import javax.persistence.Table; -import org.apache.cloudstack.api.InternalIdentity; +import org.apache.cloudstack.api.ResourceDetail; @Entity @Table(name = "image_store_details") -public class ImageStoreDetailVO implements InternalIdentity { +public class ImageStoreDetailVO implements ResourceDetail { @Id @GeneratedValue(strategy = GenerationType.IDENTITY) @Column(name = "id") long id; @Column(name = "store_id") -long storeId; +long resourceId; @Column(name = "name") String name; @Column(name = "value") String value; +@Column(name = "display") --- End diff -- What is the need for this field? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user koushik-das commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r73118335 --- Diff: setup/db/db/schema-481to490.sql --- @@ -545,3 +545,6 @@ INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervis INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created, is_user_defined) VALUES (UUID(), 'VMware', '5.0', 'centos64Guest', 228, now(), 0); INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created, is_user_defined) VALUES (UUID(), 'VMware', '5.1', 'centos64Guest', 228, now(), 0); INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created, is_user_defined) VALUES (UUID(), 'VMware', '5.5', 'centos64Guest', 228, now(), 0); + +ALTER TABLE `cloud`.`image_store_details` CHANGE COLUMN `value` `value` VARCHAR(255) NULL DEFAULT NULL COMMENT 'value of the detail', ADD COLUMN `display` tinyint(1) NOT --- End diff -- Also what is the purpose of the display field? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user koushik-das commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r73118216 --- Diff: setup/db/db/schema-481to490.sql --- @@ -545,3 +545,6 @@ INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervis INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created, is_user_defined) VALUES (UUID(), 'VMware', '5.0', 'centos64Guest', 228, now(), 0); INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created, is_user_defined) VALUES (UUID(), 'VMware', '5.1', 'centos64Guest', 228, now(), 0); INSERT IGNORE INTO `cloud`.`guest_os_hypervisor` (uuid,hypervisor_type, hypervisor_version, guest_os_name, guest_os_id, created, is_user_defined) VALUES (UUID(), 'VMware', '5.5', 'centos64Guest', 228, now(), 0); + +ALTER TABLE `cloud`.`image_store_details` CHANGE COLUMN `value` `value` VARCHAR(255) NULL DEFAULT NULL COMMENT 'value of the detail', ADD COLUMN `display` tinyint(1) NOT --- End diff -- Any reason to make the value field nullable? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
Github user koushik-das commented on a diff in the pull request: https://github.com/apache/cloudstack/pull/1615#discussion_r73114913 --- Diff: engine/schema/src/org/apache/cloudstack/storage/datastore/db/ImageStoreDaoImpl.java --- @@ -16,7 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -package org.apache.cloudstack.storage.image.db; --- End diff -- What is the reason for moving this file to a new location? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] cloudstack pull request #1615: CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Ma...
GitHub user nvazquez opened a pull request: https://github.com/apache/cloudstack/pull/1615 CLOUDSTACK-9438: Fix for CLOUDSTACK-9252 - Make NFS version changeable in UI JIRA TICKET: https://issues.apache.org/jira/browse/CLOUDSTACK-9438 ### Introduction From #1361 it was possible to configure NFS version for secondary storage mount. However, changing NFS version requires inserting an new detail on `image_store_details` table, with `name = 'nfs.version'` and `value = X` where X is desired NFS version, and then restarting management server for changes to take effect. Our improvement aims to make NFS version changeable from UI, instead of previously described workflow. ### Proposed solution Basically, NFS version is defined as an image store ConfigKey, this implied: * Adding a new Config scope: **ImageStore** * Make `ImageStoreDetailsDao` class to extend `ResourceDetailsDaoBase` and `ImageStoreDetailVO` implement `ResourceDetail` * Insert `'display'` column on `image_store_details` table * Extending `ListCfgsCmd` and `UpdateCfgCmd` to support **ImageStore** scope, which implied: ** Injecting `ImageStoreDetailsDao` and `ImageStoreDao` on `ConfigurationManagerImpl` class, on `cloud-server` module. ### Important It is important to mention that `ImageStoreDaoImpl` and `ImageStoreDetailsDaoImpl` classes were moved from `cloud-engine-storage` to `cloud-engine-schema` module in order to Spring find those beans to inject on `ConfigurationManagerImpl` in `cloud-server` module. We had this maven dependencies between modules: * `cloud-server --> cloud-engine-schema` * `cloud-engine-storage --> cloud-secondary-storage --> cloud-server` As `ImageStoreDaoImpl` and `ImageStoreDetailsDao` were defined in `cloud-engine-storage`, and they needed in `cloud-server` module, to be injected on `ConfigurationManagerImpl`, if we added dependency from `cloud-server` to `cloud-engine-storage` we would introduce a dependency cycle. To avoid this cycle, we moved those classes to `cloud-engine-schema` module You can merge this pull request into a Git repository by running: $ git pull https://github.com/nvazquez/cloudstack nfsConfigKey Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cloudstack/pull/1615.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1615 commit c35d9e6dfc6a886890ce2c1c864ec201884cb84c Author: nvazquezDate: 2016-07-12T14:41:52Z CLOUDSTACK-9438: NFS version as ConfigKey commit 0e44c8571072f67fc53c2851dc06cd0ced2a6755 Author: nvazquez Date: 2016-07-19T21:33:25Z CLOUDSTACK-9438: Move ImageStoreDaoImpl and ImageStoreDetailsDaoImpl to engine-schema module commit 8383a36b703b9d5166cb1269f8400ca193f20ea1 Author: nvazquez Date: 2016-07-21T19:14:07Z CLOUDSTACK-9438: Add display to ImageStoreDetailVO --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---