copr / copr

Clone

#1683 Chroot EOL management fixes
Merged 3 years ago by praiskup. Opened 3 years ago by praiskup.
Unknown source eol-mgmt-fixes  into  master

file modified
+13 -3 
@@ -21,9 +21,15 @@
 

  )
 

  def alter_chroot(chroot_names, action):
 

      """Activates or deactivates a chroot"""
 

+     func_alter_chroot(chroot_names, action)
 

+ 

+ def func_alter_chroot(chroot_names, action):
 

+     """
 

+     A library-like variant of 'alter_chroot', used for unit-testing purposes.
 

+     """
 

      activate = (action == "activate")
 

  

-     delete_after_days = app.config["DELETE_EOL_CHROOTS_AFTER"] + 1
 

+     delete_after_days = app.config["DELETE_EOL_CHROOTS_AFTER"]
 

      delete_after_timestamp = datetime.datetime.now() + datetime.timedelta(delete_after_days)
 

  

      for chroot_name in chroot_names:
 
@@ -36,8 +42,12 @@
 

                      continue
 

  

                  for copr_chroot in mock_chroot.copr_chroots:
 

-                     # reset EOL policy after re-activation
 

-                     copr_chroot.delete_after = None if activate else delete_after_timestamp
 

+                     if activate:
 

+                         # reset EOL policy after re-activation
 

+                         copr_chroot.delete_after = None
 

+                         copr_chroot.delete_notify = None
 

+                     else:
 

+                         copr_chroot.delete_after = delete_after_timestamp
 

  

          except exceptions.MalformedArgumentException:
 

              print_invalid_format(chroot_name)

file modified
+1 -1 
@@ -65,7 +65,7 @@
 

          """
 

          A `user` decided to extend the preservation period for some EOL chroot
 

          """
 

-         delete_after_days = app.config["DELETE_EOL_CHROOTS_AFTER"] + 1
 

+         delete_after_days = app.config["DELETE_EOL_CHROOTS_AFTER"]
 

          cls._update_copr_chroot(copr_chroot, delete_after_days)
 

          (models.ReviewedOutdatedChroot.query
 

           .filter(models.ReviewedOutdatedChroot.copr_chroot_id

file modified
+8 -4 
@@ -1584,12 +1584,16 @@
 

      @property
 

      def delete_after_expired(self):
 

          """
 

-         Is the chroot going to expire in the future or it has already expired?
 

-         Using `delete_after_days` as a boolean is not sufficient because that
 

-         would return wrong results for the last 24 hours.
 

+         Is the chroot expired, aka its contents are going to be removed very
 

+         soon, or already removed?  Using `delete_after_days` as a boolean is not
 

+         sufficient because that would return wrong results for the last 24
 

+         hours.
 

          """
 

          if not self.delete_after:
 

-             return None
 

+             if self.delete_notify:
 

+                 # already deleted, see Deleter.delete() method
 

+                 return True
 

+             return False
 

          return self.delete_after < datetime.datetime.now()
 

  

      @property

file modified
+70 
@@ -5,6 +5,8 @@
 

  from coprs.logic.outdated_chroots_logic import OutdatedChrootsLogic
 

  from coprs.logic.complex_logic import ComplexLogic
 

  from coprs import app
 

+ from commands.alter_chroot import func_alter_chroot
 

+ from commands.delete_outdated_chroots import delete_outdated_chroots_function
 

  

  

  class TestOutdatedChrootsLogic(CoprsTestCase):
 
@@ -158,3 +160,71 @@
 

  

          chroot.delete_after = datetime.now() + timedelta(days=-35)
 

          assert chroot.delete_after_expired
 

+ 

+ 

+     @new_app_context
 

+     @pytest.mark.usefixtures("f_users", "f_coprs", "f_mock_chroots", "f_builds",
 

+                              "f_db")
 

+     def test_expired_chroot_detection(self):
 

+         """
 

+         Test fix for issue #1682, that expired chroots are not printed out.
 

+         """
 

+ 

+         def _assert_unaffected(cc):
 

+             assert cc.is_active is True
 

+             assert cc.delete_after is None
 

+             assert cc.delete_notify is None
 

+ 

+         # (1) Somewhere in the future (now +180 days by default) fedora-17 is
 

+         #     going to be expired.
 

+         func_alter_chroot(["fedora-17-x86_64", "fedora-17-i386"], "eol")
 

+         found = 0

Nit, this variable is not used until later. And then it is reinitialized back to 0, so we don't need this line.

 
+         for cc in self.models.CoprChroot.query.all():
 

+             if "fedora-17" in cc.name:
 

+                 assert cc.delete_after >= datetime.now() + timedelta(days=179)
 

+                 assert cc.delete_notify is None
 

+                 assert cc.delete_after_expired is False
 

+                 # simulate that some mails were sent
 

+                 cc.delete_notify = datetime.now()
 

+                 continue
 

+             _assert_unaffected(cc)
 

+ 

+         # (2) Reactivate the chroots.  Happened at least for epel-7 chroots
 

+         #     historically.
 

+         func_alter_chroot(["fedora-17-x86_64", "fedora-17-i386"], "activate")
 

+         for cc in self.models.CoprChroot.query.all():
 

+             assert cc.delete_after_expired is False
 

+             _assert_unaffected(cc)
 

+ 

+         # (3) Mark as EOL again, when is appropriate time.  And Expire.
 

+         backup = self.app.config["DELETE_EOL_CHROOTS_AFTER"]
 

+         self.app.config["DELETE_EOL_CHROOTS_AFTER"] = 0
 

+         func_alter_chroot(["fedora-17-x86_64", "fedora-17-i386"], "eol")
 

+         self.app.config["DELETE_EOL_CHROOTS_AFTER"] = backup
 

+         found = 0
 

+         for cc in self.models.CoprChroot.query.all():
 

+             if "fedora-17" in cc.name:
 

+                 found += 1
 

+                 assert cc.is_active is False
 

+                 assert cc.delete_after <= datetime.now() + timedelta(days=179)
 

+                 assert cc.delete_notify is None
 

+                 assert cc.delete_after_expired is True
 

+                 # unblock the delete action
 

+                 cc.delete_notify = datetime.now()
 

+             else:
 

+                 _assert_unaffected(cc)
 

+         assert found == 2
 

+ 

+         # (4) Delete the expired chroots!
 

+         delete_outdated_chroots_function(False)
 

+         found = 0
 

+         for cc in self.models.CoprChroot.query.all():
 

+             if "fedora-17" in cc.name:
 

+                 found += 1
 

+                 assert cc.is_active is False
 

+                 assert cc.delete_after is None
 

+                 assert cc.delete_notify is not None
 

+                 assert cc.delete_after_expired is True
 

+             else:
 

+                 _assert_unaffected(cc)
 

+         assert found == 2

no initial comment

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci

rebased onto ce1cf86509524f3e5ea7fb35d2177c0020b91f4e
3 years ago

Build succeeded.

[copr-build]

[copr-build]

PTAL, the first patch is the one that I'd like to hot-fix in production.

[copr-build]

Nit, this variable is not used until later. And then it is reinitialized back to 0, so we don't need this line.

LGTM, thank you for the PR.
The test looks really good, hopefully, this PR will finally fix the EOL-chroots feature.

[copr-build]

rebased onto 2c380f2
3 years ago

Build succeeded.

Pull-Request has been merged by praiskup
3 years ago
Metadata
None
No Tags
Changes Summary 4
+13 -3
file changed
frontend/coprs_frontend/commands/alter_chroot.py
+1 -1
file changed
frontend/coprs_frontend/coprs/logic/outdated_chroots_logic.py
+8 -4
file changed
frontend/coprs_frontend/coprs/models.py
+70 -0
file changed
frontend/coprs_frontend/tests/test_logic/test_outdated_chroots_logic.py
Powered by Pagure 5.14.1
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.