shaba/openuds
1
0
Fork 1
mirror of https://github.com/dkmstr/openuds.git synced 2025-03-12 04:58:34 +03:00
Code Issues Releases Wiki Activity

Improvements to generics, and fixes related to macs.

Browse Source 
* Now correctly notes that if get_mac is with an empty vmid, should process acordly to the specific service.
* Now, before starting or stopping and vms, the state is checked by the userservice (using is_running)
This commit is contained in:
Adolfo Gómez García 2024-04-12 04:47:15 +02:00
parent fb3b02ac2d
commit c02c9f45a4
No known key found for this signature in database
GPG Key ID: DD1ABF20724CDA23
8 changed files with 53 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
server/src/uds/REST/handlers.py
View File

@ -145,6 +145,7 @@ class Handler:
raise AccessDenied() from e
else:
self._user = User() # Empty user for non authenticated handlers
self._user.state = types.states.State.ACTIVE # Ensure it's active
if self._user and self._user.state != types.states.State.ACTIVE:
raise AccessDenied()

2
server/src/uds/core/services/generics/dynamic/publication.py
View File

@ -373,7 +373,7 @@ class DynamicPublication(services.Publication, autoserializable.AutoSerializable
This method is called when the service is removed
By default, we need a remove machine on the service, use it
"""
self.service().remove(self, self._vmid)
self.service().delete(self, self._vmid)
def op_remove_completed(self) -> None:
"""

5
server/src/uds/core/services/generics/dynamic/service.py
View File

@ -134,6 +134,9 @@ class DynamicService(services.Service, abc.ABC): # pylint: disable=too-many-pub
"""
Returns the mac of the machine
If cannot be obtained, MUST raise an exception
Note:
vmid can be '' if we are requesting a new mac (on some services, where UDS generate the machines MAC)
If the service does not support this, it can raise an exception
"""
...
@ -195,7 +198,7 @@ class DynamicService(services.Service, abc.ABC): # pylint: disable=too-many-pub
return self.shutdown(caller_instance, vmid)
@abc.abstractmethod
def remove(
def delete(
self, caller_instance: 'DynamicUserService | DynamicPublication', vmid: str
) -> None:
"""

15
server/src/uds/core/services/generics/dynamic/userservice.py
View File

@ -220,7 +220,7 @@ class DynamicUserService(services.UserService, autoserializable.AutoSerializable
if self._vmid:
if self.service().should_maintain_on_error() is False:
try:
self.service().remove(self, self._vmid)
self.service().delete(self, self._vmid)
self._vmid = ''
except Exception as e:
logger.exception('Exception removing machine %s: %s', self._vmid, e)
@ -308,6 +308,8 @@ class DynamicUserService(services.UserService, autoserializable.AutoSerializable
def get_unique_id(self) -> str:
# Provide self to the service, so it can some of our methods to generate the unique id
# (for example, own mac generator, that will autorelease the mac as soon as the machine is removed)
# Note that get_mac is used for creating a new mac, returning the one of the vm or whatever
# This is responsibility of the service, not of the user service
if not self._mac:
self._mac = self.service().get_mac(self, self._vmid) or ''
return self._mac
@ -486,7 +488,8 @@ class DynamicUserService(services.UserService, autoserializable.AutoSerializable
"""
This method is called when the service is started
"""
self.service().start(self, self._vmid)
if not self.service().is_running(self, self._vmid):
self.service().start(self, self._vmid)
def op_start_completed(self) -> None:
"""
@ -499,7 +502,8 @@ class DynamicUserService(services.UserService, autoserializable.AutoSerializable
"""
This method is called for stopping the service
"""
self.service().stop(self, self._vmid)
if self.service().is_running(self, self._vmid):
self.service().stop(self, self._vmid)
def op_stop_completed(self) -> None:
"""
@ -513,8 +517,7 @@ class DynamicUserService(services.UserService, autoserializable.AutoSerializable
This method is called for shutdown the service
"""
shutdown_stamp = -1
is_running = self.service().is_running(self, self._vmid)
if not is_running:
if not self.service().is_running(self, self._vmid):
# Already stopped, just finish
return
@ -562,7 +565,7 @@ class DynamicUserService(services.UserService, autoserializable.AutoSerializable
"""
This method is called when the service is removed
"""
self.service().remove(self, self._vmid)
self.service().delete(self, self._vmid)
def op_remove_completed(self) -> None:
"""

2
server/src/uds/services/Proxmox/publication.py
View File

@ -128,7 +128,7 @@ class ProxmoxPublication(DynamicPublication, autoserializable.AutoSerializable):
self.service().provider().create_template(int(self._vmid))
def op_remove(self) -> None:
self.service().remove(self, self._vmid)
self.service().delete(self, self._vmid)
def machine(self) -> int:
return int(self._vmid)

23
server/src/uds/services/Proxmox/service_linked.py
View File

@ -58,7 +58,7 @@ logger = logging.getLogger(__name__)
class ProxmoxServiceLinked(DynamicService):
"""
Proxmox Linked clones service. This is based on creating a template from selected vm, and then use it to
Notes:
* We do not suspend machines, we try to shutdown them gracefully
"""
@ -286,10 +286,13 @@ class ProxmoxServiceLinked(DynamicService):
def get_ip(self, caller_instance: 'DynamicUserService | DynamicPublication', vmid: str) -> str:
return self.provider().get_guest_ip_address(int(vmid))
def get_mac(self, caller_instance: 'DynamicUserService | DynamicPublication', vmid: str) -> str:
# If vmid is empty, we are requesting a new mac
if not vmid:
return self.mac_generator().get(self.get_macs_range())
return self.get_nic_mac(int(vmid))
def start(self, caller_instance: 'DynamicUserService | DynamicPublication', vmid: str) -> None:
if isinstance(caller_instance, ProxmoxUserserviceLinked):
if self.is_running(caller_instance, vmid): # If running, skip
@ -298,7 +301,7 @@ class ProxmoxServiceLinked(DynamicService):
caller_instance._store_task(self.provider().start_machine(int(vmid)))
else:
raise Exception('Invalid caller instance (publication) for start_machine()')
def stop(self, caller_instance: 'DynamicUserService | DynamicPublication', vmid: str) -> None:
if isinstance(caller_instance, ProxmoxUserserviceLinked):
if self.is_running(caller_instance, vmid):
@ -307,10 +310,8 @@ class ProxmoxServiceLinked(DynamicService):
caller_instance._task = ''
else:
raise Exception('Invalid caller instance (publication) for stop_machine()')
def shutdown(
self, caller_instance: 'DynamicUserService | DynamicPublication', vmid: str
) -> None:
def shutdown(self, caller_instance: 'DynamicUserService | DynamicPublication', vmid: str) -> None:
if isinstance(caller_instance, ProxmoxUserserviceLinked):
if self.is_running(caller_instance, vmid):
caller_instance._store_task(self.provider().shutdown_machine(int(vmid)))
@ -319,15 +320,13 @@ class ProxmoxServiceLinked(DynamicService):
else:
raise Exception('Invalid caller instance (publication) for shutdown_machine()')
def is_running(
self, caller_instance: 'DynamicUserService | DynamicPublication', vmid: str
) -> bool:
def is_running(self, caller_instance: 'DynamicUserService | DynamicPublication', vmid: str) -> bool:
# Raise an exception if fails to get machine info
vminfo = self.get_machine_info(int(vmid))
return vminfo.status != 'stopped'
def remove(self, caller_instance: 'DynamicUserService | DynamicPublication', vmid: str) -> None:
def delete(self, caller_instance: 'DynamicUserService | DynamicPublication', vmid: str) -> None:
# All removals are deferred, so we can do it async
# Try to stop it if already running... Hard stop
jobs.ProxmoxDeferredRemoval.remove(self.provider(), int(vmid), self.try_graceful_shutdown())

10
server/tests/core/services/generics/fixtures.py
View File

@ -459,9 +459,9 @@ class DynamicTestingUserService(dynamic_userservice.DynamicUserService):
class DynamicTestingService(dynamic_service.DynamicService):
type_name = 'Dynamic Service'
type_type = 'DynamicService'
type_description = 'Dynamic Service description'
type_name = 'Dynamic Service Testing'
type_type = 'DynamicServiceTesting'
type_description = 'Dynamic Service Testing description'
user_service_type = DynamicTestingUserServiceQueue
@ -471,7 +471,7 @@ class DynamicTestingService(dynamic_service.DynamicService):
maintain_on_error = dynamic_service.DynamicService.maintain_on_error
try_soft_shutdown = dynamic_service.DynamicService.try_soft_shutdown
machine_running_flag: bool = False
machine_running_flag: bool = True
def get_ip(
self,
@ -521,7 +521,7 @@ class DynamicTestingService(dynamic_service.DynamicService):
self.mock.shutdown(caller_instance, vmid)
self.machine_running_flag = False
def remove(
def delete(
self,
caller_instance: dynamic_userservice.DynamicUserService | dynamic_publication.DynamicPublication,
vmid: str,

28
server/tests/core/services/generics/test_dynamic_service.py
View File

@ -96,6 +96,7 @@ class DynamicServiceTest(UDSTestCase):
def test_userservice_queue_works_fine(self) -> None:
service = fixtures.create_dynamic_service()
service.machine_running_flag = False
userservice = fixtures.create_dynamic_userservice_queue(service)
userservice._vmid = 'vmid'
self.check_iterations(service, userservice, EXPECTED_DEPLOY_ITERATIONS_INFO)
@ -112,10 +113,11 @@ class DynamicServiceTest(UDSTestCase):
self.assertEqual(state, types.states.TaskState.FINISHED)
service.mock.start.assert_called_once_with(userservice, userservice._vmid)
service.mock.is_running.assert_called_once_with(userservice, userservice._vmid)
self.assertEqual(service.mock.is_running.call_count, 2)
def test_userservice_deploy_for_cache_l1(self) -> None:
service = fixtures.create_dynamic_service()
service.machine_running_flag = False
userservice = fixtures.create_dynamic_userservice(service)
state = userservice.deploy_for_cache(types.services.CacheLevel.L1)
@ -126,10 +128,11 @@ class DynamicServiceTest(UDSTestCase):
self.assertEqual(state, types.states.TaskState.FINISHED)
service.mock.start.assert_called_once_with(userservice, userservice._vmid)
service.mock.is_running.assert_called_once_with(userservice, userservice._vmid)
self.assertEqual(service.mock.is_running.call_count, 2)
def test_userservice_deploy_for_cache_l2(self) -> None:
service = fixtures.create_dynamic_service()
service.machine_running_flag = False
userservice = fixtures.create_dynamic_userservice(service)
state = userservice.deploy_for_cache(types.services.CacheLevel.L2)
@ -145,10 +148,11 @@ class DynamicServiceTest(UDSTestCase):
self.assertEqual(state, types.states.TaskState.FINISHED)
service.mock.start.assert_called_once_with(userservice, userservice._vmid)
# is_running must has been called 3 times:
# * Before starting, to ensure it's not running
# * for start check
# * for suspend check before suspend (to ensure it's running)
# * for check_suspend after suspend
self.assertEqual(service.mock.is_running.call_count, 3)
self.assertEqual(service.mock.is_running.call_count, 4)
service.mock.is_running.assert_called_with(userservice, userservice._vmid)
# And shutdown must be called once
service.mock.shutdown.assert_called_once_with(userservice, userservice._vmid)
@ -179,7 +183,9 @@ class DynamicServiceTest(UDSTestCase):
self.assertEqual(state, types.states.TaskState.FINISHED)
service.mock.stop.assert_called_once_with(userservice, userservice._vmid)
service.mock.is_running.assert_called_once_with(userservice, userservice._vmid)
service.mock.is_running.assert_called_with(userservice, userservice._vmid)
self.assertEqual(service.mock.is_running.call_count, 2)
service.mock.remove.assert_called_once_with(userservice, userservice._vmid)
def test_userservice_maintain_on_error_no_created(self) -> None:
@ -276,7 +282,9 @@ class DynamicServiceTest(UDSTestCase):
self.assertEqual(userservice.check_state(), types.states.TaskState.ERROR)
self.assertEqual(userservice.error_reason(), 'Max retries reached')
self.assertEqual(counter, 11) # 4 retries + 5 retries after reset + 1 of the reset itself + 1 of initial NOP
self.assertEqual(
counter, 11
) # 4 retries + 5 retries after reset + 1 of the reset itself + 1 of initial NOP
def test_userservice_max_retries_checker(self) -> None:
service = fixtures.create_dynamic_service()
@ -321,7 +329,10 @@ EXPECTED_DEPLOY_ITERATIONS_INFO: typing.Final[list[DynamicServiceIterationInfo]]
DynamicServiceIterationInfo( # 4, START
queue=fixtures.ALL_TESTEABLE_OPERATIONS[3:],
user_service_calls=[call.create_completed_checker()],
service_calls=[call.start(MustBeOfType(fixtures.DynamicTestingUserServiceQueue), MustBeOfType(str))],
service_calls=[
call.is_running(MustBeOfType(fixtures.DynamicTestingUserServiceQueue), MustBeOfType(str)),
call.start(MustBeOfType(fixtures.DynamicTestingUserServiceQueue), MustBeOfType(str)),
],
),
DynamicServiceIterationInfo( # 5, START_COMPLETED
queue=fixtures.ALL_TESTEABLE_OPERATIONS[4:],
@ -333,7 +344,10 @@ EXPECTED_DEPLOY_ITERATIONS_INFO: typing.Final[list[DynamicServiceIterationInfo]]
DynamicServiceIterationInfo( # 6, STOP
queue=fixtures.ALL_TESTEABLE_OPERATIONS[5:],
user_service_calls=[call.start_completed_checker()],
service_calls=[call.stop(MustBeOfType(fixtures.DynamicTestingUserServiceQueue), MustBeOfType(str))],
service_calls=[
call.is_running(MustBeOfType(fixtures.DynamicTestingUserServiceQueue), MustBeOfType(str)),
call.stop(MustBeOfType(fixtures.DynamicTestingUserServiceQueue), MustBeOfType(str)),
],
),
DynamicServiceIterationInfo( # 7, STOP_COMPLETED
queue=fixtures.ALL_TESTEABLE_OPERATIONS[6:],