Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable pkg.installed to detect packages by their origin name on FreeBSD #67127

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/67126.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed pkg.install in test mode would not detect FreeBSD packages installed by their origin name
12 changes: 6 additions & 6 deletions salt/modules/pkgng.py
Original file line number Diff line number Diff line change
Expand Up @@ -927,7 +927,7 @@ def remove(

.. note::

This function can accessed using ``pkg.delete`` in addition to
This function can be accessed using ``pkg.delete`` in addition to
``pkg.remove``, to more closely match the CLI usage of ``pkg(8)``.

name
Expand Down Expand Up @@ -1904,7 +1904,7 @@ def hold(name=None, pkgs=None, **kwargs): # pylint: disable=W0613
.. note::
This function is provided primarily for compatibility with some
parts of :py:mod:`states.pkg <salt.states.pkg>`.
Consider using Consider using :py:func:`pkg.lock <salt.modules.pkgng.lock>` instead. instead.
Consider using :py:func:`pkg.lock <salt.modules.pkgng.lock>` instead. instead.

name
The name of the package to be held.
Expand Down Expand Up @@ -2030,7 +2030,7 @@ def list_locked(**kwargs):
Query the package database those packages which are
locked against reinstallation, modification or deletion.

Returns returns a list of package names with version strings
Returns a list of package names with version strings

CLI Example:

Expand Down Expand Up @@ -2369,15 +2369,15 @@ def _parse_upgrade(stdout):
"""
# Match strings like 'python36: 3.6.3 -> 3.6.4 [FreeBSD]'
upgrade_regex = re.compile(
r"^\s+([^:]+):\s([0-9a-z_,.]+)\s+->\s+([0-9a-z_,.]+)\s*(\[([^]]+)\])?\s*(\(([^)]+)\))?"
r"^\s+([^:]+):\s([0-9a-z_,.]+)\s+->\s+([0-9a-z_,.]+)\s*(\[([^]]+)])?\s*(\(([^)]+)\))?"
)
# Match strings like 'rubygem-bcrypt_pbkdf: 1.0.0 [FreeBSD]'
install_regex = re.compile(
r"^\s+([^:]+):\s+([0-9a-z_,.]+)\s*(\[([^]]+)\])?\s*(\(([^)]+)\))?"
r"^\s+([^:]+):\s+([0-9a-z_,.]+)\s*(\[([^]]+)])?\s*(\(([^)]+)\))?"
)
# Match strings like 'py27-yaml-3.11_2 [FreeBSD] (direct dependency changed: py27-setuptools)'
reinstall_regex = re.compile(
r"^\s+(\S+)-(?<=-)([0-9a-z_,.]+)\s*(\[([^]]+)\])?\s*(\(([^)]+)\))?"
r"^\s+(\S+)-(?<=-)([0-9a-z_,.]+)\s*(\[([^]]+)])?\s*(\(([^)]+)\))?"
)

result = {
Expand Down
10 changes: 9 additions & 1 deletion salt/states/pkg.py
Original file line number Diff line number Diff line change
Expand Up @@ -743,7 +743,15 @@ def _find_install_targets(
warnings = []
failed_verify = False
for package_name, version_string in desired.items():
cver = cur_pkgs.get(package_name, [])

# FreeBSD pkg supports `openjdk` and `java/openjdk7` package names
origin = bool(re.search('/', package_name))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need a regex to check if a substring is in a string; it can just be has_origin = '/' in package_name. However we might want to use a regex in order to not match cases like /package_name and package_name/


if __grains__['os'] == 'FreeBSD' and origin:
cver = [k for k, v in cur_pkgs.items() if v['origin'] == package_name]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are often hundreds or thousands of packages installed on a system. If someone is attempting to install many of these packages that have a / in the name then iterating over all the packages every time (O(n*m) might not be as performant as one would desire; although it could be an acceptable since there are probably not millions or hundreds of thousands of packages.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It appears there is a orgins dict stored in the context (

def version(*names, **kwargs):
); perhaps that could be utilized.

else:
cver = cur_pkgs.get(package_name, [])

if resolve_capabilities and not cver and package_name in cur_prov:
cver = cur_pkgs.get(cur_prov.get(package_name)[0], [])

Expand Down
52 changes: 52 additions & 0 deletions tests/pytests/unit/states/test_pkg.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import salt.modules.pacmanpkg as pacmanpkg
import salt.modules.pkg_resource as pkg_resource
import salt.modules.yumpkg as yumpkg
import salt.modules.pkgng as pkgng
import salt.states.beacon as beaconstate
import salt.states.pkg as pkg
import salt.utils.state as state_utils
Expand Down Expand Up @@ -48,6 +49,11 @@ def configure_loader_modules(minion_opts):
"__salt__": {},
"__grains__": {"os": "CentOS", "os_family": "RedHat"},
},
pkgng: {
"__salt__": {},
"__grains__": {"os": "FreeBSD", "osarch": "amd64", "osmajorrelease": 14},
"__opts__": minion_opts,
},
yumpkg: {
"__salt__": {},
"__grains__": {"osarch": "x86_64", "osmajorrelease": 7},
Expand Down Expand Up @@ -883,6 +889,52 @@ def test_installed_with_single_normalize():
assert ret["changes"] == expected


def test_installed_with_freebsd_origin():
"""
Test pkg.installed where the package name is specified as an origin name
"""

list_pkgs = MagicMock(
return_value={
"pkga": {
"origin": "test/pkga",
"version": ["1.0.1"],
}
}
)

install_mock = MagicMock(return_value={})

salt_dict = {
"pkg.install": install_mock,
"pkg.list_pkgs": list_pkgs,
"pkg_resource.version_clean": pkg_resource.version_clean,
"pkg_resource.parse_targets": pkg_resource.parse_targets,
"pkg_resource.check_extra_requirements": pkg_resource.check_extra_requirements,
}

with (
patch("salt.modules.pkgng.list_pkgs", list_pkgs),
patch("salt.modules.pkgng.version_cmp", MagicMock(return_value=0)),
patch.dict(pkg.__salt__, salt_dict),
patch.dict(pkg_resource.__salt__, salt_dict),
patch.dict(pkgng.__salt__, salt_dict),
patch.dict(
pkg.__grains__,
{
"os": "FreeBSD",
"osarch": "amd64",
"os_family": "FreeBSD",
"osmajorrelease": 14,
},
),
):

ret = pkg.installed("test/pkga")
install_mock.assert_not_called()
assert ret["result"]


def test_removed_with_single_normalize():
"""
Test pkg.removed with preventing multiple package name normalisation
Expand Down
Loading