slapproxy: propagate parent partition state to children

slapos/proxy/views.py

@@ -547,6 +547,19 @@ def getAllocatedSlaveInstance(slave_reference, requested_computer_id):
   # XXX: check there is only one result
   return execute_db(table, q, args, one=True)
 
+def getRootPartition(reference):
+  p = 'SELECT * FROM %s WHERE reference=?'
+  parent_partition = execute_db('partition', p, [reference], one=True)
+
+  while parent_partition is not None:
+    parent_reference = parent_partition['requested_by']
+    if not parent_reference or parent_reference == reference:
+      break
+    reference = parent_reference
+    parent_partition = execute_db('partition', p, [reference], one=True)
+
+  return parent_partition
+
 def requestNotSlave(software_release, software_type, partition_reference, partition_id, partition_parameter_kw, filter_kw, requested_state):
   instance_xml = dict2xml(partition_parameter_kw)
   requested_computer_id = filter_kw['computer_guid']
@@ -563,10 +576,6 @@ def requestNotSlave(software_release, software_type, partition_reference, partit
   a = args.append
   q = 'UPDATE %s SET slap_state="busy"'
 
-  if requested_state:
-    q += ', requested_state=?'
-    a(requested_state)
-
   if partition is None:
     partition = execute_db('partition',
         'SELECT * FROM %s WHERE slap_state="free" and computer_reference=?',
@@ -589,6 +598,16 @@ def requestNotSlave(software_release, software_type, partition_reference, partit
     if partition['software_release'].encode() != software_release:
       q += ' ,software_release=?'
       a(software_release)
+    if partition['requested_by']:
+      root_partition = getRootPartition(partition['requested_by'])
+      if root_partition and root_partition['requested_state'] != "started":
+        # propagate parent state to child
+        # child can be stopped or destroyed while parent is started
+        requested_state = root_partition['requested_state']
+
+  if requested_state:
+    q += ', requested_state=?'
+    a(requested_state)
 
   #
   # XXX change software_type when requested

slapos/tests/slapproxy/__init__.py

@@ -285,7 +285,7 @@ class MasterMixin(BasicMixin, unittest.TestCase):
   Define advanced tool for test proxy simulating behavior slap library tools
   """
   def _requestComputerPartition(self, software_release, software_type, partition_reference,
-              partition_id,
+              partition_id=None,
               shared=False, partition_parameter_kw=None, filter_kw=None,
               state=None):
     """
@@ -406,6 +406,48 @@ class TestRequest(MasterMixin):
         self.request('http://sr//', None, 'MyFirstInstance', 'slappart2').__dict__,
         self.request('http://sr//', None, 'MyFirstInstance', 'slappart2').__dict__)
 
+  def test_request_propagate_partition_state(self):
+    """
+    Request will return same partition for two equal requests
+    """
+    self.add_free_partition(2)
+    partition_parent = self.request('http://sr//', None, 'MyFirstInstance')
+    parent_dict = partition_parent.__dict__
+    partition_child = self.request('http://sr//', None, 'MySubInstance', parent_dict['_partition_id'])
+    
+    self.assertEqual(partition_parent.getState(), 'started')
+    self.assertEqual(partition_child.getState(), 'started')
+
+    partition_parent = self.request('http://sr//', None, 'MyFirstInstance', state='stopped')
+    partition_child = self.request('http://sr//', None, 'MySubInstance', parent_dict['_partition_id'])
+    
+    self.assertEqual(partition_parent.getState(), 'stopped')
+    self.assertEqual(partition_child.getState(), 'stopped')
+
+    partition_parent = self.request('http://sr//', None, 'MyFirstInstance', state='started')
+    partition_child = self.request('http://sr//', None, 'MySubInstance', parent_dict['_partition_id'])
+    
+    self.assertEqual(partition_parent.getState(), 'started')
+    self.assertEqual(partition_child.getState(), 'started')
+
+  def test_request_parent_started_children_stopped(self):
+    """
+    Request will return same partition for two equal requests
+    """
+    self.add_free_partition(2)
+    partition_parent = self.request('http://sr//', None, 'MyFirstInstance')
+    parent_dict = partition_parent.__dict__
+    partition_child = self.request('http://sr//', None, 'MySubInstance', parent_dict['_partition_id'])
+    
+    self.assertEqual(partition_parent.getState(), 'started')
+    self.assertEqual(partition_child.getState(), 'started')
+
+    partition_parent = self.request('http://sr//', None, 'MyFirstInstance')
+    partition_child = self.request('http://sr//', None, 'MySubInstance', parent_dict['_partition_id'], state='stopped')
+    
+    self.assertEqual(partition_parent.getState(), 'started')
+    self.assertEqual(partition_child.getState(), 'stopped')
+
   def test_two_requests_with_different_parameters_but_same_reference(self):
     """
     Request will return same partition for two different requests but will

[Thomas Gambier]

@alain.takoudjou why did you do this commit ? It seems to me that slapproxy should mimic what real slapos master is doing and in this case, real slapos master doesn't enforce the children partition state to be the same as parent partition...

/cc @jerome @rafael @luke @jm

[Alain Takoudjou]

I don't understand what you mean, but the parent state is propagated as default to children. On slapos master, if you start a root instance, children are requested to be started (if requested_state is not explicitely set). If you stop root children will be stopped.

in test test_request_parent_started_children_stopped, children has a requested_state which is not modified by the parend.

[Thomas Gambier]

If you stop root children will be stopped.

no, in real slapos master, if you stop the root instance and this instance doesn't explicitely request that its children are stopped, then the children will stay started.

This is the problem I'm trying to address in slapos!760 (closed)

[Alain Takoudjou]

no, in real slapos master, if you stop the root instance and this instance doesn't explicitely request that its children are stopped, then the children will stay started.

do you mean that when a webrunner is stopped, runner0, runner1 and PBS are still start_requested ?

[Thomas Gambier]

do you mean that when a webrunner is stopped, runner0, runner1 and PBS are still start_requested ?

yes since webrunner switched to switch-softwaretype recipe (slapos@8725ab1a)

before this commit, the root instance correctly requested the children partition in the same state as itself.

[Thomas Gambier]

It seems you did this commit because ERP5 SR was always using switch-softwaretype recipe so it didn't have the feature and you implemented it inside slapproxy instead of implementing it inside ERP5 SR.

I just want to know if I can safely revert this commit and in the same time, make sure that all SR correctly request the children partition in the same state as the master partition.

[Alain Takoudjou]

not sure it was for ERP5 (it was long time ago), maybe it was to solve runner1. But you can revert as slapos@8725ab1a will be used.

[Julien Muchembled]

It seems to me that slapproxy should mimic what real slapos master is doing and in this case, real slapos master doesn't enforce the children partition state to be the same as parent partition...

Yes, there's at least something clear: full slapos master and slapproxy must behave the same, else we're sure to never fix the whole problem correctly.

[Rafael Monnerat]

In short, slapos master has an alarm for stop all instances from a Stopped hosting subscription:

https://lab.nexedi.com/nexedi/slapos.core/blob/master/master/bt5/slapos_cloud/SkinTemplateItem/portal_skins/slapos_cloud/Alarm_stopCollectInstance.py

And another to destroy:

• https://lab.nexedi.com/nexedi/slapos.core/blob/master/master/bt5/slapos_cloud/SkinTemplateItem/portal_skins/slapos_cloud/Alarm_garbageCollectDestroyedRootTree.py

In general, the current behaviour is like this (from slapos master point of view):

• If an HS is destroyed, buildout don't process (so state arent propagated by the children by the instance)
• If an HS is stopped, buildout process but the alarm act as fail safe (to ensure all instance from the cluster are stoped in case some leftover stay behind)
• If an HS is started, ONLY the root instance is started, and propagation are exclusively done by the instances
• If an instance isn't requested anymore by nobody, it is excluded the tree and it is marked to be removed regardless of the state (currently for safety this alarm is disabled):
  • https://lab.nexedi.com/nexedi/slapos.core/blob/master/master/bt5/slapos_cloud/SkinTemplateItem/portal_skins/slapos_cloud/Alarm_garbageCollectDestroyUnlinkedInstance.py

Notice, if is acceptable that an started instance request something in stopped state.

Peharps this should be moved to a discussion in a forum.

[Alain Takoudjou]

https://lab.nexedi.com/nexedi/slapos.core/blob/master/master/bt5/slapos_cloud/SkinTemplateItem/portal_skins/slapos_cloud/Alarm_stopCollectInstance.py

I did not remember about this alarm, so the commit is trying to reproduce what the alarm does.

If an instance isn't requested anymore by nobody, it is excluded the tree and it is marked to be removed regardless of the state (currently for safety this alarm is disabled):

• https://lab.nexedi.com/nexedi/slapos.core/blob/master/master/bt5/slapos_cloud/SkinTemplateItem/portal_skins/slapos_cloud/Alarm_garbageCollectDestroyUnlinkedInstance.py

The Alarm is enabled for IMT to cleanup unlinked VM.