opkg: refactor module to use StateModuleHelper and CmdRunner #5718

russoz · 2022-12-21T07:55:25Z

SUMMARY

Refactor the module to use StateModuleHelper and CmdRunner. As a side effect, running opkg is now passing parameters in a safer way.

ISSUE TYPE

Feature Pull Request

COMPONENT NAME

plugins/modules/opkg.py

ansibullbot · 2022-12-21T08:00:28Z

cc @skinp
click here for bot help

felixfontein · 2022-12-22T05:59:41Z

CC @joergho who contributed to this module recently.

felixfontein · 2022-12-22T06:06:13Z

This conflicts with #5688, which is going to be merged before this. Also note that this module has no tests, so this needs to be verified with manual testing whether it works.

russoz · 2022-12-22T07:05:08Z

I am happy to adjust this one after the other PR gets merged.

And I could get unit tests done, in the meantime. Though they will not support the "old way" of calling run_command with a single string for cmd line.

joergho

With the changes included, the opkg module behaves as before (I ran some tests).
However, in the old implementation there was the output installed/removed %d package(s) which is now missing. Assigning self.vars.msg is not possible. Is there another way to do this? Alternatively, I set the install_c and remove_c variables as output, so the the information how many packages got installed / removed is still available as output.

plugins/modules/opkg.py

russoz · 2022-12-22T11:33:17Z

With the changes included, the opkg module behaves as before (I ran some tests). However, in the old implementation there was the output installed/removed %d package(s) which is now missing. Assigning self.vars.msg is not possible. Is there another way to do this? Alternatively, I set the install_c and remove_c variables as output, so the the information how many packages got installed / removed is still available as output.

@joergho thanks for the thorough check (and catching the typo). We can set the msg output same as before. And I will respond other to other reviews on a case by case.

felixfontein · 2022-12-22T18:32:22Z

I merged #5688, this one now has conflicts.

russoz · 2022-12-22T22:40:36Z

I will still create unit tests for this, kinda cookiecutting from previous tests using CmdRunner

plugins/modules/opkg.py

Co-authored-by: joergho <[email protected]>

russoz · 2023-01-09T09:48:34Z

Rebasing the PR

russoz · 2023-01-09T09:52:17Z

before this PR:

TASK [install] ******************
changed: [agent] => {"changed": true, "msg": "installed 1 package(s)"}

after this PR:

TASK [install] ******************
changed: [agent] => {"_msg": "installed 1 package(s)", "changed": true}

If you update to the latest version of c.g. from the main branch, the result will always be in msg.

russoz · 2023-01-10T10:52:43Z

@felixfontein I think everybody is happy with this one now

patchback · 2023-01-12T19:42:41Z

Backport to stable-6: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-6/682bb4b88a00b880756a891f9280083164dc8fb2/pr-5718

Backported as #5824

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

* opkg: refactor module to use StateModuleHelper and CmdRunner * add changelog fragment * Update plugins/modules/opkg.py Co-authored-by: joergho <[email protected]> * Update plugins/modules/opkg.py Co-authored-by: joergho <[email protected]> * Update plugins/modules/opkg.py Co-authored-by: joergho <[email protected]> * Update plugins/modules/opkg.py Co-authored-by: joergho <[email protected]> * Update plugins/modules/opkg.py Co-authored-by: joergho <[email protected]> * Update plugins/modules/opkg.py Co-authored-by: joergho <[email protected]> * Update plugins/modules/opkg.py Co-authored-by: joergho <[email protected]> * generate message outcome as before * aggregated changes from 5688 * fix package query * add unit tests * fix sanity error * Update plugins/modules/opkg.py Co-authored-by: joergho <[email protected]> * add test for specifying version * refactor parameter name Co-authored-by: joergho <[email protected]> (cherry picked from commit 682bb4b)

felixfontein · 2023-01-12T19:42:55Z

@russoz thanks for doing this!
@joergho thanks for testing and reviewing!

…tateModuleHelper and CmdRunner (#5824) opkg: refactor module to use StateModuleHelper and CmdRunner (#5718) * opkg: refactor module to use StateModuleHelper and CmdRunner * add changelog fragment * Update plugins/modules/opkg.py Co-authored-by: joergho <[email protected]> * Update plugins/modules/opkg.py Co-authored-by: joergho <[email protected]> * Update plugins/modules/opkg.py Co-authored-by: joergho <[email protected]> * Update plugins/modules/opkg.py Co-authored-by: joergho <[email protected]> * Update plugins/modules/opkg.py Co-authored-by: joergho <[email protected]> * Update plugins/modules/opkg.py Co-authored-by: joergho <[email protected]> * Update plugins/modules/opkg.py Co-authored-by: joergho <[email protected]> * generate message outcome as before * aggregated changes from 5688 * fix package query * add unit tests * fix sanity error * Update plugins/modules/opkg.py Co-authored-by: joergho <[email protected]> * add test for specifying version * refactor parameter name Co-authored-by: joergho <[email protected]> (cherry picked from commit 682bb4b) Co-authored-by: Alexei Znamensky <[email protected]>

ansibullbot added bug This issue/PR relates to a bug module module os packaging plugins plugin (any type) feature This issue/PR relates to a feature request labels Dec 21, 2022

felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-6 labels Dec 22, 2022

joergho suggested changes Dec 22, 2022

View reviewed changes

ansibullbot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Dec 22, 2022

felixfontein mentioned this pull request Dec 22, 2022

opkg module: allow installing a package in a certain version #5688

Merged

ansibullbot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI labels Dec 22, 2022

russoz force-pushed the opkg-refactor branch from 6351982 to 97fce37 Compare December 22, 2022 22:37

joergho reviewed Dec 23, 2022

View reviewed changes

plugins/modules/opkg.py Outdated Show resolved Hide resolved

russoz and others added 11 commits January 9, 2023 22:16

Update plugins/modules/opkg.py

8270e10

Co-authored-by: joergho <[email protected]>

Update plugins/modules/opkg.py

636ee73

Co-authored-by: joergho <[email protected]>

Update plugins/modules/opkg.py

d1bbd46

Co-authored-by: joergho <[email protected]>

Update plugins/modules/opkg.py

c0de341

Co-authored-by: joergho <[email protected]>

Update plugins/modules/opkg.py

6ce77b3

Co-authored-by: joergho <[email protected]>

generate message outcome as before

62a68e8

aggregated changes from 5688

77c5231

fix package query

035b233

add unit tests

c5e8004

fix sanity error

06e1822

Update plugins/modules/opkg.py

484008d

Co-authored-by: joergho <[email protected]>

joergho approved these changes Jan 9, 2023

View reviewed changes

russoz force-pushed the opkg-refactor branch from ec7368c to 484008d Compare January 9, 2023 09:48

russoz added 2 commits January 10, 2023 00:15

add test for specifying version

2af1d6d

refactor parameter name

f3776f7

joergho mentioned this pull request Jan 10, 2023

opkg: extend docu about compatibilty with OpenWrt vs. Yocto based Linux distribitions #5810

Merged

russoz requested a review from felixfontein January 10, 2023 10:52

felixfontein merged commit 682bb4b into ansible-collections:main Jan 12, 2023

patchback bot mentioned this pull request Jan 12, 2023

[PR #5718/682bb4b8 backport][stable-6] opkg: refactor module to use StateModuleHelper and CmdRunner #5824

Merged

felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Jan 12, 2023

patchback bot mentioned this pull request Jan 12, 2023

[PR #5810/b9ac2dcd backport][stable-6] opkg: extend docu about compatibilty with OpenWrt vs. Yocto based Linux distribitions #5825

Merged

russoz deleted the opkg-refactor branch January 12, 2023 20:36

felixfontein mentioned this pull request Feb 17, 2023

opkg update_cache not working after ansible update #6004

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

opkg: refactor module to use StateModuleHelper and CmdRunner #5718

opkg: refactor module to use StateModuleHelper and CmdRunner #5718

russoz commented Dec 21, 2022 •

edited

Loading

ansibullbot commented Dec 21, 2022

felixfontein commented Dec 22, 2022

felixfontein commented Dec 22, 2022

russoz commented Dec 22, 2022

joergho left a comment

russoz commented Dec 22, 2022

felixfontein commented Dec 22, 2022

russoz commented Dec 22, 2022

russoz commented Jan 9, 2023

russoz commented Jan 9, 2023

russoz commented Jan 10, 2023

patchback bot commented Jan 12, 2023 •

edited

Loading

felixfontein commented Jan 12, 2023

opkg: refactor module to use StateModuleHelper and CmdRunner #5718

opkg: refactor module to use StateModuleHelper and CmdRunner #5718

Conversation

russoz commented Dec 21, 2022 • edited Loading

SUMMARY

ISSUE TYPE

COMPONENT NAME

ansibullbot commented Dec 21, 2022

felixfontein commented Dec 22, 2022

felixfontein commented Dec 22, 2022

russoz commented Dec 22, 2022

joergho left a comment

Choose a reason for hiding this comment

russoz commented Dec 22, 2022

felixfontein commented Dec 22, 2022

russoz commented Dec 22, 2022

russoz commented Jan 9, 2023

russoz commented Jan 9, 2023

russoz commented Jan 10, 2023

patchback bot commented Jan 12, 2023 • edited Loading

Backport to stable-6: 💚 backport PR created

felixfontein commented Jan 12, 2023

russoz commented Dec 21, 2022 •

edited

Loading

patchback bot commented Jan 12, 2023 •

edited

Loading