From 628f43a2e76517a2ef67e97b4ffbf05d8022852d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adolfo=20G=C3=B3mez=20Garc=C3=ADa?= Date: Sun, 28 Apr 2024 16:33:24 +0200 Subject: [PATCH] Removed unused (since years ago) "must_assign_manually" --- server/src/uds/REST/methods/services.py | 5 +- .../core/services/generics/dynamic/service.py | 1 - .../core/services/generics/fixed/service.py | 1 - .../src/uds/core/services/provider_factory.py | 9 +-- server/src/uds/core/services/service.py | 21 ----- .../src/uds/services/OVirt/service_linked.py | 3 - server/src/uds/services/OpenGnsys/service.py | 3 - server/src/uds/services/OpenNebula/service.py | 3 - server/src/uds/services/OpenStack/service.py | 3 - .../PhysicalMachines/service_multi.py | 25 +++--- .../PhysicalMachines/service_single.py | 1 - .../uds/services/Proxmox/service_linked.py | 3 - server/src/uds/services/Sample/service.py | 4 - server/src/uds/services/Test/service.py | 2 - server/src/uds/services/Xen/service.py | 3 - .../physical_machines/test_service_multi.py | 78 ++++++++++++++++++- 16 files changed, 90 insertions(+), 75 deletions(-) diff --git a/server/src/uds/REST/methods/services.py b/server/src/uds/REST/methods/services.py index 2430a4a4f..f45243835 100644 --- a/server/src/uds/REST/methods/services.py +++ b/server/src/uds/REST/methods/services.py @@ -79,10 +79,7 @@ class Services(DetailHandler): # pylint: disable=too-many-public-methods 'cache_tooltip_l2': _(info.cache_tooltip_l2), 'needs_osmanager': info.needs_osmanager, 'allowed_protocols': info.allowed_protocols, - 'services_type_provided': [ - info.services_type_provided - ], # As a list for compatibility, to be removed TODO: Remove - 'must_assign_manually': info.must_assign_manually, + 'services_type_provided': info.services_type_provided, 'can_reset': info.can_reset, 'can_list_assignables': info.can_assign(), } diff --git a/server/src/uds/core/services/generics/dynamic/service.py b/server/src/uds/core/services/generics/dynamic/service.py index b6c978a2f..6f98865f2 100644 --- a/server/src/uds/core/services/generics/dynamic/service.py +++ b/server/src/uds/core/services/generics/dynamic/service.py @@ -55,7 +55,6 @@ class DynamicService(services.Service, abc.ABC): # pylint: disable=too-many-pub uses_cache = False # Cache are running machine awaiting to be assigned uses_cache_l2 = False # L2 Cache are running machines in suspended state needs_osmanager = False # If the service needs a s.o. manager (managers are related to agents provided by services, i.e. virtual machines with agent) - must_assign_manually = False # If true, the system can't do an automatic assignation of a deployed user service from this service # can_reset = True # : Types of publications (preparated data for deploys) diff --git a/server/src/uds/core/services/generics/fixed/service.py b/server/src/uds/core/services/generics/fixed/service.py index be6d610ce..64e01c98e 100644 --- a/server/src/uds/core/services/generics/fixed/service.py +++ b/server/src/uds/core/services/generics/fixed/service.py @@ -58,7 +58,6 @@ class FixedService(services.Service, abc.ABC): # pylint: disable=too-many-publi uses_cache = False # Cache are running machine awaiting to be assigned uses_cache_l2 = False # L2 Cache are running machines in suspended state needs_osmanager = False # If the service needs a s.o. manager (managers are related to agents provided by services, i.e. virtual machines with agent) - must_assign_manually = False # If true, the system can't do an automatic assignation of a deployed user service from this service # can_reset = True # If machine has an alternate field with it, it will be used instead of "machines" field diff --git a/server/src/uds/core/services/provider_factory.py b/server/src/uds/core/services/provider_factory.py index 0ec520266..aae3b666b 100644 --- a/server/src/uds/core/services/provider_factory.py +++ b/server/src/uds/core/services/provider_factory.py @@ -96,12 +96,5 @@ class ServiceProviderFactory(factory.ModuleFactory[ServiceProvider]): s for p in self.providers().values() for s in p.offers - if s.publication_type is None and s.must_assign_manually is False + if s.publication_type is None ] - # old code :-) - # res = [] - # for p in self.providers().values(): - # for s in p.offers: - # if s.publication_type is None and s.must_assign_manually is False: - # res.append(s) - # return res diff --git a/server/src/uds/core/services/service.py b/server/src/uds/core/services/service.py index bd59edd9f..5fbfe5c63 100644 --- a/server/src/uds/core/services/service.py +++ b/server/src/uds/core/services/service.py @@ -160,15 +160,6 @@ class Service(Module): # : If the service needs a o.s. manager (see os managers section) needs_osmanager: bool = False - # : If the service can be autoassigned or needs to be assigned by administrator - # : Not all services are for assigning it. Thing, i.e., a Service that manages - # : a number of Server. The desired behavior will be to let administrator - # : the service to a user in the administration interface, an not the system - # : to assign the service automatically. If this is true, the core will not - # : assign the service automatically, so if the user do not have a consumable - # : assigned, the user will never get one (of this kind, of course) - must_assign_manually: typing.ClassVar[bool] = False - # : Types of publications (preparated data for deploys) # : If you provide this, UDS will assume that the service needs a preparation. # : If not provided (it is None), UDS will assume that service do not needs @@ -307,18 +298,6 @@ class Service(Module): # Keep untouched if services_limit is not present - def user_services_for_assignation(self, **kwargs: typing.Any) -> collections.abc.Iterable['UserService']: - """ - override this if mustAssignManualy is True - @params kwargs: Named arguments - @return an iterable with the services that we can assign manually (they must be of type UserDeployment) - We will access the returned iterable in "name" basis. This means that the service will be assigned by "name", so be care that every single service - name returned is unique :-) - """ - raise Exception( - f'The class {self.__class__.__name__} has been marked as manually asignable but no requestServicesForAssignetion provided!!!' - ) - def mac_generator(self) -> 'UniqueMacGenerator': """ Utility method to access provided macs generator (inside environment) diff --git a/server/src/uds/services/OVirt/service_linked.py b/server/src/uds/services/OVirt/service_linked.py index c1f9d617c..0bd4dabce 100644 --- a/server/src/uds/services/OVirt/service_linked.py +++ b/server/src/uds/services/OVirt/service_linked.py @@ -91,9 +91,6 @@ class OVirtLinkedService(services.Service): # pylint: disable=too-many-public-m # : If the service needs a s.o. manager (managers are related to agents # : provided by services itselfs, i.e. virtual machines with actors) needs_osmanager = True - # : If true, the system can't do an automatic assignation of a deployed user - # : service from this service - must_assign_manually = False can_reset = True # : Types of publications (preparated data for deploys) diff --git a/server/src/uds/services/OpenGnsys/service.py b/server/src/uds/services/OpenGnsys/service.py index 0504d58f2..c709d153f 100644 --- a/server/src/uds/services/OpenGnsys/service.py +++ b/server/src/uds/services/OpenGnsys/service.py @@ -80,9 +80,6 @@ class OGService(services.Service): # : If the service needs a s.o. manager (managers are related to agents # : provided by services itselfs, i.e. virtual machines with actors) needs_osmanager = False - # : If true, the system can't do an automatic assignation of a deployed user - # : service from this service - must_assign_manually = False # : Types of publications (preparated data for deploys) # : In our case, we do no need a publication, so this is None diff --git a/server/src/uds/services/OpenNebula/service.py b/server/src/uds/services/OpenNebula/service.py index 7806ddce2..5ff89be62 100644 --- a/server/src/uds/services/OpenNebula/service.py +++ b/server/src/uds/services/OpenNebula/service.py @@ -86,9 +86,6 @@ class OpenNebulaLiveService(services.Service): # : If the service needs a s.o. manager (managers are related to agents # : provided by services itselfs, i.e. virtual machines with actors) needs_osmanager = True - # : If true, the system can't do an automatic assignation of a deployed user - # : service from this service - must_assign_manually = False can_reset = True # : Types of publications (preparated data for deploys) diff --git a/server/src/uds/services/OpenStack/service.py b/server/src/uds/services/OpenStack/service.py index 5897bb856..b25526fbc 100644 --- a/server/src/uds/services/OpenStack/service.py +++ b/server/src/uds/services/OpenStack/service.py @@ -88,9 +88,6 @@ class OpenStackLiveService(services.Service): # : If the service needs a s.o. manager (managers are related to agents # : provided by services itselfs, i.e. virtual machines with actors) needs_osmanager = True - # : If true, the system can't do an automatic assignation of a deployed user - # : service from this service - must_assign_manually = False can_reset = True # : Types of publications (preparated data for deploys) diff --git a/server/src/uds/services/PhysicalMachines/service_multi.py b/server/src/uds/services/PhysicalMachines/service_multi.py index c875f2a81..f4d0de6be 100644 --- a/server/src/uds/services/PhysicalMachines/service_multi.py +++ b/server/src/uds/services/PhysicalMachines/service_multi.py @@ -132,7 +132,6 @@ class IPMachinesService(services.Service): uses_cache = False # Cache are running machine awaiting to be assigned uses_cache_l2 = False # L2 Cache are running machines in suspended state needs_osmanager = False # If the service needs a s.o. manager (managers are related to agents provided by services itselfs, i.e. virtual machines with agent) - must_assign_manually = False # If true, the system can't do an automatic assignation of a deployed user service from this service user_service_type = IPMachinesUserService @@ -149,7 +148,7 @@ class IPMachinesService(services.Service): def enumerate_assignables(self) -> collections.abc.Iterable[types.ui.ChoiceItem]: now = sql_now() return [ - gui.choice_item(f'{server.host}|{server.mac}', server.uuid) + gui.choice_item(server.uuid, f'{server.host}|{server.mac}') for server in fields.get_server_group_from_field(self.server_group).servers.all() if server.locked_until is None or server.locked_until < now ] @@ -163,13 +162,11 @@ class IPMachinesService(services.Service): server: 'models.Server' = models.Server.objects.get(uuid=assignable_id) ipmachine_instance: IPMachinesUserService = typing.cast(IPMachinesUserService, userservice_instance) if server.locked_until is None or server.locked_until < sql_now(): - # Lock the server for 10 year right now... - server.locked_until = sql_now() + datetime.timedelta(days=365) + self.lock_server(server.uuid) + return ipmachine_instance.assign(server.uuid) - return ipmachine_instance.assign(server.host) + return ipmachine_instance._error('Host already assigned') - return ipmachine_instance._error('IP already assigned') - def get_unassigned(self) -> str: ''' Returns an unassigned machine @@ -177,11 +174,11 @@ class IPMachinesService(services.Service): list_of_servers = list(fields.get_server_group_from_field(self.server_group).servers.all()) if self.randomize_host.as_bool() is True: random.shuffle(list_of_servers) # Reorder the list randomly if required - for server in list_of_servers: - if server.locked_until is None or server.locked_until < sql_now(): - return server.uuid + for server in list_of_servers: + if server.locked_until is None or server.locked_until < sql_now(): + return server.uuid raise exceptions.services.InsufficientResourcesException() - + def get_host_mac(self, server_uuid: str) -> typing.Tuple[str, str]: server = models.Server.objects.get(uuid=server_uuid) return server.host, server.mac @@ -208,7 +205,10 @@ class IPMachinesService(services.Service): # Maybe, an user has logged in on an unassigned machine # if lockForExternalAccess is enabled, we must lock it if self.lock_on_external_access.as_bool() is True: - self.do_log(types.log.LogLevel.DEBUG, f'External login detected for {id}, locking machine for {self.get_max_lock_time()} or until logout') + self.do_log( + types.log.LogLevel.DEBUG, + f'External login detected for {id}, locking machine for {self.get_max_lock_time()} or until logout', + ) self.lock_server(id) def process_logout(self, id: str, remote_login: bool) -> None: @@ -255,4 +255,3 @@ class IPMachinesService(services.Service): # always is available def is_avaliable(self) -> bool: return True - diff --git a/server/src/uds/services/PhysicalMachines/service_single.py b/server/src/uds/services/PhysicalMachines/service_single.py index 682203fbc..23b6a5af9 100644 --- a/server/src/uds/services/PhysicalMachines/service_single.py +++ b/server/src/uds/services/PhysicalMachines/service_single.py @@ -59,7 +59,6 @@ class IPSingleMachineService(services.Service): uses_cache = False # Cache are running machine awaiting to be assigned uses_cache_l2 = False # L2 Cache are running machines in suspended state needs_osmanager = False # If the service needs a s.o. manager (managers are related to agents provided by services itselfs, i.e. virtual machines with agent) - must_assign_manually = False # If true, the system can't do an automatic assignation of a deployed user service from this service user_service_type = IPMachineUserService diff --git a/server/src/uds/services/Proxmox/service_linked.py b/server/src/uds/services/Proxmox/service_linked.py index 4c6520ceb..70c8999c3 100644 --- a/server/src/uds/services/Proxmox/service_linked.py +++ b/server/src/uds/services/Proxmox/service_linked.py @@ -95,9 +95,6 @@ class ProxmoxServiceLinked(DynamicService): # : If the service needs a s.o. manager (managers are related to agents # : provided by services itselfs, i.e. virtual machines with actors) needs_osmanager = True - # : If true, the system can't do an automatic assignation of a deployed user - # : service from this service - must_assign_manually = False can_reset = True # : Types of publications (preparated data for deploys) diff --git a/server/src/uds/services/Sample/service.py b/server/src/uds/services/Sample/service.py index 79e343343..32f3c6da0 100644 --- a/server/src/uds/services/Sample/service.py +++ b/server/src/uds/services/Sample/service.py @@ -104,9 +104,6 @@ class ServiceOne(services.Service): # : If the service needs a s.o. manager (managers are related to agents # : provided by services itselfs, i.e. virtual machines with actors) needs_osmanager = False - # : If true, the system can't do an automatic assignation of a deployed user - # : service from this service - must_assign_manually = False # : Types of publications (preparated data for deploys) # : In our case, we do no need a publication, so this is None @@ -212,7 +209,6 @@ class ServiceTwo(services.Service): cache_tooltip_l2 = _('L2 cache for dummy elements') needs_osmanager = False - must_assign_manually = False # : Types of publications. In this case, we will include a publication # : type for this one diff --git a/server/src/uds/services/Test/service.py b/server/src/uds/services/Test/service.py index 11af79bb1..e1c524f92 100644 --- a/server/src/uds/services/Test/service.py +++ b/server/src/uds/services/Test/service.py @@ -65,7 +65,6 @@ class TestServiceNoCache(services.Service): cache_tooltip_l2 = _('None') needs_osmanager = False - must_assign_manually = False publication_type = None user_service_type = TestUserService @@ -97,7 +96,6 @@ class TestServiceCache(services.Service): cache_tooltip_l2 = _('L2 cache for dummy elements') needs_osmanager = False - must_assign_manually = False # : Types of publications. In this case, we will include a publication # : type for this one diff --git a/server/src/uds/services/Xen/service.py b/server/src/uds/services/Xen/service.py index c37abfd6b..4cee3ec40 100644 --- a/server/src/uds/services/Xen/service.py +++ b/server/src/uds/services/Xen/service.py @@ -86,9 +86,6 @@ class XenLinkedService(services.Service): # pylint: disable=too-many-public-met # : If the service needs a s.o. manager (managers are related to agents # : provided by services itselfs, i.e. virtual machines with actors) needs_osmanager = True - # : If true, the system can't do an automatic assignation of a deployed user - # : service from this service - must_assign_manually = False can_reset = True # : Types of publications (preparated data for deploys) diff --git a/server/tests/services/physical_machines/test_service_multi.py b/server/tests/services/physical_machines/test_service_multi.py index 641f11b66..0c76ccd48 100644 --- a/server/tests/services/physical_machines/test_service_multi.py +++ b/server/tests/services/physical_machines/test_service_multi.py @@ -30,6 +30,7 @@ """ Author: Adolfo Gómez, dkmaster at dkmon dot com """ +import random from unittest import mock from uds import models @@ -51,9 +52,12 @@ class TestServiceMulti(UDSTransactionTestCase): self.assertTrue(server_group.servers.count() > 0) self.assertEqual(service.port.value, fixtures.SERVICE_MULTI_VALUES_DICT['port']) self.assertEqual( - service.ignore_minutes_on_failure.value, fixtures.SERVICE_MULTI_VALUES_DICT['ignore_minutes_on_failure'] + service.ignore_minutes_on_failure.value, + fixtures.SERVICE_MULTI_VALUES_DICT['ignore_minutes_on_failure'], + ) + self.assertEqual( + service.max_session_hours.value, fixtures.SERVICE_MULTI_VALUES_DICT['max_session_hours'] ) - self.assertEqual(service.max_session_hours.value, fixtures.SERVICE_MULTI_VALUES_DICT['max_session_hours']) self.assertEqual( service.lock_on_external_access.value, fixtures.SERVICE_MULTI_VALUES_DICT['lock_on_external_access'] ) @@ -163,3 +167,73 @@ class TestServiceMulti(UDSTransactionTestCase): service.unlock_server(server.uuid) server.refresh_from_db() self.assertIsNone(server.locked_until) + + def test_get_unassigned(self) -> None: + service = fixtures.create_service_multi() + # Without random host + service.randomize_host.value = False + server_list = list(fields.get_server_group_from_field(service.server_group).servers.all()) + + for num, server in enumerate(server_list, 1): + unassigned_uuid = service.get_unassigned() + # Must be the first server + self.assertEqual(unassigned_uuid, server.uuid, f'Error on element {num}') # type: ignore # if first is None,raises error + # Lock it, so it's not returned again + service.lock_server(unassigned_uuid) + + # Now, randomized, we must no receive same servers + service.randomize_host.value = True + # unlock all servers + for server in server_list: + service.unlock_server(server.uuid) + + # Now, execute at most 128 iterations, if we get first server all times, it's a problem + count = 0 + for count in range(128): + unassigned_uuid = service.get_unassigned() + self.assertNotEqual(unassigned_uuid, server_list[0].uuid) + if unassigned_uuid != server_list[0].uuid: + break + + if count == 127: + self.fail('Randomized server selection failed') + + # Now ensure we can lock all servers + for num in range(len(server_list)): + unassigned_uuid = service.get_unassigned() + self.assertIsNotNone(unassigned_uuid) + service.lock_server(unassigned_uuid) + + def test_enumerate_assignables(self) -> None: + service = fixtures.create_service_multi() + server_group = fields.get_server_group_from_field(service.server_group) + server_list = list(server_group.servers.all()) + + # lock 4 random servers, and remove them from the list + locked_servers = random.sample(server_list, 4) + for server in locked_servers: + service.lock_server(server.uuid) + server_list.remove(server) + + assignables = {i['id'] for i in service.enumerate_assignables()} + self.assertEqual(assignables, {i.uuid for i in server_list}) + + def test_assign_from_asignables(self) -> None: + service = fixtures.create_service_multi() + server_group = fields.get_server_group_from_field(service.server_group) + server_list = list(server_group.servers.all()) + + # lock 4 random servers, and remove them from the list + locked_servers = random.sample(server_list, 4) + for server in locked_servers: + service.lock_server(server.uuid) + server_list.remove(server) + + for server in server_list: + # Assign a server + userservice_mock = mock.MagicMock() + service.assign_from_assignables(server.uuid, mock.Mock(), userservice_mock) + userservice_mock.assign.assert_called_once_with(server.uuid) + # Ensure is locked + server.refresh_from_db() + self.assertIsNotNone(server.locked_until) \ No newline at end of file