Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) #50125

aplanas · 2018-10-19T08:55:10Z

What does this PR do?

Provides a new root parameter in the public interfaces for rpm and zypper

What issues does this PR fix or reference?

New feature

Tests written?

For the low level RPM yes, for the zypper level I adjusted the current tests.

aplanas · 2018-10-19T13:23:18Z

Rebased

salt/modules/rpm.py

salt/modules/zypper.py

aplanas · 2018-10-23T13:31:43Z

@isbm ready for a deeper review!

rallytime · 2018-10-30T13:17:51Z

@aplanas There are some tests failing with this change. Can you take a look?

https://jenkinsci.saltstack.com/job/pr-kitchen-centos7-py2/job/PR-50125/12/

aplanas · 2018-10-30T13:22:24Z

@rallytime indeed, thanks

isbm

@aplanas OK, I stopped, but there are more. Please first fix these. 😉

salt/modules/rpm.py

aplanas · 2018-10-31T19:19:40Z

@isbm I am done with the 'in's. Thanks!

isbm · 2018-11-06T09:08:22Z

salt/modules/zypper.py

        # Ignore exit code for 106 (repo is not available)
        if 'no_repo_failure' in kwargs:
            self.__ignore_repo_failure = kwargs['no_repo_failure']
        if 'systemd_scope' in kwargs:
            self.__systemd_scope = kwargs['systemd_scope']
+        if 'root' in kwargs:


if kwargs.get('root'):

On a second though, are you sure? This check if to assign self.__root later in the same fashion that 'no_repo_failure' and 'systemd_scope'

Well, OK, copypaste. Maybe let's to as is. But I still think the other parts also needs to be type-safe.

salt/modules/zypper.py

aplanas · 2018-11-09T12:57:54Z

@isbm any more reviews? Any thing that I can do?

isbm

@aplanas nope, good to go. 😉

salt/modules/zypper.py

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.

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.

_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.

Add no_recommends parameter to install and upgrade actions.

aplanas · 2018-11-15T10:07:33Z

This PR is reaching the 1 month old mark. If there anything that I can do the help here?

The code do not change the default behavior, as I did not change the semantic of the tests.

rallytime · 2018-11-15T14:50:22Z

@terminalmage and @meaksh How does this look to you now?

aplanas · 2018-11-21T08:21:49Z

thanks a lot for the reviews!

* Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) saltstack/salt#50125 * Add root parameter to useradd, shadow and groupadd saltstack/salt#50175 * cmd: Add root parameter for wait and run states saltstack/salt#50302 * systemd: add optional root parameter saltstack/salt#50380 * Add new chroot module https://github.com/openSUSE/salt/pull/50418 * Add new module freezer saltstack/salt#50452 * btrfs: add all subvolume commands saltstack/salt#50541 * btrfs: add new btrfs state saltstack/salt#50635 * zypper: demote log from error to warning saltstack/salt#50671 * blkid: add search by token saltstack/salt#50706 * mount: add fstab_{present,absent} states saltstack/salt#50725 * btrfs: add option to not set subvolumes as default https://github.com/openSUSE/salt/pull/50801 * Add disk_set and disk_toggle functions, and update valid partition flags https://github.com/openSUSE/salt/pull/50834 * disk: support setting FAT size for format_ https://github.com/openSUSE/salt/pull/51074 * parted: fix set_ valid flags comment. https://github.com/openSUSE/salt/pull/51704 * grains/core: ignore HOST_NOT_FOUND errno in fqdns() https://github.com/openSUSE/salt/pull/51706 * cmdmod: add 'binds' parameter in run_chroot https://github.com/openSUSE/salt/pull/51871 * mount: fix extra -t parameter https://github.com/openSUSE/salt/pull/51905 * lvm: be quiet when a pv, lv or vg is not expected https://github.com/openSUSE/salt/pull/51929 * linux_lvm: clean error in pvcreate and pvremove https://github.com/openSUSE/salt/pull/51954 * blockdev: hide blkid errors when are expected https://github.com/openSUSE/salt/pull/51956 * partially unify public functions signature for pkg and lowpkg https://github.com/openSUSE/salt/pull/51973 * extmods: add utils directories in sys.path https://github.com/openSUSE/salt/pull/52001

* Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) saltstack/salt#50125 * Add root parameter to useradd, shadow and groupadd saltstack/salt#50175 * cmd: Add root parameter for wait and run states saltstack/salt#50302 * systemd: add optional root parameter saltstack/salt#50380 * Add new chroot module https://github.com/openSUSE/salt/pull/50418 * Add new module freezer saltstack/salt#50452 * btrfs: add all subvolume commands saltstack/salt#50541 * btrfs: add new btrfs state saltstack/salt#50635 * zypper: demote log from error to warning saltstack/salt#50671 * blkid: add search by token saltstack/salt#50706 * mount: add fstab_{present,absent} states saltstack/salt#50725 * btrfs: add option to not set subvolumes as default saltstack/salt#50801 * Add disk_set and disk_toggle functions, and update valid partition flags saltstack/salt#50834 * disk: support setting FAT size for format_ saltstack/salt#51074 * parted: fix set_ valid flags comment. saltstack/salt#51704 * grains/core: ignore HOST_NOT_FOUND errno in fqdns() saltstack/salt#51706 * cmdmod: add 'binds' parameter in run_chroot saltstack/salt#51871 * mount: fix extra -t parameter saltstack/salt#51905 * lvm: be quiet when a pv, lv or vg is not expected saltstack/salt#51929 * linux_lvm: clean error in pvcreate and pvremove saltstack/salt#51954 * blockdev: hide blkid errors when are expected saltstack/salt#51956 * partially unify public functions signature for pkg and lowpkg saltstack/salt#51973 * extmods: add utils directories in sys.path saltstack/salt#52001

ghost self-requested a review October 19, 2018 08:55

aplanas changed the title ~~Fix zypper~~ Add root parameter in the public interface for Zypper and RPM (lowpkg) Oct 19, 2018

aplanas force-pushed the fix_zypper branch 2 times, most recently from dd171d3 to 4b1ba30 Compare October 19, 2018 13:23

isbm suggested changes Oct 21, 2018

View reviewed changes

salt/modules/rpm.py Outdated Show resolved Hide resolved

salt/modules/zypper.py Outdated Show resolved Hide resolved

salt/modules/zypper.py Outdated Show resolved Hide resolved

salt/modules/zypper.py Outdated Show resolved Hide resolved

aplanas force-pushed the fix_zypper branch 2 times, most recently from d5528a0 to e9e2320 Compare October 22, 2018 08:32

isbm suggested changes Oct 22, 2018

View reviewed changes

salt/modules/zypper.py Outdated Show resolved Hide resolved

aplanas force-pushed the fix_zypper branch 2 times, most recently from c0905e9 to a053f9c Compare October 23, 2018 09:16

aplanas changed the title ~~Add root parameter in the public interface for Zypper and RPM (lowpkg)~~ Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) Oct 23, 2018

rallytime requested a review from isbm October 26, 2018 14:02

aplanas force-pushed the fix_zypper branch from e06c284 to 49dc49e Compare October 30, 2018 10:49

aplanas force-pushed the fix_zypper branch 5 times, most recently from ae8895f to 54d46b4 Compare October 31, 2018 09:42

isbm suggested changes Oct 31, 2018

View reviewed changes

salt/modules/rpm.py Show resolved Hide resolved

salt/modules/rpm.py Outdated Show resolved Hide resolved

salt/modules/rpm.py Outdated Show resolved Hide resolved

salt/modules/rpm.py Outdated Show resolved Hide resolved

aplanas force-pushed the fix_zypper branch from 54d46b4 to 0a78ad9 Compare October 31, 2018 19:17

aplanas force-pushed the fix_zypper branch 3 times, most recently from cb8b670 to 6722b49 Compare November 5, 2018 12:02

rallytime requested a review from isbm November 5, 2018 14:40

isbm suggested changes Nov 6, 2018

View reviewed changes

isbm approved these changes Nov 9, 2018

View reviewed changes

rallytime requested a review from terminalmage November 9, 2018 14:36

isbm approved these changes Nov 14, 2018

View reviewed changes

meaksh reviewed Nov 14, 2018

View reviewed changes

salt/modules/zypper.py Outdated Show resolved Hide resolved

aplanas force-pushed the fix_zypper branch from ddb1807 to e65ba2c Compare November 14, 2018 16:49

aplanas added 9 commits November 15, 2018 11:03

zypper: add root configuration parameter

199f337

Fix typo in comment

6aae270

pkg: Transfer optional parameters to lower levels.

7430da8

pkgrepo: Transfer optional parameters to lower levels.

ecdfb89

zypper: add no_recommends parameter

c2ace99

Add no_recommends parameter to install and upgrade actions.

zypper: fix pylint complain

05db3cb

aplanas force-pushed the fix_zypper branch from e65ba2c to 05db3cb Compare November 15, 2018 10:03

terminalmage approved these changes Nov 16, 2018

View reviewed changes

cachedout merged commit ccafe79 into saltstack:develop Nov 20, 2018

aplanas deleted the fix_zypper branch November 21, 2018 08:21

aplanas mentioned this pull request Jan 30, 2019

Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) #51409

Closed

aplanas mentioned this pull request Mar 6, 2019

Backport from develop features required by yomi openSUSE/salt#120

Closed

aplanas mentioned this pull request Oct 11, 2019

(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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) #50125

Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) #50125

aplanas commented Oct 19, 2018

aplanas commented Oct 19, 2018

aplanas commented Oct 23, 2018

rallytime commented Oct 30, 2018

aplanas commented Oct 30, 2018

isbm left a comment

aplanas commented Oct 31, 2018

isbm Nov 6, 2018

aplanas Nov 6, 2018

isbm Nov 9, 2018

aplanas commented Nov 9, 2018

isbm left a comment

aplanas commented Nov 15, 2018

rallytime commented Nov 15, 2018

aplanas commented Nov 21, 2018

Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) #50125

Add root and no_recommends parameters in the public interface for Zypper and RPM (lowpkg) #50125

Conversation

aplanas commented Oct 19, 2018

What does this PR do?

What issues does this PR fix or reference?

Tests written?

aplanas commented Oct 19, 2018

aplanas commented Oct 23, 2018

rallytime commented Oct 30, 2018

aplanas commented Oct 30, 2018

isbm left a comment

Choose a reason for hiding this comment

aplanas commented Oct 31, 2018

isbm Nov 6, 2018

Choose a reason for hiding this comment

aplanas Nov 6, 2018

Choose a reason for hiding this comment

isbm Nov 9, 2018

Choose a reason for hiding this comment

aplanas commented Nov 9, 2018

isbm left a comment

Choose a reason for hiding this comment

aplanas commented Nov 15, 2018

rallytime commented Nov 15, 2018

aplanas commented Nov 21, 2018