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

pkgutil: add update all, check-mode, squashing and examples #799

Merged
merged 12 commits into from
Sep 30, 2020

Conversation

mavit
Copy link
Contributor

@mavit mavit commented Aug 18, 2020

Taken from ansible/ansible#51651 by @dagwieers, which was taken from ansible/ansible#27866 by @scathatheworm. Let’s have one last attempt to get this merged.

SUMMARY

Original PR #27866 from @scathatheworm

When working with Solaris pkgutil CSW packages, I came across this module
being very basic in functionality, in particular, that I could not use it
to update all CSW packages.

When going into details into the code I also found it did not incorporate
a possibility of doing dry-run from the underlying utility, or supported to
specify multiple packages for operations.

This module probably sees very little use, but it seemed like nice
functionality to add and make it behave a little more like other package
modules.

ISSUE TYPE
* Feature Pull Request
COMPONENT NAME

pkgutil module

ANSIBLE VERSION
ansible 2.3.1.0
  config file = /etc/ansible/ansible.cfg
  configured module search path = Default w/o overrides
  python version = 2.7.5 (default, Aug  2 2016, 04:20:16) [GCC 4.8.5

20150623 (Red Hat 4.8.5-4)]


##### ADDITIONAL INFORMATION

    * Added ability to upgrade all packages:


```yaml
- pkgutil:
    name: '*'
    state: latest
* Added ability to modify state of a list of packages:
- pkgutil:
    name:
    - CSWtop
    - CSWwget
    - CSWlsof
    state: present
* Added ability to have underlying tool perform a dry-run when using

check mode, pkgutil -n

* Added ability to configure force option to force packages to state

determined by repository (downgrade for example)

- pkgutil:
    name: CSWtop
    state: latest
    force: yes
* Added more examples and documentation to show the new functionality

@ansibullbot
Copy link
Collaborator

@mavit This PR contains @ mentions in at least one commit message. Those mentions can cause cascading notifications through GitHub and need to be removed. Please squash or amend your commits to remove the mentions.

click here for bot help

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added affects_2.10 feature This issue/PR relates to a feature request integration tests/integration module module needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR needs_triage new_contributor Help guide this first time contributor os packaging plugins plugin (any type) tests tests labels Aug 18, 2020
mavit added 2 commits August 18, 2020 14:13
Taken from ansible/ansible#51651 by dagwieers, which was taken from ansible/ansible#27866 by scathatheworm.  Let’s have one last attempt to get this merged.

> ##### SUMMARY
>
> Original PR #27866 from scathatheworm
>
> When working with Solaris pkgutil CSW packages, I came across this module being very basic in functionality, in particular, that I could not use it to update all CSW packages.
>
> When going into details into the code I also found it did not incorporate a possibility of doing dry-run from the underlying utility, or supported to specify multiple packages for operations.
>
> This module probably sees very little use, but it seemed like nice functionality to add and make it behave a little more like other package modules.
> ##### ISSUE TYPE
>
>     * Feature Pull Request
>
>
> ##### COMPONENT NAME
>
> pkgutil module
> ##### ANSIBLE VERSION
>
> ```
> ansible 2.3.1.0
>   config file = /etc/ansible/ansible.cfg
>   configured module search path = Default w/o overrides
>   python version = 2.7.5 (default, Aug  2 2016, 04:20:16) [GCC 4.8.5 20150623 (Red Hat 4.8.5-4)]
> ```
>
> ##### ADDITIONAL INFORMATION
>
>     * Added ability to upgrade all packages:
>
>
> ```yaml
> - pkgutil:
>     name: '*'
>     state: latest
> ```
>
>     * Added ability to modify state of a list of packages:
>
>
> ```yaml
> - pkgutil:
>     name:
>     - CSWtop
>     - CSWwget
>     - CSWlsof
>     state: present
> ```
>
>     * Added ability to have underlying tool perform a dry-run when using check mode, pkgutil -n
>
>     * Added ability to configure force option to force packages to state determined by repository (downgrade for example)
>
>
> ```yaml
> - pkgutil:
>     name: CSWtop
>     state: latest
>     force: yes
> ```
>
>     * Added more examples and documentation to show the new functionality
@mavit mavit force-pushed the pkgutil-check-mode-etc branch from 09b54af to 51446f1 Compare August 18, 2020 13:13
@ansibullbot ansibullbot added community_review and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Aug 18, 2020
@ansibullbot
Copy link
Collaborator

The test ansible-test sanity --test ansible-doc [explain] failed with the error:

Output on stderr from ansible-doc is considered an error.

Command "ansible-doc -t module community.general.pkgutil" returned exit status 0.
>>> Standard Error
[WARNING]: While constructing a mapping from
/root/ansible/ansible_collections/community/general/plugins/modules/pkgutil.py,
line 16, column 5, found a duplicate dict key (type). Using last defined value
only.

The test ansible-test sanity --test validate-modules [explain] failed with 2 errors:

plugins/modules/packaging/os/pkgutil.py:0:0: doc-type-does-not-match-spec: Argument 'name' in argument_spec defines type as 'list' but documentation defines type as 'str'
plugins/modules/packaging/os/pkgutil.py:0:0: undocumented-parameter: Argument 'pkg' is listed in the argument_spec, but not documented in the module documentation

The test ansible-test sanity --test yamllint [explain] failed with 1 error:

plugins/modules/packaging/os/pkgutil.py:34:5: key-duplicates: DOCUMENTATION: duplication of key "type" in mapping

The test ansible-test sanity --test ansible-doc [explain] failed with the error:

Output on stderr from ansible-doc is considered an error.

Command "ansible-doc -t module community.general.pkgutil" returned exit status 0.
>>> Standard Error
[WARNING]: While constructing a mapping from
/root/ansible_collections/community/general/plugins/modules/pkgutil.py, line
16, column 5, found a duplicate dict key (type). Using last defined value only.

The test ansible-test sanity --test validate-modules [explain] failed with 4 errors:

plugins/modules/packaging/os/pkgutil.py:0:0: doc-type-does-not-match-spec: Argument 'name' in argument_spec defines type as 'list' but documentation defines type as 'str'
plugins/modules/packaging/os/pkgutil.py:0:0: module-invalid-version-added: DOCUMENTATION: version_added ('1.3') is not a valid collection version (see specification at https://semver.org/): invalid semantic version '1.3'. Got {'module': 'pkgutil', 'short_description': 'Manage CSW-Packages on Solaris', 'description': ['Manages CSW packages (SVR4 format) on Solaris 10 and 11.', 'These were the native packages on Solaris <= 10 and are available as a legacy feature in Solaris 11.', 'Pkgutil is an advanced packaging system, which resolves dependency on installation. It is designed for CSW packages.'], 'version_added': '1.3', 'author': ['Alexander Winkler (@dermute)', 'David Ponessa (@scathatheworm)'], 'options': {'name...
plugins/modules/packaging/os/pkgutil.py:0:0: parameter-list-no-elements: Argument 'name' in argument_spec defines type as list but elements is not defined
plugins/modules/packaging/os/pkgutil.py:0:0: undocumented-parameter: Argument 'pkg' is listed in the argument_spec, but not documented in the module documentation

The test ansible-test sanity --test yamllint [explain] failed with 1 error:

plugins/modules/packaging/os/pkgutil.py:34:5: key-duplicates: DOCUMENTATION: duplication of key "type" in mapping

The test ansible-test sanity --test ansible-doc [explain] failed with the error:

Output on stderr from ansible-doc is considered an error.

Command "ansible-doc -t module community.general.pkgutil" returned exit status 0.
>>> Standard Error
[WARNING]: While constructing a mapping from
/root/ansible_collections/community/general/plugins/modules/pkgutil.py, line
16, column 5, found a duplicate dict key (type). Using last defined value only.

The test ansible-test sanity --test validate-modules [explain] failed with 4 errors:

plugins/modules/packaging/os/pkgutil.py:0:0: doc-type-does-not-match-spec: Argument 'name' in argument_spec defines type as 'list' but documentation defines type as 'str'
plugins/modules/packaging/os/pkgutil.py:0:0: module-invalid-version-added: DOCUMENTATION: version_added ('1.3') is not a valid collection version (see specification at https://semver.org/): invalid semantic version '1.3'. Got {'module': 'pkgutil', 'short_description': 'Manage CSW-Packages on Solaris', 'description': ['Manages CSW packages (SVR4 format) on Solaris 10 and 11.', 'These were the native packages on Solaris <= 10 and are available as a legacy feature in Solaris 11.', 'Pkgutil is an advanced packaging system, which resolves dependency on installation. It is designed for CSW packages.'], 'version_added': '1.3', 'author': ['Alexander Winkler (@dermute)', 'David Ponessa (@scathatheworm)'], 'options': {'name...
plugins/modules/packaging/os/pkgutil.py:0:0: parameter-list-no-elements: Argument 'name' in argument_spec defines type as list but elements is not defined
plugins/modules/packaging/os/pkgutil.py:0:0: undocumented-parameter: Argument 'pkg' is listed in the argument_spec, but not documented in the module documentation

The test ansible-test sanity --test yamllint [explain] failed with 1 error:

plugins/modules/packaging/os/pkgutil.py:34:5: key-duplicates: DOCUMENTATION: duplication of key "type" in mapping

click here for bot help

changelogs/fragments/pkgutil-check-mode-etc.yaml Outdated Show resolved Hide resolved
plugins/modules/packaging/os/pkgutil.py Outdated Show resolved Hide resolved
plugins/modules/packaging/os/pkgutil.py Outdated Show resolved Hide resolved
plugins/modules/packaging/os/pkgutil.py Outdated Show resolved Hide resolved
plugins/modules/packaging/os/pkgutil.py Outdated Show resolved Hide resolved
plugins/modules/packaging/os/pkgutil.py Outdated Show resolved Hide resolved
plugins/modules/packaging/os/pkgutil.py Outdated Show resolved Hide resolved
plugins/modules/packaging/os/pkgutil.py Show resolved Hide resolved
@ansibullbot ansibullbot added stale_ci CI is older than 7 days, rerun before merging and removed new_contributor Help guide this first time contributor labels Aug 27, 2020
plugins/modules/packaging/os/pkgutil.py Show resolved Hide resolved
plugins/modules/packaging/os/pkgutil.py Outdated Show resolved Hide resolved
plugins/modules/packaging/os/pkgutil.py Show resolved Hide resolved
plugins/modules/packaging/os/pkgutil.py Show resolved Hide resolved
)
name = module.params['name']
state = module.params['state']
site = module.params['site']
update_catalog = module.params['update_catalog']
force = module.params['force']

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe do a check for '*' in name and name != ['*'], and fail in that case. I don't think pkgutil likes to get * passed (without knowing it).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It says, Package * not in catalog. Exiting., returning 1, which I don't think is any worse than detecting this ourselves.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Except if in the future pkgutil supports * :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The most-recent commit to pkgutil was in 2014, so don't hold your breath waiting for this (or any) new feature!

@ansibullbot ansibullbot removed the stale_ci CI is older than 7 days, rerun before merging label Sep 18, 2020
@felixfontein
Copy link
Collaborator

@dermute @dagwieers @scathatheworm could you please take a look at this PR? I'd like to have another opinion of someone actually using this module :)

Co-authored-by: Felix Fontein <felix@fontein.de>
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-1 labels Sep 28, 2020
@felixfontein
Copy link
Collaborator

If nobody complains until tomorrow morning, I'll merge and backport this so it gets included in community.general 1.2.0.

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Sep 30, 2020
@felixfontein felixfontein merged commit dd9e999 into ansible-collections:main Sep 30, 2020
patchback bot pushed a commit that referenced this pull request Sep 30, 2020
* pkgutil: add update all, check-mode, squashing and examples

Taken from ansible/ansible#51651 by dagwieers, which was taken from ansible/ansible#27866 by scathatheworm.  Let’s have one last attempt to get this merged.

> ##### SUMMARY
>
> Original PR #27866 from scathatheworm
>
> When working with Solaris pkgutil CSW packages, I came across this module being very basic in functionality, in particular, that I could not use it to update all CSW packages.
>
> When going into details into the code I also found it did not incorporate a possibility of doing dry-run from the underlying utility, or supported to specify multiple packages for operations.
>
> This module probably sees very little use, but it seemed like nice functionality to add and make it behave a little more like other package modules.
> ##### ISSUE TYPE
>
>     * Feature Pull Request
>
>
> ##### COMPONENT NAME
>
> pkgutil module
> ##### ANSIBLE VERSION
>
> ```
> ansible 2.3.1.0
>   config file = /etc/ansible/ansible.cfg
>   configured module search path = Default w/o overrides
>   python version = 2.7.5 (default, Aug  2 2016, 04:20:16) [GCC 4.8.5 20150623 (Red Hat 4.8.5-4)]
> ```
>
> ##### ADDITIONAL INFORMATION
>
>     * Added ability to upgrade all packages:
>
>
> ```yaml
> - pkgutil:
>     name: '*'
>     state: latest
> ```
>
>     * Added ability to modify state of a list of packages:
>
>
> ```yaml
> - pkgutil:
>     name:
>     - CSWtop
>     - CSWwget
>     - CSWlsof
>     state: present
> ```
>
>     * Added ability to have underlying tool perform a dry-run when using check mode, pkgutil -n
>
>     * Added ability to configure force option to force packages to state determined by repository (downgrade for example)
>
>
> ```yaml
> - pkgutil:
>     name: CSWtop
>     state: latest
>     force: yes
> ```
>
>     * Added more examples and documentation to show the new functionality

* Add changelog fragment.

* Observe changelog style guide

https://docs.ansible.com/ansible/devel/community/development_process.html#changelogs

Co-authored-by: Felix Fontein <felix@fontein.de>

* Since module split, version_added no-longer refers to core Ansbile

Co-authored-by: Felix Fontein <felix@fontein.de>

* Tweak documentation

* Apply the new `elements` feature for specifying list types

Co-authored-by: Felix Fontein <felix@fontein.de>

* Set version_added

Co-authored-by: Felix Fontein <felix@fontein.de>

* Document `pkg` alias for `name`

* Be explicit about the purpose of states `installed` and `removed`.

* Force the user to specify their desired state.

* Review documentation for pkgutil module.

* Fully qualify svr4pkg module name

Co-authored-by: Felix Fontein <felix@fontein.de>

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit dd9e999)
@felixfontein
Copy link
Collaborator

@mavit thanks a lot for reviving this!

felixfontein pushed a commit that referenced this pull request Sep 30, 2020
…1009)

* pkgutil: add update all, check-mode, squashing and examples

Taken from ansible/ansible#51651 by dagwieers, which was taken from ansible/ansible#27866 by scathatheworm.  Let’s have one last attempt to get this merged.

> ##### SUMMARY
>
> Original PR #27866 from scathatheworm
>
> When working with Solaris pkgutil CSW packages, I came across this module being very basic in functionality, in particular, that I could not use it to update all CSW packages.
>
> When going into details into the code I also found it did not incorporate a possibility of doing dry-run from the underlying utility, or supported to specify multiple packages for operations.
>
> This module probably sees very little use, but it seemed like nice functionality to add and make it behave a little more like other package modules.
> ##### ISSUE TYPE
>
>     * Feature Pull Request
>
>
> ##### COMPONENT NAME
>
> pkgutil module
> ##### ANSIBLE VERSION
>
> ```
> ansible 2.3.1.0
>   config file = /etc/ansible/ansible.cfg
>   configured module search path = Default w/o overrides
>   python version = 2.7.5 (default, Aug  2 2016, 04:20:16) [GCC 4.8.5 20150623 (Red Hat 4.8.5-4)]
> ```
>
> ##### ADDITIONAL INFORMATION
>
>     * Added ability to upgrade all packages:
>
>
> ```yaml
> - pkgutil:
>     name: '*'
>     state: latest
> ```
>
>     * Added ability to modify state of a list of packages:
>
>
> ```yaml
> - pkgutil:
>     name:
>     - CSWtop
>     - CSWwget
>     - CSWlsof
>     state: present
> ```
>
>     * Added ability to have underlying tool perform a dry-run when using check mode, pkgutil -n
>
>     * Added ability to configure force option to force packages to state determined by repository (downgrade for example)
>
>
> ```yaml
> - pkgutil:
>     name: CSWtop
>     state: latest
>     force: yes
> ```
>
>     * Added more examples and documentation to show the new functionality

* Add changelog fragment.

* Observe changelog style guide

https://docs.ansible.com/ansible/devel/community/development_process.html#changelogs

Co-authored-by: Felix Fontein <felix@fontein.de>

* Since module split, version_added no-longer refers to core Ansbile

Co-authored-by: Felix Fontein <felix@fontein.de>

* Tweak documentation

* Apply the new `elements` feature for specifying list types

Co-authored-by: Felix Fontein <felix@fontein.de>

* Set version_added

Co-authored-by: Felix Fontein <felix@fontein.de>

* Document `pkg` alias for `name`

* Be explicit about the purpose of states `installed` and `removed`.

* Force the user to specify their desired state.

* Review documentation for pkgutil module.

* Fully qualify svr4pkg module name

Co-authored-by: Felix Fontein <felix@fontein.de>

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit dd9e999)

Co-authored-by: Peter Oliver <git@mavit.org.uk>
@mavit
Copy link
Contributor Author

mavit commented Sep 30, 2020

@felixfontein Thanks for the fast and thorough review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community_review feature This issue/PR relates to a feature request integration tests/integration module module os packaging plugins plugin (any type) tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants