diff --git a/bibigrid/core/actions/create.py b/bibigrid/core/actions/create.py index c8c8f543..e12af3bd 100644 --- a/bibigrid/core/actions/create.py +++ b/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) diff --git a/bibigrid/core/provider.py b/bibigrid/core/provider.py index 759c29f8..f1968aaa 100644 --- a/bibigrid/core/provider.py +++ b/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 diff --git a/bibigrid/openstack/openstack_provider.py b/bibigrid/openstack/openstack_provider.py index c50e85ad..7c8d7b64 100644 --- a/bibigrid/openstack/openstack_provider.py +++ b/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): """ diff --git a/resources/playbook/roles/bibigrid/files/slurm/create_server.py b/resources/playbook/roles/bibigrid/files/slurm/create_server.py index b5f3fe71..afe1f22f 100644 --- a/resources/playbook/roles/bibigrid/files/slurm/create_server.py +++ b/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) diff --git a/tests/unit_tests/provider/test_provider.py b/tests/unit_tests/provider/test_provider.py index 506d8275..1913a46b 100644 --- a/tests/unit_tests/provider/test_provider.py +++ b/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))