updated tests and comments

bibigrid/core/actions/create.py

@@ -328,7 +328,7 @@ def create_server_volumes(self, provider, instance, name):
                         raise ConfigurationException(f"Snapshot {volume['snapshot']} not found!")
                 else:
                     self.log.debug("Creating volume...")
-                    return_volume = provider.create_volume(size=volume.get("size", 50), name=volume["name"],
+                    return_volume = provider.create_volume(name=volume["name"], size=volume.get("size", 50),
                                                            volume_type=volume.get("type"),
                                                            description=f"Created for {name}")
             return_volumes.append(return_volume)

bibigrid/core/provider.py

@@ -279,11 +279,12 @@ def get_security_group(self, name_or_id):
         """
 
     @abstractmethod
-    def create_volume(self, name, size, volume_type=None, description=None):
+    def create_volume(self, *, name, size, wait=True, volume_type=None, description=None):
         """
         Creates a volume
         @param name: name of the created volume
         @param size: size of the created volume in GB
+        @param wait: if true waits for volume to be created
         @param volume_type: depends on the location, but for example NVME or HDD
         @param description: a non-functional description to help dashboard users
         @return: the created volume

bibigrid/openstack/openstack_provider.py

@@ -114,7 +114,8 @@ def get_subnet_by_id_or_name(self, subnet_id_or_name):
     def list_servers(self):
         return [elem.toDict() for elem in self.conn.list_servers()]
 
-    def create_server(self, name, flavor, image, network, key_name=None, wait=True, volumes=None, security_groups=None, # pylint: disable=too-many-positional-arguments
+    def create_server(self, name, flavor, image, network, key_name=None, wait=True, volumes=None, security_groups=None,
+                      # pylint: disable=too-many-positional-arguments
                       boot_volume=None, boot_from_volume=False, terminate_boot_volume=False, volume_size=50):
         try:
             server = self.conn.create_server(name=name, flavor=flavor, image=image, network=network, key_name=key_name,
@@ -343,16 +344,18 @@ def get_server(self, name_or_id):
         """
         return self.conn.get_server(name_or_id)
 
-    def create_volume(self, name, size, volume_type=None, description=None):
+    def create_volume(self, *, name, size, wait=True, volume_type=None, description=None):
         """
         Creates a volume
         @param name: name of the created volume
         @param size: size of the created volume in GB
+        @param wait: if true waits for volume to be created
         @param volume_type: depends on the location, but for example NVME or HDD
         @param description: a non-functional description to help dashboard users
         @return: the created volume
         """
-        return self.conn.create_volume(size=size, name=name, volume_type=volume_type, description=description)
+        return self.conn.create_volume(size=size, name=name, wait=wait, volume_type=volume_type,
+                                       description=description)
 
     def delete_volume(self, name_or_id):
         """

resources/playbook/roles/bibigrid/files/slurm/create_server.py

@@ -93,8 +93,7 @@ def create_volume_from_snapshot(connection, snapshot_name_or_id, volume_name_or_
             size = snapshot["size"]
             name = volume_name_or_id or f"bibigrid-{snapshot['name']}"
             description = f"Created from snapshot {snapshot_name_or_id} by BiBiGrid"
-            volume = connection.create_volume(size=size, snapshot_id=snapshot["id"], name=name,
-                                              description=description)
+            volume = connection.create_volume(name=name, size=size, description=description)
             return volume.toDict()
         logging.warning("Snapshot %s is %s; must be available.", snapshot_name_or_id, snapshot['status'])
     else:
@@ -136,7 +135,7 @@ def create_server_volumes(connection, host_vars, name):
                     raise ConfigurationException(f"Snapshot {volume['snapshot']} not found!")
             else:
                 logging.debug("Creating volume...")
-                return_volume = connection.create_volume(size=volume.get("size", 50), name=volume['name'],
+                return_volume = connection.create_volume(name=volume['name'], size=volume.get("size", 50),
                                                          volume_type=volume.get("type"),
                                                          description=f"Created for {name}")
         return_volumes.append(return_volume)

tests/unit_tests/provider/test_provider.py

@@ -5,6 +5,7 @@
 import logging
 import os
 import unittest
+import time # TODO remove dirty test
 
 import bibigrid.core.utility.paths.basic_path as bP
 from bibigrid.core import startup
@@ -53,12 +54,9 @@
 SNAPSHOT_KEYS = {'id', 'created_at', 'updated_at', 'name', 'description', 'volume_id', 'status', 'size', 'metadata',
                  'os-extended-snapshot-attributes:project_id', 'os-extended-snapshot-attributes:progress'}
 
-VOLUME_KEYS = {'location', 'id', 'name', 'description', 'size', 'attachments', 'status', 'migration_status', 'host',
-               'replication_driver', 'replication_status', 'replication_extended_status', 'snapshot_id', 'created_at',
-               'updated_at', 'source_volume_id', 'consistencygroup_id', 'volume_type', 'metadata', 'is_bootable',
-               'is_encrypted', 'can_multiattach', 'properties', 'display_name', 'display_description', 'bootable',
-               'encrypted', 'multiattach', 'availability_zone', 'source_volid', 'user_id',
-               'os-vol-tenant-attr:tenant_id'}
+VOLUME_KEYS = {'description', 'id', 'metadata', 'availability_zone', 'replication_status', 'status', 'links', 'size',
+               'snapshot_id', 'attachments', 'encrypted', 'bootable', 'name', 'source_volid', 'updated_at',
+               'volume_type', 'user_id', 'created_at', 'multiattach', 'consistencygroup_id'}
 
 FREE_RESOURCES_KEYS = {'total_cores', 'floating_ips', 'instances', 'total_ram', 'volumes', 'volume_gigabytes',
                        'snapshots', 'backups', 'backup_gigabytes'}
@@ -164,8 +162,8 @@ def test_active_server_methods(self):
                 self.assertEqual("bibigrid_test_keypair", provider_server["key_name"])
                 self.assertEqual(FLOATING_IP_KEYS, set(floating_ip.keys()))
                 list_server = next(server for server in server_list if
-                                    server["name"] == "bibigrid_test_server" and server[
-                                        "public_v4"] == floating_ip.floating_ip_address)
+                                   server["name"] == "bibigrid_test_server" and server[
+                                       "public_v4"] == floating_ip.floating_ip_address)
                 self.assertEqual("bibigrid_test_server", get_server["name"])
                 self.assertEqual(get_server, list_server)
             provider.delete_keypair("bibigrid_test_keypair")
@@ -233,13 +231,13 @@ def test_create_delete_volume(self):
         @return:
         """
         for provider in PROVIDERS:
-            volume_id = provider.create_volume(name="test_create_delete_volume", size=1)
+            volume_id = provider.create_volume(name="test_create_delete_volume", size=1, description="Test run")
             self.assertTrue(volume_id)
             volume = provider.get_volume_by_id_or_name(volume_id)
-            self.assertTrue(volume)
+            self.assertEqual(VOLUME_KEYS, set(volume))
             self.assertEqual("test_create_delete_volume", volume["name"])
             self.assertTrue(provider.delete_volume(volume_id))
-
+            # maybe explicitly look up that the volume has been deleted
 
     # TODO test_get_images
     # TODO test_get_flavors
@@ -260,7 +258,12 @@ def test_get_snapshot(self):
         def test_create_volume_from_snapshot_with_delete(self):
             for provider, configuration in zip(PROVIDERS, CONFIGURATIONS):
                 with self.subTest(provider.NAME):
-                    volume_id = provider.create_volume_from_snapshot(configuration["snapshotImage"])
+                    volume_name = "test_create_volume_from_snapshot_with_delete"
+                    volume_id = provider.create_volume_from_snapshot(snapshot_name_or_id=configuration["snapshotImage"],
+                                                                     volume_name_or_id=volume_name,
+                                                                     description="Test run")
                     volume = provider.get_volume_by_id_or_name(volume_id)
                     self.assertEqual(VOLUME_KEYS, set(volume.keys()))
-                provider.delete_volume(volume_id)
+                    self.assertEqual(volume_name, volume["name"])
+                time.sleep(1) # TODO remove dirty test
+                self.assertTrue(provider.delete_volume(volume_id))