From eb08e60765003e1732eaa1218d6d933b4d44bb37 Mon Sep 17 00:00:00 2001
From: Vsevolod Fedorov <vsevolod.fedorov@gmail.com>
Date: Fri, 22 Mar 2024 12:56:48 +0300
Subject: [PATCH] Story 2011049: Delete also old views when --delete-old option
 is specified

Improve 'update' command to handle old views also, not just jobs.

Task: 49601

Change-Id: I60431f662d5a69c084b2698d08104a1524d02767
---
 doc/source/changelog.rst              |   5 +
 jenkins_jobs/builder.py               |  40 ++++++--
 jenkins_jobs/cli/subcommand/update.py |   6 +-
 tests/cmd/subcommands/test_update.py  | 139 +++++++++++++++++++++++++-
 tests/jenkins_manager/test_manager.py |  30 +++++-
 5 files changed, 205 insertions(+), 15 deletions(-)

diff --git a/doc/source/changelog.rst b/doc/source/changelog.rst
index 022ee302b..2e5eaed4c 100644
--- a/doc/source/changelog.rst
+++ b/doc/source/changelog.rst
@@ -4,6 +4,11 @@ Changelog
 Release 6.2.0
 -------------
 
+Features added
+~~~~~~~~~~~~~~
+
+* `--delete-old` flag for `update` cli command is now deletes obsolete views also, not only jobs.
+
 Bugs fixed
 ~~~~~~~~~~
 
diff --git a/jenkins_jobs/builder.py b/jenkins_jobs/builder.py
index a37de91a5..a48833e88 100644
--- a/jenkins_jobs/builder.py
+++ b/jenkins_jobs/builder.py
@@ -183,7 +183,7 @@ class JenkinsManager(object):
             self._job_list = None
         return self.jobs
 
-    def is_managed(self, job_name):
+    def is_managed_job(self, job_name):
         xml = self.jenkins.get_job_config(job_name)
         try:
             out = XML.fromstring(xml.encode("utf-8"))
@@ -199,7 +199,7 @@ class JenkinsManager(object):
             self._plugins_list = self.get_plugins_info()
         return self._plugins_list
 
-    def delete_old_managed(self, keep=None):
+    def delete_old_managed_jobs(self, keep=None):
         jobs = self.get_jobs()
         deleted_jobs = 0
         if keep is None:
@@ -210,7 +210,7 @@ class JenkinsManager(object):
             if job["fullname"] not in keep and self.is_job(
                 job["fullname"], use_cache=False
             ):
-                if self.is_managed(job["fullname"]):
+                if self.is_managed_job(job["fullname"]):
                     logger.info(
                         "Removing obsolete jenkins job {0}".format(job["fullname"])
                     )
@@ -376,14 +376,42 @@ class JenkinsManager(object):
             self._view_list = None
         return self.views
 
-    def is_view(self, view_name):
-        # first use cache
-        if view_name in self.view_list:
+    def is_view(self, view_name, use_cache=True):
+        if use_cache and view_name in self.view_list:
             return True
 
         # if not exists, use jenkins
         return self.jenkins.view_exists(view_name)
 
+    def is_managed_view(self, view_name):
+        xml = self.jenkins.get_view_config(view_name)
+        try:
+            out = XML.fromstring(xml.encode("utf-8"))
+            description = out.find(".//description").text
+            return description.endswith(MAGIC_MANAGE_STRING)
+        except (TypeError, AttributeError):
+            pass
+        return False
+
+    def delete_old_managed_views(self, keep=None):
+        view_list = self.get_views()
+        deleted_views = 0
+        if keep is None:
+            keep = []
+        for view in view_list:
+            if view["name"] not in keep and self.is_view(view["name"], use_cache=False):
+                if self.is_managed_view(view["name"]):
+                    logger.info(
+                        "Removing obsolete jenkins view {0}".format(view["name"])
+                    )
+                    self.delete_view(view["name"])
+                    deleted_views += 1
+                else:
+                    logger.debug("Not deleting unmanaged jenkins view %s", view["name"])
+            else:
+                logger.debug("Keeping view %s", view["name"])
+        return deleted_views
+
     def delete_view(self, view_name):
         if self.is_view(view_name):
             logger.info("Deleting jenkins view {}".format(view_name))
diff --git a/jenkins_jobs/cli/subcommand/update.py b/jenkins_jobs/cli/subcommand/update.py
index 0478e67d7..a5df18495 100644
--- a/jenkins_jobs/cli/subcommand/update.py
+++ b/jenkins_jobs/cli/subcommand/update.py
@@ -147,5 +147,9 @@ class UpdateSubCommand(base.JobsSubCommand):
         if options.delete_old:
             if options.update in {"jobs", "all"}:
                 keep_jobs = [job.name for job in xml_jobs]
-                n = builder.delete_old_managed(keep=keep_jobs)
+                n = builder.delete_old_managed_jobs(keep=keep_jobs)
                 logger.info("Number of jobs deleted: %d", n)
+            if options.update in {"views", "all"}:
+                keep_views = [view.name for view in xml_views]
+                n = builder.delete_old_managed_views(keep=keep_views)
+                logger.info("Number of views deleted: %d", n)
diff --git a/tests/cmd/subcommands/test_update.py b/tests/cmd/subcommands/test_update.py
index 237ed1f53..83aa21f11 100644
--- a/tests/cmd/subcommands/test_update.py
+++ b/tests/cmd/subcommands/test_update.py
@@ -112,6 +112,8 @@ def test_update_jobs_and_delete_old(
     )
     jenkins_delete_job = mocker.patch("jenkins_jobs.builder.jenkins.Jenkins.delete_job")
 
+    mocker.patch("jenkins_jobs.builder.jenkins.Jenkins.get_views")
+
     yaml_jobs = ["bar001", "bar002", "baz001", "bam001"]
     extra_jobs = ["old_job001", "old_job002", "unmanaged"]
 
@@ -123,7 +125,7 @@ def test_update_jobs_and_delete_old(
     ]
 
     mocker.patch(
-        "jenkins_jobs.builder.JenkinsManager.is_managed",
+        "jenkins_jobs.builder.JenkinsManager.is_managed_job",
         side_effect=(lambda name: name != "unmanaged"),
     )
     execute_jenkins_jobs(args)
@@ -155,22 +157,39 @@ def test_update_jobs_and_delete_old_views_only(
     jenkins_delete_job = mocker.patch("jenkins_jobs.builder.jenkins.Jenkins.delete_job")
 
     mocker.patch("jenkins_jobs.builder.jenkins.Jenkins.view_exists")
-    mocker.patch("jenkins_jobs.builder.jenkins.Jenkins.get_views")
+    jenkins_get_all_views = mocker.patch(
+        "jenkins_jobs.builder.jenkins.Jenkins.get_views"
+    )
     jenkins_reconfig_view = mocker.patch(
         "jenkins_jobs.builder.jenkins.Jenkins.reconfig_view"
     )
+    jenkins_delete_view = mocker.patch(
+        "jenkins_jobs.builder.jenkins.Jenkins.delete_view"
+    )
 
     yaml_jobs = ["job-1", "job-2", "job-3"]
     extra_managed_jobs = ["old-job-1", "old-job-2"]
-    unmanaged_jobs = ["unmanaged"]
+    unmanaged_jobs = ["unmanaged-job"]
+
+    yaml_views = ["view-1", "view-2", "view-3"]
+    extra_managed_views = ["old-view-1", "old-view-2"]
+    unmanaged_views = ["unmanaged-view"]
 
     jenkins_get_all_jobs.return_value = [
         {"fullname": name} for name in yaml_jobs + extra_managed_jobs + unmanaged_jobs
     ]
+    jenkins_get_all_views.return_value = [
+        {"name": name} for name in yaml_views + extra_managed_views + unmanaged_views
+    ]
+
     mocker.patch(
-        "jenkins_jobs.builder.JenkinsManager.is_managed",
+        "jenkins_jobs.builder.JenkinsManager.is_managed_job",
         side_effect=(lambda name: name not in unmanaged_jobs),
     )
+    mocker.patch(
+        "jenkins_jobs.builder.JenkinsManager.is_managed_view",
+        side_effect=(lambda name: name not in unmanaged_views),
+    )
 
     path = fixtures_dir / "update-both.yaml"
     args = [
@@ -188,6 +207,7 @@ def test_update_jobs_and_delete_old_views_only(
     assert jenkins_delete_job.call_count == 0
 
     assert jenkins_reconfig_view.call_count == 3
+    assert jenkins_delete_view.call_count == 2
 
 
 def test_update_views(mocker, fixtures_dir, default_config_file, execute_jenkins_jobs):
@@ -296,6 +316,117 @@ def test_update_views_only(
     assert reconfig_view.call_count == 3
 
 
+def test_update_views_and_delete_old(
+    mocker, fixtures_dir, default_config_file, execute_jenkins_jobs
+):
+    """Test update behaviour with --delete-old option for views."""
+    mocker.patch("jenkins_jobs.builder.jenkins.Jenkins.get_all_jobs")
+    mocker.patch("jenkins_jobs.builder.jenkins.Jenkins.view_exists")
+    jenkins_get_all_views = mocker.patch(
+        "jenkins_jobs.builder.jenkins.Jenkins.get_views"
+    )
+    jenkins_reconfig_view = mocker.patch(
+        "jenkins_jobs.builder.jenkins.Jenkins.reconfig_view"
+    )
+    jenkins_delete_view = mocker.patch(
+        "jenkins_jobs.builder.jenkins.Jenkins.delete_view"
+    )
+
+    yaml_views = ["view-1", "view-2", "view-3"]
+    extra_managed_views = ["old-view-1", "old-view-2"]
+    unmanaged_views = ["unmanaged"]
+
+    path = fixtures_dir / "update-views.yaml"
+    args = ["--conf", default_config_file, "update", "--delete-old", str(path)]
+
+    jenkins_get_all_views.return_value = [
+        {"name": name} for name in yaml_views + extra_managed_views + unmanaged_views
+    ]
+
+    mocker.patch(
+        "jenkins_jobs.builder.JenkinsManager.is_managed_view",
+        side_effect=(lambda name: name not in unmanaged_views),
+    )
+    execute_jenkins_jobs(args)
+
+    jenkins_reconfig_view.assert_has_calls(
+        [mock.call(view_name, mock.ANY) for view_name in yaml_views], any_order=True
+    )
+    calls = [mock.call(name) for name in extra_managed_views]
+    jenkins_delete_view.assert_has_calls(calls)
+    assert jenkins_delete_view.call_count == len(calls)
+
+
+def test_update_views_and_delete_old_jobs_only(
+    mocker, fixtures_dir, default_config_file, execute_jenkins_jobs
+):
+    """Test update behaviour with --delete-old option
+    with --jobs-only option specified.
+    No views should be deleted or updated.
+    """
+    mocker.patch("jenkins_jobs.builder.jenkins.Jenkins.job_exists")
+    jenkins_get_all_jobs = mocker.patch(
+        "jenkins_jobs.builder.jenkins.Jenkins.get_all_jobs"
+    )
+    jenkins_reconfig_job = mocker.patch(
+        "jenkins_jobs.builder.jenkins.Jenkins.reconfig_job"
+    )
+    jenkins_delete_job = mocker.patch("jenkins_jobs.builder.jenkins.Jenkins.delete_job")
+
+    mocker.patch("jenkins_jobs.builder.jenkins.Jenkins.view_exists")
+    jenkins_get_all_views = mocker.patch(
+        "jenkins_jobs.builder.jenkins.Jenkins.get_views"
+    )
+    jenkins_reconfig_view = mocker.patch(
+        "jenkins_jobs.builder.jenkins.Jenkins.reconfig_view"
+    )
+    jenkins_delete_view = mocker.patch(
+        "jenkins_jobs.builder.jenkins.Jenkins.delete_view"
+    )
+
+    yaml_jobs = ["job-1", "job-2", "job-3"]
+    extra_managed_jobs = ["old-job-1", "old-job-2"]
+    unmanaged_jobs = ["unmanaged-job"]
+
+    yaml_views = ["view-1", "view-2", "view-3"]
+    extra_managed_views = ["old-view-1", "old-view-2"]
+    unmanaged_views = ["unmanaged-view"]
+
+    path = fixtures_dir / "update-both.yaml"
+    args = [
+        "--conf",
+        default_config_file,
+        "update",
+        "--delete-old",
+        "--jobs-only",
+        str(path),
+    ]
+
+    jenkins_get_all_jobs.return_value = [
+        {"fullname": name} for name in yaml_jobs + extra_managed_jobs + unmanaged_jobs
+    ]
+    jenkins_get_all_views.return_value = [
+        {"name": name} for name in yaml_views + extra_managed_views + unmanaged_views
+    ]
+
+    mocker.patch(
+        "jenkins_jobs.builder.JenkinsManager.is_managed_job",
+        side_effect=(lambda name: name not in unmanaged_jobs),
+    )
+    mocker.patch(
+        "jenkins_jobs.builder.JenkinsManager.is_managed_view",
+        side_effect=(lambda name: name not in unmanaged_views),
+    )
+
+    execute_jenkins_jobs(args)
+
+    assert jenkins_reconfig_job.call_count == 3
+    assert jenkins_delete_job.call_count == 2
+
+    assert jenkins_reconfig_view.call_count == 0
+    assert jenkins_delete_view.call_count == 0
+
+
 @pytest.mark.skip(reason="TODO: Develop actual update timeout test approach.")
 def test_update_timeout_not_set():
     """Validate update timeout behavior when timeout not explicitly configured."""
diff --git a/tests/jenkins_manager/test_manager.py b/tests/jenkins_manager/test_manager.py
index ddde7eac0..7386cd2da 100644
--- a/tests/jenkins_manager/test_manager.py
+++ b/tests/jenkins_manager/test_manager.py
@@ -50,7 +50,7 @@ def test_plugins_list_from_jenkins(mocker, jjb_config):
     assert list(builder.plugins_list) == list(_plugins_info.values())
 
 
-def test_delete_managed(mocker, jjb_config):
+def test_delete_old_managed_jobs(mocker, jjb_config):
     jjb_config.builder["plugins_info"] = None
     builder = jenkins_jobs.builder.JenkinsManager(jjb_config)
 
@@ -58,20 +58,42 @@ def test_delete_managed(mocker, jjb_config):
         "jenkins_jobs.builder.JenkinsManager",
         get_jobs=mock.DEFAULT,
         is_job=mock.DEFAULT,
-        is_managed=mock.DEFAULT,
+        is_managed_job=mock.DEFAULT,
         delete_job=mock.DEFAULT,
     )
     patches["get_jobs"].return_value = [
         {"fullname": "job1"},
         {"fullname": "job2"},
     ]
-    patches["is_managed"].side_effect = [True, True]
+    patches["is_managed_job"].side_effect = [True, True]
     patches["is_job"].side_effect = [True, True]
 
-    builder.delete_old_managed()
+    builder.delete_old_managed_jobs()
     assert patches["delete_job"].call_count == 2
 
 
+def test_delete_old_managed_views(mocker, jjb_config):
+    jjb_config.builder["plugins_info"] = None
+    builder = jenkins_jobs.builder.JenkinsManager(jjb_config)
+
+    patches = mocker.patch.multiple(
+        "jenkins_jobs.builder.JenkinsManager",
+        get_views=mock.DEFAULT,
+        is_view=mock.DEFAULT,
+        is_managed_view=mock.DEFAULT,
+        delete_view=mock.DEFAULT,
+    )
+    patches["get_views"].return_value = [
+        {"name": "view-1"},
+        {"name": "view-2"},
+    ]
+    patches["is_managed_view"].side_effect = [True, True]
+    patches["is_view"].side_effect = [True, True]
+
+    builder.delete_old_managed_views()
+    assert patches["delete_view"].call_count == 2
+
+
 @pytest.mark.parametrize("error_string", ["Connection refused", "Forbidden"])
 def test_get_plugins_info_error(mocker, jjb_config, error_string):
     builder = jenkins_jobs.builder.JenkinsManager(jjb_config)