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

(Backport 50125, 50671, 51973, 52412, 52744, 52844, 54024, 54379) Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) #54954

Merged
merged 18 commits into from
May 19, 2020

Conversation

aplanas
Copy link
Contributor

@aplanas aplanas commented Oct 11, 2019

What does this PR do?

(From #50125)
Provides a new root parameter in the public interfaces for rpm and zypper

(From #50671)
In _get_configured_repos if the directory where the repositories
are stored is not found, an log error is emitted.

Most of the times this is indeed an indication of an error, but is
not always the case. For example, when we are creating a chroot
environment we do not need to create this directory, as is generated
by zypper when a new repository is added.

This patch demote the error to a warning, as this directory will
be present eventually is most situations after the first call to
zypper.

(From #51973)
The virtual module 'pkg' is heavily used by other states, like the
'pkg' state, so differences in the signature of public functions
can generate errors like:

TypeError: xxx() got an unexpected keyword argument 'yyy'

The fix is to add the parameter 'yyy' into all versions of the
function 'xxx()' in all the 'pkg' virtual modules, but this
approach is cumbersome and not very resilient, as any refactoring
on the public interface or in the pkg state module will break
again the already agreed contract.

This patch append **kwarg in most of the public methods on virtual
module pkg where it is missing, fixing the described issue and
making more easy the refactoring of the code.

Also do the same for 'lowpkg'

(From #52412)

In the openSUSE family we have several kind of packages, like
patterns, patches and products. Currently we can use the pkg
state and the pkg module to install them, but as they are not
listed under any rpm command, the module and the state will
report a Fail.

This patch updates the zypperpkg list_pkgs to explicitly include
this kind of objects under the list, so the rest of the commands
from the same execution module (install, update, etc) will report
the correct restul at the end.

Because the way that the installed state from pkg is designed,
we cannot be implicit about the kind of objects that we are
installing, so we need to indicate in the SLS that at least one
object is a pattern or a patch:

  example:
    pkg.installed:
      - name: pattern:SUSE-MicroOS
      - includes: [pattern]

The includes parameter will be passed to the list_pkgs from
the zypper execution module, that will return the package list
using RPM (for packages) and zypper (for patterns). This more
complete report will be used in pkg.installed to identify the
correct installation of the new object.

Closes #52402

(From #52744)

Some patters are non-visible, and cannot be detected via

zypper se -t pattern

unless we know the name.

When we are listing all installed patterns, we still do not know
the names of the patterns, so we need to use a different approach.

This patch use rpm -q --provides --whatprovides search to detect
the non-visible patters, and complete the result for
list_installed_patterns.

(From #52844)

The `rpm -q --provides --whatprovides 'pattern()' command return
error code 1 when any pattern is present. This patch ignore the
return error code, as a not found pattern is not an error.

(From #54024)

The cache from pkg.list_pkgs for the zypper installer is too aggresive.
Some parameters will deliver different package lists, like root and
includes. The current cache do not take those parameters into
consideration, so the next time that this function is called, the last
list of packages will be returned, without checking if the current
parameters match the old one.

This patch create a different cache key for each parameter combination,
so the cached data will be separated too.

(From #54379)

When a patch is installed via advisory_ids, we do not want to change the
behavior of list_pkgs, via the includes field.

Currently, if a patch is installed via advisory_ids (state
pkg.patch_installed), it will call zypperpkg.install with this parameter
inside kwargs. This parameter is parsed by pkg_resource.parse_targets,
and the prefix patch: will be added very early, just before taking the
list of the current packages.

This last step will change the behavior of list_pkgs, as all the other
patches will be listed.

This sound OK except when we want to maintain the old behavior, when
patches are not listed in the install function by default.

This patch will add the patch: prefix in the list of targets later,
once we have the list of packages, restoring the old behavior.

Note that we can still install a patch as a normal package, if we add
the prefix in the name before calling install, and we do not use the
advisory_ids field.

What issues does this PR fix or reference?

New feature

Tests written?

Yes

(backport #50125, #50671, #51973, #52412, #52744, #52844 and #54024, already merged in develop)

(backport #54379 from develop, under review)

@aplanas aplanas changed the title (Backport 50125) Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) (Backport 50125, 50671) Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) Oct 11, 2019
@aplanas aplanas changed the title (Backport 50125, 50671) Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) (Backport 50125, 50671, 51973) Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) Oct 14, 2019
@aplanas aplanas changed the title (Backport 50125, 50671, 51973) Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) (Backport 50125, 50671, 51973, 52412) Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) Oct 14, 2019
@aplanas aplanas changed the title (Backport 50125, 50671, 51973, 52412) Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) (Backport 50125, 50671, 51973, 52412, 52744) Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) Oct 14, 2019
@aplanas aplanas changed the title (Backport 50125, 50671, 51973, 52412, 52744) Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) (Backport 50125, 50671, 51973, 52412, 52744, 52844) Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) Oct 14, 2019
@aplanas aplanas changed the title (Backport 50125, 50671, 51973, 52412, 52744, 52844) Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) (Backport 50125, 50671, 51973, 52412, 52744, 52844, 54024) Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) Oct 14, 2019
@aplanas aplanas changed the title (Backport 50125, 50671, 51973, 52412, 52744, 52844, 54024) Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) (Backport 50125, 50671, 51973, 52412, 52744, 52844, 54024, 54379) Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) Oct 14, 2019
@aplanas
Copy link
Contributor Author

aplanas commented Oct 14, 2019

Any change for this to be reviewed soon? It is actually a contention point, as this is a chain of changes that cover more than one year of commits, and there are more on the queue.

@aplanas
Copy link
Contributor Author

aplanas commented Dec 5, 2019

Ping: If I split it into each backport, will help the review? The chain is very deep, and the all the different splits will contain all the previous PR, and the last one will contain all the PR like this one, so I am not sure if this will help or damage.

aplanas and others added 17 commits May 14, 2020 13:24
(cherry picked from commit 6aae270)
The CLI rpm command allows the --root parameter to change the
expected location where the rpm database can be found.

This patch add a new optional parameter in the public interface
to allow the set of the new root location.

Update the tests to use the extra parameter.

(cherry picked from commit 5747448)
The zypper CLI provides a way to change the path where zypper expect
to find the required configuration files and repositories.

This feature is useful to bootstrap chroot environments, inspect
repositories and packages from locally mounted devices, or help
during the installation of a new OS from the SUSE family.

This patch add the root optional parameter for each command in the
public interface, and fix the tests.

(cherry picked from commit 1e57b30)
_Zypper class take note when a .call() is done, to clean up the data
when we access to some attribute.

This produces a bug when two calls are one after another an we set
some attributes via the __call__ method, as whatever is set will be
cleared after the first attribute is accessed.

For example:

zypper.attrib.call(..)
zypper(root=root).otherattrib.call(..)

The first call will set __called as True, and the reset of the inner
state of zypper will be cleared when otherattrib is accessed,
cleanning the status for __root.

This patch makes sure to clean the status also during the __call__
method, avoiding the cleanning when the attribute is accessed.

(cherry picked from commit 50e549d)
Add no_recommends parameter to install and upgrade actions.

(cherry picked from commit c2ace99)
In _get_configured_repos if the directory where the repositories
are stored is not found, an log error is emitted.

Most of the times this is indeed an indication of an error, but is
not always the case. For example, when we are creating a chroot
environment we do not need to create this directory, as is generated
by zypper when a new repository is added.

This patch demote the error to a warning, as this directory will
be present eventually is most situations after the first call to
zypper.

(cherry picked from commit 441e116)
The virtual module 'pkg' is heavily used by other states, like the
'pkg' state, so differences in the signature of public functions
can generate errors like:

  TypeError: xxx() got an unexpected keyword argument 'yyy'

The fix is to add the parameter 'yyy' into all versions of the
function 'xxx()' in all the 'pkg' virtual modules, but this
approach is cumbersome and not very resilient, as any refactoring
on the public interface or in the pkg state module will break
again the already agreed contract.

This patch append **kwarg in most of the public methods on virtual
module pkg where it is missing, fixing the described issue and
making more easy the refactoring of the code.

(cherry picked from commit fbb8a29)
In the same way that the previous commit for pkg, add **kwargs
into the public function signature for lowpkg virtual module, to
unify the different signatures.

(cherry picked from commit c9473a0)
In the openSUSE family we have several kind of packages, like
patterns, patches and products. Currently we can use the `pkg`
state and the `pkg` module to install them, but as they are not
listed under any `rpm` command, the module and the state will
report a Fail.

This patch updates the zypperpkg `list_pkgs` to explicitly include
this kind of objects under the list, so the rest of the commands
from the same execution module (install, update, etc) will report
the correct restul at the end.

Because the way that the `installed` state from `pkg` is designed,
we cannot be implicit about the kind of objects that we are
installing, so we need to indicate in the SLS that at least one
object is a pattern or a patch:

  example:
    pkg.installed:
      - name: pattern:SUSE-MicroOS
      - includes: [pattern]

The `includes` parameter will be passed to the `list_pkgs` from
the zypper execution module, that will return the package list
using RPM (for packages) and zypper (for patterns). This more
complete report will be used in `pkg.installed` to identify the
correct installation of the new object.

Closes saltstack#52402

(cherry picked from commit 0663d89)
Some patters are non-visible, and cannot be detected via

   zypper se -t pattern

unless we know the name.

When we are listing all installed patterns, we still do not know
the names of the patterns, so we need to use a different approach.

This patch use `rpm -q --provides --whatprovides` search to detect
the non-visible patters, and complete the result for
`list_installed_patterns`.

(cherry picked from commit 43ad8a6)
The `rpm -q --provides --whatprovides 'pattern()' command return
error code 1 when any pattern is present. This patch ignore the
return error code, as a not found pattern is not an error.

(cherry picked from commit 1afaf59)
The cache from pkg.list_pkgs for the zypper installer is too aggresive.
Some parameters will deliver different package lists, like root and
includes. The current cache do not take those parameters into
consideration, so the next time that this function is called, the last
list of packages will be returned, without checking if the current
parameters match the old one.

This patch create a different cache key for each parameter combination,
so the cached data will be separated too.

(cherry picked from commit 9c54bb3)
When a patch is installed via advisory_ids, we do not want to change the
behavior of list_pkgs, via the includes field.

Currently, if a patch is installed via advisory_ids (state
pkg.patch_installed), it will call zypperpkg.install with this parameter
inside kwargs. This parameter is parsed by pkg_resource.parse_targets,
and the prefix `patch:` will be added very early, just before taking the
list of the current packages.

This last step will change the behavior of list_pkgs, as all the other
patches will be listed.

This sound OK except when we want to maintain the old behavior, when
patches are not listed in the `install` function by default.

This patch will add the `patch:` prefix in the list of targets later,
once we have the list of packages, restoring the old behavior.

Note that we can still install a patch as a normal package, if we add
the prefix in the name before calling `install`, and we do not use the
`advisory_ids` field.

(cherry picked from commit 65c9944)
@aplanas
Copy link
Contributor Author

aplanas commented May 15, 2020

no tests are failing, any re-review?

@dwoz dwoz merged commit 3219b2d into saltstack:master May 19, 2020
@sagetherage sagetherage added the ZRelease-Sodium retired label label May 20, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pkg.installed and zypperpkg.py do not recognize installed patterns
5 participants