Fix Upgrade decision generation

- fix upgrade decision duplicated when an hosting subscription is deployed in many partitions
- Prevent concurrent transaction to create 2 upgrade decision for the same Hosting Subscription

/reviewed-on nexedi/slapos.core!79

master/bt5/slapos_pdm/PathTemplateItem/portal_alarms/slapos_pdm_hosting_subscription_create_upgrade_decision.xml

@@ -8,7 +8,11 @@
       <dictionary>
         <item>
             <key> <string>active_sense_method_id</string> </key>
-            <value> <string>Alarm_computerCheckUpgradeHostingSubscription</string> </value>
+            <value> <string>Alarm_checkAndUpgradeHostingSubscription</string> </value>
+        </item>
+        <item>
+            <key> <string>automatic_solve</string> </key>
+            <value> <int>0</int> </value>
         </item>
         <item>
             <key> <string>description</string> </key>

master/bt5/slapos_pdm/SkinTemplateItem/portal_skins/slapos_pdm/Alarm_checkAndUpgradeHostingSubscription.py

+portal = context.getPortalObject()
+portal.portal_catalog.searchAndActivate(
+  portal_type='Hosting Subscription',
+  validation_state = 'validated',
+  slap_state=['start_requested', 'stop_requested'],
+  method_id = 'HostingSubscription_createUpgradeDecision',
+  packet_size=1,
+  activate_kw = {'tag':tag}
+)
+
+context.activate(after_tag=tag).getId()

master/bt5/slapos_pdm/SkinTemplateItem/portal_skins/slapos_pdm/Alarm_computerCheckUpgradeHostingSubscription.xml → master/bt5/slapos_pdm/SkinTemplateItem/portal_skins/slapos_pdm/Alarm_checkAndUpgradeHostingSubscription.xml

@@ -54,7 +54,7 @@
         </item>
         <item>
             <key> <string>id</string> </key>
-            <value> <string>Alarm_computerCheckUpgradeHostingSubscription</string> </value>
+            <value> <string>Alarm_checkAndUpgradeHostingSubscription</string> </value>
         </item>
       </dictionary>
     </pickle>

master/bt5/slapos_pdm/SkinTemplateItem/portal_skins/slapos_pdm/Alarm_computerCheckUpgradeHostingSubscription.py

-portal = context.getPortalObject()
-default_allocation_scope_uid = [category.getUid() \
-         for category in portal.portal_categories.allocation_scope.open.objectValues()]
-
-if default_allocation_scope_uid:
-  portal.portal_catalog.searchAndActivate(
-    portal_type='Computer',
-    validation_state = 'validated',
-    default_allocation_scope_uid=default_allocation_scope_uid,
-    method_id = 'Computer_createHostingSubscriptionUpgradeDecision',
-    packet_size=1,
-    activate_kw = {'tag':tag}
-  )
-
-  context.activate(after_tag=tag).getId()

master/bt5/slapos_pdm/SkinTemplateItem/portal_skins/slapos_pdm/Computer_createHostingSubscriptionUpgradeDecision.py

-from DateTime import DateTime
-portal = context.getPortalObject()
-
-partition_list = portal.portal_catalog(portal_type='Computer Partition',
-                                        free_for_request=0,
-                                        parent_uid=context.getUid())
-valid_slap_state = ['start_requested', 'stop_requested']
-
-hosting_subscription_list = []
-upgrade_decision_list = []
-for partition in partition_list:
-  software_instance = partition.getAggregateRelatedValue(
-                            portal_type='Software Instance')
-  if not software_instance:
-    continue
-
-  hosting_subscription = software_instance.getSpecialiseValue(
-                portal_type='Hosting Subscription')
-  if hosting_subscription and hosting_subscription.getSlapState() \
-      in valid_slap_state and not \
-      hosting_subscription in hosting_subscription_list:
-    hosting_subscription_list.append(hosting_subscription)
-  else:
-    continue
-  newer_release = hosting_subscription.\
-                    HostingSubscription_getUpgradableSoftwareRelease()
-  if newer_release is None:
-    continue
-
-  decision_in_progress = newer_release.\
-      SoftwareRelease_getUpgradeDecisionInProgress(hosting_subscription.getUid())
-  
-  if decision_in_progress and \
-      not decision_in_progress.UpgradeDecision_tryToCancel(
-        newer_release.getUrlString()):
-    continue
-  
-  upgrade_decision = newer_release.SoftwareRelease_createUpgradeDecision(
-    source_url=hosting_subscription.getRelativeUrl(),
-    title='A new upgrade is available for %s' % hosting_subscription.getTitle()
-  )
-  upgrade_decision.plan()
-  upgrade_decision.setStartDate(DateTime())
-  upgrade_decision_list.append(upgrade_decision)
-
-return upgrade_decision_list

master/bt5/slapos_pdm/SkinTemplateItem/portal_skins/slapos_pdm/HostingSubscription_createUpgradeDecision.py

+from DateTime import DateTime
+portal = context.getPortalObject()
+
+hosting_subscription = context
+tag = "%s_requestUpgradeDecisionCreation_inProgress" % hosting_subscription.getUid()
+activate_kw = {'tag': tag}
+if portal.portal_activities.countMessageWithTag(tag) > 0:
+  # nothing to do
+  return
+
+root_instance = hosting_subscription.getPredecessorValue()
+if root_instance is None or root_instance.getPortalType() == "Slave Instance":
+  return
+if hosting_subscription.getSlapState() == "destroy_requested":
+  return
+partition = root_instance.getAggregateValue(portal_type="Computer Partition")
+if partition is None:
+  return
+
+if not partition.getParent().getAllocationScopeUid() in [category.getUid() \
+    for category in portal.portal_categories.allocation_scope.open.objectValues()]:
+  return
+
+decision_title = 'A new upgrade is available for %s' % hosting_subscription.getTitle()
+newer_release = hosting_subscription.\
+                  HostingSubscription_getUpgradableSoftwareRelease()
+if newer_release is None:
+  return
+
+decision_in_progress = newer_release.\
+    SoftwareRelease_getUpgradeDecisionInProgress(hosting_subscription.getUid())
+
+if decision_in_progress and \
+    not decision_in_progress.UpgradeDecision_tryToCancel(
+      newer_release.getUrlString()):
+  return
+
+upgrade_decision = newer_release.SoftwareRelease_createUpgradeDecision(
+  source_url=hosting_subscription.getRelativeUrl(),
+  title=decision_title
+)
+with upgrade_decision.defaultActivateParameterDict(activate_kw):
+  upgrade_decision.plan()
+  upgrade_decision.setStartDate(DateTime())
+
+# Prevent concurrent transaction to create 2 upgrade decision for the same hosting_subscription
+hosting_subscription.serialize()
+
+return upgrade_decision

master/bt5/slapos_pdm/SkinTemplateItem/portal_skins/slapos_pdm/Computer_createHostingSubscriptionUpgradeDecision.xml → master/bt5/slapos_pdm/SkinTemplateItem/portal_skins/slapos_pdm/HostingSubscription_createUpgradeDecision.xml

@@ -54,7 +54,7 @@
         </item>
         <item>
             <key> <string>id</string> </key>
-            <value> <string>Computer_createHostingSubscriptionUpgradeDecision</string> </value>
+            <value> <string>HostingSubscription_createUpgradeDecision</string> </value>
         </item>
       </dictionary>
     </pickle>

master/bt5/slapos_pdm/TestTemplateItem/portal_components/test.erp5.testSlapOSPDMAlarm.py

@@ -33,6 +33,19 @@ class TestSlapOSUpgradeDecisionProcess(SlapOSTestCaseMixin):
 
     return computer
 
+  def _makeHostingSubscription(self, slap_state="start_requested"):
+    hosting_subscription = self.portal\
+      .hosting_subscription_module.template_hosting_subscription\
+      .Base_createCloneDocument(batch_mode=1)
+    hosting_subscription.validate()
+    hosting_subscription.edit(
+        title= "Test hosting sub start %s" % self.new_id,
+        reference="TESTHSS-%s" % self.new_id,
+    )
+    self.portal.portal_workflow._jumpToStateFor(hosting_subscription, slap_state)
+
+    return hosting_subscription
+
   def _simulateScript(self, script_name, fake_return='True'):
     if script_name in self.portal.portal_skins.custom.objectIds():
       raise ValueError('Precondition failed: %s exists in custom' % script_name)
@@ -126,30 +139,44 @@ return %s
       computer3.workflow_history['edit_workflow'][-1]['comment'])
   
   def test_alarm_hosting_subscription_create_upgrade_decision(self):
-    computer = self._makeComputer(self.new_id)
-    computer.edit(allocation_scope = 'open/public')
-    computer2 = self._makeComputer(self.generateNewId())
-    computer2.edit(allocation_scope = 'open/personal')
-    computer3 = self._makeComputer(self.generateNewId())
-    computer3.edit(allocation_scope = 'open/friend')
+    hosting_subscription = self._makeHostingSubscription()
+    hosting_subscription2 = self._makeHostingSubscription()
+    hosting_subscription3 = self._makeHostingSubscription()
     
-    self._simulateScript('Computer_createHostingSubscriptionUpgradeDecision')
+    self._simulateScript('HostingSubscription_createUpgradeDecision')
     try:
       self.portal.portal_alarms.slapos_pdm_hosting_subscription_create_upgrade_decision.\
         activeSense()
       self.tic()
     finally:
-      self._dropScript('Computer_createHostingSubscriptionUpgradeDecision')
+      self._dropScript('HostingSubscription_createUpgradeDecision')
 
-    self.assertEqual('Visited by Computer_createHostingSubscriptionUpgradeDecision',
-      computer.workflow_history['edit_workflow'][-1]['comment'])
+    self.assertEqual('Visited by HostingSubscription_createUpgradeDecision',
+      hosting_subscription.workflow_history['edit_workflow'][-1]['comment'])
     
-    self.assertEqual('Visited by Computer_createHostingSubscriptionUpgradeDecision',
-      computer2.workflow_history['edit_workflow'][-1]['comment'])
+    self.assertEqual('Visited by HostingSubscription_createUpgradeDecision',
+      hosting_subscription2.workflow_history['edit_workflow'][-1]['comment'])
 
-    self.assertEqual('Visited by Computer_createHostingSubscriptionUpgradeDecision',
-      computer3.workflow_history['edit_workflow'][-1]['comment'])
+    self.assertEqual('Visited by HostingSubscription_createUpgradeDecision',
+      hosting_subscription3.workflow_history['edit_workflow'][-1]['comment'])
+
+  def test_alarm_create_upgrade_decision_destroyed_hosting_subscription(self):
+    hosting_subscription = self._makeHostingSubscription(slap_state="destroy_requested")
+    hosting_subscription2 = self._makeHostingSubscription(slap_state="destroy_requested")
+    
+    self._simulateScript('HostingSubscription_createUpgradeDecision')
+    try:
+      self.portal.portal_alarms.slapos_pdm_hosting_subscription_create_upgrade_decision.\
+        activeSense()
+      self.tic()
+    finally:
+      self._dropScript('HostingSubscription_createUpgradeDecision')
+
+    self.assertNotEqual('Visited by HostingSubscription_createUpgradeDecision',
+      hosting_subscription.workflow_history['edit_workflow'][-1]['comment'])
     
+    self.assertNotEqual('Visited by HostingSubscription_createUpgradeDecision',
+      hosting_subscription2.workflow_history['edit_workflow'][-1]['comment'])
 
   def test_alarm_create_upgrade_decision_closed_computer(self):
     computer = self._makeComputer(self.new_id)

master/bt5/slapos_pdm/TestTemplateItem/portal_components/test.erp5.testSlapOSPDMSkins.py

@@ -26,6 +26,7 @@
 #
 ##############################################################################
 
+import transaction
 from erp5.component.test.SlapOSTestCaseMixin import SlapOSTestCaseMixin, simulate
 from DateTime import DateTime
 
@@ -55,7 +56,7 @@ class TestSlapOSPDMSkins(SlapOSTestCaseMixin):
     person_user = self.makePerson(new_id=self.new_id)
     return person_user
 
-  def _makeComputer(self):
+  def _makeComputer(self, allocation_scope='open/public'):
     # Clone computer document
     computer = self.portal.computer_module\
       .template_computer.Base_createCloneDocument(batch_mode=1)
@@ -64,6 +65,7 @@ class TestSlapOSPDMSkins(SlapOSTestCaseMixin):
       reference="TESTCOMPS-%s" % (self.new_id, )
     )
     computer.validate()
+    computer.edit(allocation_scope = allocation_scope)
 
     return computer
 
@@ -1074,7 +1076,7 @@ class TestSlapOSPDMSkins(SlapOSTestCaseMixin):
     self.assertEqual(upgrade_decision.getSimulationState(), 'started')
 
   
-  def testComputer_createHostingSubscriptionUpgradeDecision_no_newer(self):
+  def testHostingSubscription_createUpgradeDecision_no_newer(self):
     person = self._makePerson()
     computer = self._makeComputer()
     computer.edit(source_administration_value=person)
@@ -1086,16 +1088,13 @@ class TestSlapOSPDMSkins(SlapOSTestCaseMixin):
     self._makeSoftwareInstallation(computer, url_string)
     self.tic()
 
-    upgrade_decision = computer.Computer_createHostingSubscriptionUpgradeDecision()
-    self.assertEqual(len(upgrade_decision), 0)
-    
     # Create Hosting Subscription
     hosting_subscription = self._makeFullHostingSubscription(
                                     url_string, person)
     self.tic()
     
-    upgrade_decision = computer.Computer_createHostingSubscriptionUpgradeDecision()
-    self.assertEqual(len(upgrade_decision), 0)
+    upgrade_decision = hosting_subscription.HostingSubscription_createUpgradeDecision()
+    self.assertEqual(upgrade_decision, None)
     
     self._makeFullSoftwareInstance(hosting_subscription, url_string)
     self._markComputerPartitionBusy(computer,
@@ -1104,10 +1103,74 @@ class TestSlapOSPDMSkins(SlapOSTestCaseMixin):
     self._requestSoftwareRelease(software_product.getRelativeUrl())
     self.tic()
     
-    upgrade_decision = computer.Computer_createHostingSubscriptionUpgradeDecision()
-    self.assertEqual(len(upgrade_decision), 0)
+    upgrade_decision = hosting_subscription.HostingSubscription_createUpgradeDecision()
+    self.assertEqual(upgrade_decision, None)
+
+  def testHostingSubscription_createUpgradeDecision_closed_computer(self):
+    person = self._makePerson()
+    computer = self._makeComputer(allocation_scope="close/outdated")
+    computer.edit(source_administration_value=person)
+    self._makeComputerPartitions(computer)
+    software_product = self._makeSoftwareProduct()
+    software_release = self._requestSoftwareRelease(
+                                    software_product.getRelativeUrl())
+    url_string = software_release.getUrlString()
+    
+    self._makeSoftwareInstallation( computer, url_string)
+    
+    # Create Hosting Subscription and Software Instance
+    hosting_subscription = self._makeFullHostingSubscription(
+                                    url_string, person)
+    self._makeFullSoftwareInstance(hosting_subscription, url_string)
+    self._markComputerPartitionBusy(computer,
+                                    hosting_subscription.getPredecessorValue())
+    
+    # Install the Newest software release
+    software_release2 = self._requestSoftwareRelease(
+                                      software_product.getRelativeUrl())
+    self._makeSoftwareInstallation(computer,
+                                    software_release2.getUrlString())
+    self.tic()
+    
+    up_decision = hosting_subscription.HostingSubscription_createUpgradeDecision()
+    self.assertEqual(up_decision, None)
 
-  def testComputer_createHostingSubscriptionUpgradeDecision(self):
+  def testHostingSubscription_createUpgradeDecision_create_once_transaction(self):
+    person = self._makePerson()
+    computer = self._makeComputer(allocation_scope="open/personal")
+    computer.edit(source_administration_value=person)
+    self._makeComputerPartitions(computer)
+    software_product = self._makeSoftwareProduct()
+    software_release = self._requestSoftwareRelease(
+                                    software_product.getRelativeUrl())
+    url_string = software_release.getUrlString()
+    
+    self._makeSoftwareInstallation( computer, url_string)
+    
+    # Create Hosting Subscription and Software Instance
+    hosting_subscription = self._makeFullHostingSubscription(
+                                    url_string, person)
+    self._makeFullSoftwareInstance(hosting_subscription, url_string)
+    self._markComputerPartitionBusy(computer,
+                                    hosting_subscription.getPredecessorValue())
+    
+    # Install the Newest software release
+    software_release2 = self._requestSoftwareRelease(
+                                      software_product.getRelativeUrl())
+    self._makeSoftwareInstallation(computer,
+                                    software_release2.getUrlString())
+    self.tic()
+    
+    up_decision = hosting_subscription.HostingSubscription_createUpgradeDecision()
+    self.assertNotEqual(up_decision, None)
+    self.assertEqual(up_decision.getSimulationState(), 'planned')
+    transaction.commit()
+    # call a second time without tic
+    up_decision = hosting_subscription.HostingSubscription_createUpgradeDecision()
+    # no new Upgrade decision created
+    self.assertEqual(up_decision, None)
+
+  def testHostingSubscription_createUpgradeDecision(self):
     person = self._makePerson()
     computer = self._makeComputer()
     computer.edit(source_administration_value=person)
@@ -1133,7 +1196,8 @@ class TestSlapOSPDMSkins(SlapOSTestCaseMixin):
                                     software_release2.getUrlString())
     self.tic()
     
-    up_decision = computer.Computer_createHostingSubscriptionUpgradeDecision()[0]
+    up_decision = hosting_subscription.HostingSubscription_createUpgradeDecision()
+    self.assertNotEqual(up_decision, None)
     self.assertEqual(up_decision.getSimulationState(), 'planned')
     
     self.assertEqual(up_decision.UpgradeDecision_getHostingSubscription().\
@@ -1143,11 +1207,11 @@ class TestSlapOSPDMSkins(SlapOSTestCaseMixin):
                               getUrlString(), software_release2.getUrlString())
     
     self.tic()
-    up_decision2 = computer.Computer_createHostingSubscriptionUpgradeDecision()
-    self.assertEqual(len(up_decision2), 0)
+    up_decision2 = hosting_subscription.HostingSubscription_createUpgradeDecision()
+    self.assertEqual(up_decision2, None)
   
   
-  def testComputer_createHostingSubscriptionUpgradeDecision_with_exist(self):
+  def testHostingSubscription_createUpgradeDecision_with_exist(self):
     person = self._makePerson()
     computer = self._makeComputer()
     computer.edit(source_administration_value=person)
@@ -1172,7 +1236,7 @@ class TestSlapOSPDMSkins(SlapOSTestCaseMixin):
     self._makeSoftwareInstallation(computer, software_release2.getUrlString())
     self.tic()
     
-    up_decision = computer.Computer_createHostingSubscriptionUpgradeDecision()[0]
+    up_decision = hosting_subscription.HostingSubscription_createUpgradeDecision()
     self.assertEqual(up_decision.getSimulationState(), 'planned')
     
     # Install the another software release
@@ -1181,14 +1245,14 @@ class TestSlapOSPDMSkins(SlapOSTestCaseMixin):
     self._makeSoftwareInstallation(computer, software_release3.getUrlString())
     self.tic()
     
-    up_decision2 = computer.Computer_createHostingSubscriptionUpgradeDecision()[0]
+    up_decision2 = hosting_subscription.HostingSubscription_createUpgradeDecision()
     self.assertEqual(up_decision2.getSimulationState(), 'planned')
     self.assertEqual(up_decision.getSimulationState(), 'cancelled')
     release = up_decision2.UpgradeDecision_getSoftwareRelease()
     self.assertEqual(release.getUrlString(),
                                 software_release3.getUrlString())
 
-  def testComputer_createHostingSubscriptionUpgradeDecision_rejected(self):
+  def testHostingSubscription_createUpgradeDecision_rejected(self):
     person = self._makePerson()
     computer = self._makeComputer()
     computer.edit(source_administration_value=person)
@@ -1213,22 +1277,22 @@ class TestSlapOSPDMSkins(SlapOSTestCaseMixin):
     self._makeSoftwareInstallation(computer, software_release2.getUrlString())
     self.tic()
     
-    decision_list = computer.Computer_createHostingSubscriptionUpgradeDecision()
-    self.assertEqual(decision_list[0].getSimulationState(), 'planned')
+    up_decision = hosting_subscription.HostingSubscription_createUpgradeDecision()
+    self.assertEqual(up_decision.getSimulationState(), 'planned')
     
     # Reject upgrade decision
-    decision_list[0].reject()
+    up_decision.reject()
     self.tic()
     
     in_progress = software_release2.SoftwareRelease_getUpgradeDecisionInProgress(
                                 hosting_subscription.getUid())
-    decision_list = computer.Computer_createHostingSubscriptionUpgradeDecision()
+    up_decision = hosting_subscription.HostingSubscription_createUpgradeDecision()
     # There is an upgrade decision in progress
     self.assertNotEqual(in_progress, None)
     # No new upgrade decision created with software_release2
-    self.assertEqual(decision_list, [])
+    self.assertEqual(up_decision, None)
 
-  def testComputer_createHostingSubscriptionUpgradeDecision_rejected_2(self):
+  def testHostingSubscription_createUpgradeDecision_rejected_2(self):
     person = self._makePerson()
     computer = self._makeComputer()
     computer.edit(source_administration_value=person)
@@ -1253,11 +1317,10 @@ class TestSlapOSPDMSkins(SlapOSTestCaseMixin):
     self._makeSoftwareInstallation(computer, software_release2.getUrlString())
     self.tic()
     
-    decision_list = computer.Computer_createHostingSubscriptionUpgradeDecision()
-    self.assertEqual(decision_list[0].getSimulationState(), 'planned')
+    up_decision = hosting_subscription.HostingSubscription_createUpgradeDecision()
+    self.assertEqual(up_decision.getSimulationState(), 'planned')
     
     # Reject upgrade decision
-    up_decision = decision_list[0]
     up_decision.reject()
     self.tic()
     
@@ -1267,7 +1330,7 @@ class TestSlapOSPDMSkins(SlapOSTestCaseMixin):
     self._makeSoftwareInstallation(computer, software_release3.getUrlString())
     self.tic()
     
-    decision2 = computer.Computer_createHostingSubscriptionUpgradeDecision()[0]
+    decision2 = hosting_subscription.HostingSubscription_createUpgradeDecision()
     self.assertEqual(decision2.getSimulationState(), 'planned')
     self.assertEqual(up_decision.getSimulationState(), 'rejected')
     release = decision2.UpgradeDecision_getSoftwareRelease()