slapos_cloud: check more before destroy instance without parent

master/bt5/slapos_cloud/SkinTemplateItem/portal_skins/slapos_cloud/Alarm_garbageCollectDestroyUnlinkedInstance.py

@@ -5,7 +5,7 @@ portal.portal_catalog.searchAndActivate(
   portal_type=["Software Instance", "Slave Instance"],
   validation_state="validated",
   specialise_validation_state="validated",
-  predecessor_related_title=SimpleQuery(predecessor_related_title=None, comparison_operator='is'),
+  predecessor_related_uid=SimpleQuery(predecessor_related_uid=None, comparison_operator='is'),
   method_id='SoftwareInstance_tryToGarbageUnlinkedInstance',
   activate_kw={'tag': tag}
 )

master/bt5/slapos_cloud/SkinTemplateItem/portal_skins/slapos_cloud/SoftwareInstance_tryToGarbageUnlinkedInstance.py

@@ -6,6 +6,20 @@ if REQUEST is not None:
 
 instance = context
 
+def checkInstanceTree(instance_list):
+  """
+  Check if predecessor link is really removed to this instance
+  """
+  sub_instance_list = []
+  if instance_list == []:
+    return
+  for item in instance_list:
+    if item.getUid() == instance.getUid():
+      return item
+    sub_instance_list.extend(item.getPredecessorValueList())
+
+  return checkInstanceTree(sub_instance_list)
+
 if instance.getSlapState() == "destroy_requested":
   return
 
@@ -14,18 +28,18 @@ if hosting_subscription is None or \
     hosting_subscription.getSlapState() == "destroy_requested":
   return
 
+root_instance = hosting_subscription.getPredecessorValue()
+if root_instance is None:
+  # Refuse to destroy root instance
+  raise ValueError("Hosting Subscription %s has no root instance, this should "\
+                   "not happen!!" % hosting_subscription.getRelativeUrl())
+
 # If instance modificationDate is too recent, skip
 # Delay destroy of unlinked instances
-if not delay_time:
-  delay_time = 50
 if instance.getModificationDate() - addToDate(DateTime(), {'minute': -1*delay_time}) > 0:
   return
 
-if hosting_subscription.getTitle() == instance.getTitle():
-  # Refuse to destroy root instance
-  return
-
-if instance.getPredecessorRelatedValue() is None:
+if checkInstanceTree([root_instance]) is None:
   # This unlinked instance to parent should be removed
   is_slave = False
   if instance.getPortalType() == 'Slave Instance':

master/bt5/slapos_cloud/TestTemplateItem/portal_components/test.erp5.testSlapOSCloudAlarm.py

@@ -2191,10 +2191,29 @@ portal_workflow.doActionFor(context, action='edit_action', comment='Visited by S
     self.tic()
     self.assertEqual(instance.getPredecessorRelatedTitle(), None)
     # will not destroy
-    instance.SoftwareInstance_tryToGarbageUnlinkedInstance(delay_time=-10)
+    self.assertRaises(
+      ValueError,
+      instance.SoftwareInstance_tryToGarbageUnlinkedInstance,
+      delay_time=-10)
     self.tic()
     self.assertEqual(instance.getSlapState(), 'start_requested')
 
+  def test_SoftwareInstance_tryToGarbageUnlinkedInstance_not_unlinked(self):
+    instance = self.createInstance()
+    partition = self.createComputerPartition()
+    instance.edit(aggregate_value=partition)
+    self.tic()
+    instance0 = self.doRequestInstance(instance, 'instance0')
+    instance_instance0 = self.doRequestInstance(instance0, 'Subinstance0')
+    self.assertEqual(instance_instance0.getPredecessorRelatedTitle(),
+                     'instance0')
+    self.assertEqual(instance_instance0.getSlapState(), 'start_requested')
+
+    # Try to remove without delete predecessor link
+    instance_instance0.SoftwareInstance_tryToGarbageUnlinkedInstance(delay_time=-1)
+    self.tic()
+    self.assertEqual(instance_instance0.getSlapState(), 'start_requested')
+
   def test_alarm_search_inlinked_instance(self):
     instance = self.createInstance()
     partition = self.createComputerPartition()