Merge pull request #644 from xcp-ng/pcoval/lib/review/master

l/host.py: Fix issue that update from non declared repositories

Author: glehmann

README.md

@@ -654,7 +654,7 @@ For each pool target :
 
 1. Update master host of the pool:
   * Clean cached metadata
-  * Update with repository manager (yum): Optionally enables repositories
+  * Update with repository manager (yum): Optionally enables or disables repositories
   * Reboot
 2. Get other hosts of the pool
   * Repeat step `1.` for each host
@@ -679,6 +679,7 @@ Take a look at an example inventory file:
 
 [default]
 repositories = ["xcp-ng-base"]
+disabled_repositories = ["epel"]
 hosting_pool = "A"
 
 [hosts]
@@ -696,6 +697,9 @@ hosting_pool = "B"
 > * Config values under `hosts` override values under `default`. For instance, the above inventory would produce
 > the following python dict:
 >
-> `{'ip_or_hostname-1': {'repositories': ['xcp-ng-base'], 'hosting_pool': 'A'}, 'ip_or_hostname-2': {'repositories': ['xcp-ng-updates'], 'hosting_pool': 'B'}}`
+> `{'ip_or_hostname-1': {'repositories': ['xcp-ng-base'], 'disabled_repositories': ['epel'], 'hosting_pool': 'A'}, 'ip_or_hostname-2': {'repositories': ['xcp-ng-updates'], 'hosting_pool': 'B'}}`
+>
+> * `disabled_repositories` disables one or more repositories during the update. It can be set under `default` or overridden per host, and can also be passed with the `-x/--disablerepo` flag.
+> * `*` as a repository value disables **all** repositories (e.g. `disabled_repositories = ["*"]`).
 >
 > * When `--inventory` flag is present, repos passed to `-e` flag won't be considered.

lib/host.py

@@ -511,27 +511,33 @@ def yum_clean_metadata(self) -> str:
         logging.info(f"[{self}] Removing cache metadata...")
         return self.ssh("yum clean metadata -q")
 
-    def yum_update(self, enablerepos: list[str] = []) -> str:
+    def yum_update(self, enablerepos: list[str] = [], disablerepos: list[str] = []) -> str:
         """Updates packages on target.
 
         Performs the following shell command::
 
             yum update -y
             # with enablerepos
             yum update -y --enablerepo=extra1 --enablerepos=extra2
+            # with disablerepos
+            yum update -y --disablerepo=extra1 --disablerepos=extra2
 
-        :param enablerepos: Enable one or more repositories (default: []).
+        :param enablerepos: Enable one or more repositories (default: [])
+        :param disablerepos: Disable one or more repositories (default: [])
         """
         base_command = "yum update -y"
 
         logging.info(f"[{self}] Updating packages...")
+        if disablerepos:
+            extra = " ".join(f"--disablerepo={r}" for r in disablerepos)
+            base_command = f"{base_command} {extra}"
         if enablerepos:
             extra = " ".join(f"--enablerepo={r}" for r in enablerepos)
             base_command = f"{base_command} {extra}"
 
         return self.ssh(base_command)
 
-    def update(self, enablerepos: list[str] = [], reboot: bool = True) -> None:
+    def update(self, enablerepos: list[str] = [], disablerepos: list[str] = [], reboot: bool = True) -> None:
         """Updates current host.
 
         An helper function that wraps update tasks on current host.
@@ -541,13 +547,15 @@ def update(self, enablerepos: list[str] = [], reboot: bool = True) -> None:
 
         :param list[str] enablerepos:
             Repositories to enable when updating.
+        :param list[str] disablerepos:
+            Repositories to disable when updating.
         :param bool reboot:
             Choose to reboot or not after update (default: True).
         """
         logging.info(f"[{self}] Updating...")
 
         self.yum_clean_metadata()
-        output = self.yum_update(enablerepos=enablerepos)
+        output = self.yum_update(enablerepos=enablerepos, disablerepos=disablerepos)
         if reboot and "No packages marked for update" not in output:
             # Everything's ok, just reboot
             self.reboot(verify=True)

lib/tools/cli.py

@@ -18,7 +18,7 @@ def _command_update(args: argparse.Namespace) -> None:
     if args.inventory:
         inventory = load_inventory(args.inventory)
     else:
-        inventory = into_inventory(args.hosts, args.repos, args.hosting_pool)
+        inventory = into_inventory(args.hosts, args.repos, args.hosting_pool, disabled_repositories=args.disablerepos)
 
     update_pools(inventory)
 
@@ -63,6 +63,13 @@ def cli() -> None:
         dest="repos",
         help="repositories to enable when updating",
     )
+    subparser_cmd_update.add_argument(
+        "-x", "--disablerepo",
+        metavar="REPO",
+        action="append",
+        dest="disablerepos",
+        help="repositories to disable when updating",
+    )
     subparser_cmd_update.add_argument(
         "-P",
         "--hosting-pool",

lib/tools/inventory.py

@@ -11,6 +11,7 @@
 
 class HostConfig(TypedDict):
     repositories: list[str]
+    disabled_repositories: list[str]
     hosting_pool: HostAddress | None
 
 
@@ -30,11 +31,13 @@ def load_inventory(inventory_path: Path) -> Inventory:
     inventory_hosts: HostConfigs = {}
     for h, config in hosts.items():
         repos = config.get("repositories", [])
+        disabled_repositories = config.get("disabled_repositories", [])
         hosting_pool = config.get("hosting_pool", None)
         if hosting_pool is None:
             hosting_pool = default.get("hosting_pool", None)
         host: HostConfig = {
             "repositories": repos or default.get("repositories", []),
+            "disabled_repositories": disabled_repositories or default.get("disabled_repositories", []),
             "hosting_pool": hosting_pool,
         }
         inventory_hosts[h] = host
@@ -44,7 +47,12 @@ def load_inventory(inventory_path: Path) -> Inventory:
     }
 
 
-def into_inventory(hosts: list[HostAddress], repositories: list[str], hosting_pool: HostAddress) -> Inventory:
+def into_inventory(
+    hosts: list[HostAddress],
+    repositories: list[str],
+    hosting_pool: HostAddress,
+    disabled_repositories: list[str] = [],
+) -> Inventory:
     """Create an inventory object from arguments.
 
     Basically, it is used as compatibility when we don't want inventory from file.
@@ -53,6 +61,7 @@ def into_inventory(hosts: list[HostAddress], repositories: list[str], hosting_po
     for h in hosts:
         host: HostConfig = {
             "repositories": repositories or [],
+            "disabled_repositories": disabled_repositories or [],
             "hosting_pool": hosting_pool or None,
         }
         inventory_hosts[h] = host

lib/tools/tasks/update.py

@@ -96,7 +96,10 @@ def update_pools(inventory: Inventory) -> None:
     # update master hosts
     with ThreadPoolExecutor() as executor:
         future_masters = {executor.submit(
-            p.master.update, inventory_hosts[p.master.hostname_or_ip]["repositories"]): p.master for p in pools}
+            p.master.update,
+            inventory_hosts[p.master.hostname_or_ip]["repositories"],
+            disablerepos=inventory_hosts[p.master.hostname_or_ip]["disabled_repositories"],
+        ): p.master for p in pools}
         for future in as_completed(future_masters):
             future_master = future_masters[future]
             try:
@@ -117,7 +120,8 @@ def update_pools(inventory: Inventory) -> None:
             for h in p.hosts[1:]:
                 # repos are the same as for the master host
                 repos = inventory_hosts[p.master.hostname_or_ip]["repositories"]
-                future_other_hosts[executor.submit(h.update, repos)] = h
+                disablerepos = inventory_hosts[p.master.hostname_or_ip]["disabled_repositories"]
+                future_other_hosts[executor.submit(h.update, repos, disablerepos=disablerepos)] = h
         for future in as_completed(future_other_hosts):
             other_host = future_other_hosts[future]
             try: