dnf,dnf5 - add the best option and fix nobest (#82627)

best/nobest options are one of the options whose default values are set
by an OS distribution. For example in our CI, both Fedora and RHEL set
the best option to different default values. As such we should defer to
the distributions for the default value and not change it by default but
if users wish to change it they can do so explicitly.

Currently the dnf module sets the nobest option inconsistenly and not for
all cases. This patch fixes that to reflect the behavior described
above. In addition adding the best option for both dnf and dnf5 modules
since the best option is prefer to nobest in dnf while in dnf5 nobest is
completely removed in favor of best.

Fixes #82616

changelogs/fragments/82616-dnf-best-nobest.yml

@@ -0,0 +1,6 @@
+minor_changes:
+  - dnf - add the ``best`` option
+  - dnf5 - add the ``best`` option
+bugfixes:
+  - dnf - the ``nobest`` option only overrides the distribution default when explicitly used, and is used for all supported operations (https://github.com/ansible/ansible/issues/82616)
+  - dnf5 - the ``nobest`` option only overrides the distribution default when used

lib/ansible/module_utils/yumdnf.py

@@ -18,6 +18,7 @@ yumdnf_argument_spec = dict(
         allow_downgrade=dict(type='bool', default=False),
         allowerasing=dict(default=False, type="bool"),
         autoremove=dict(type='bool', default=False),
+        best=dict(type="bool"),
         bugfix=dict(required=False, type='bool', default=False),
         cacheonly=dict(type='bool', default=False),
         conf_file=dict(type='str'),
@@ -38,7 +39,7 @@ yumdnf_argument_spec = dict(
         install_weak_deps=dict(type='bool', default=True),
         list=dict(type='str'),
         name=dict(type='list', elements='str', aliases=['pkg'], default=[]),
-        nobest=dict(default=False, type="bool"),
+        nobest=dict(type="bool"),
         releasever=dict(default=None),
         security=dict(type='bool', default=False),
         skip_broken=dict(type='bool', default=False),
@@ -51,7 +52,7 @@ yumdnf_argument_spec = dict(
         lock_timeout=dict(type='int', default=30),
     ),
     required_one_of=[['name', 'list', 'update_cache']],
-    mutually_exclusive=[['name', 'list']],
+    mutually_exclusive=[['name', 'list'], ['best', 'nobest']],
     supports_check_mode=True,
 )
 
@@ -70,6 +71,7 @@ class YumDnf(metaclass=ABCMeta):
         self.allow_downgrade = self.module.params['allow_downgrade']
         self.allowerasing = self.module.params['allowerasing']
         self.autoremove = self.module.params['autoremove']
+        self.best = self.module.params['best']
         self.bugfix = self.module.params['bugfix']
         self.cacheonly = self.module.params['cacheonly']
         self.conf_file = self.module.params['conf_file']

lib/ansible/modules/dnf.py

@@ -245,11 +245,19 @@ options:
     version_added: "2.10"
   nobest:
     description:
-      - Set best option to False, so that transactions are not limited to best candidates only.
+      - This is the opposite of the O(best) option kept for backwards compatibility.
+      - Since ansible-core 2.17 the default value is set by the operating system distribution.
     required: false
     type: bool
-    default: "no"
     version_added: "2.11"
+  best:
+    description:
+      - When set to V(true), either use a package with the highest version available or fail.
+      - When set to V(false), if the latest version cannot be installed go with the lower version.
+      - Default is set by the operating system distribution.
+    required: false
+    type: bool
+    version_added: "2.17"
   cacheonly:
     description:
       - Tells dnf to run entirely from system cache; does not download or update metadata.
@@ -678,9 +686,11 @@ class DnfModule(YumDnf):
         if self.skip_broken:
             conf.strict = 0
 
-        # Set best
-        if self.nobest:
-            conf.best = 0
+        # best and nobest are mutually exclusive
+        if self.nobest is not None:
+            conf.best = not self.nobest
+        elif self.best is not None:
+            conf.best = self.best
 
         if self.download_only:
             conf.downloadonly = True
@@ -1195,13 +1205,6 @@ class DnfModule(YumDnf):
                         response['results'].append("Packages providing %s not installed due to update_only specified" % spec)
                 else:
                     for pkg_spec in pkg_specs:
-                        # Previously we forced base.conf.best=True here.
-                        # However in 2.11+ there is a self.nobest option, so defer to that.
-                        # Note, however, that just because nobest isn't set, doesn't mean that
-                        # base.conf.best is actually true. We only force it false in
-                        # _configure_base(), we never set it to true, and it can default to false.
-                        # Thus, we still need to explicitly set it here.
-                        self.base.conf.best = not self.nobest
                         install_result = self._mark_package_install(pkg_spec, upgrade=True)
                         if install_result['failed']:
                             if install_result['msg']:

lib/ansible/modules/dnf5.py

@@ -208,10 +208,18 @@ options:
     default: "no"
   nobest:
     description:
-      - Set best option to False, so that transactions are not limited to best candidates only.
+      - This is the opposite of the O(best) option kept for backwards compatibility.
+      - Since ansible-core 2.17 the default value is set by the operating system distribution.
     required: false
     type: bool
-    default: "no"
+  best:
+    description:
+      - When set to V(true), either use a package with the highest version available or fail.
+      - When set to V(false), if the latest version cannot be installed go with the lower version.
+      - Default is set by the operating system distribution.
+    required: false
+    type: bool
+    version_added: "2.17"
   cacheonly:
     description:
       - Tells dnf to run entirely from system cache; does not download or update metadata.
@@ -498,7 +506,11 @@ class Dnf5Module(YumDnf):
                 self.disable_excludes = "*"
             conf.disable_excludes = self.disable_excludes
         conf.skip_broken = self.skip_broken
-        conf.best = not self.nobest
+        # best and nobest are mutually exclusive
+        if self.nobest is not None:
+            conf.best = not self.nobest
+        elif self.best is not None:
+            conf.best = self.best
         conf.install_weak_deps = self.install_weak_deps
         conf.gpgcheck = not self.disable_gpg_check
         conf.localpkg_gpgcheck = not self.disable_gpg_check

test/integration/targets/dnf/tasks/skip_broken_and_nobest.yml

@@ -264,6 +264,7 @@
         name:
           - broken-a
         state: latest
+        best: true
       ignore_errors: true
       register: dnf_fail