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

mksysb - revamped module + fix bug in backup_dmapi_fs option #3295

Merged
merged 3 commits into from
Mar 14, 2022
Merged

mksysb - revamped module + fix bug in backup_dmapi_fs option #3295

merged 3 commits into from
Mar 14, 2022

Conversation

russoz
Copy link
Collaborator

@russoz russoz commented Aug 29, 2021

SUMMARY

Revamped the module + fixed bug: option should be -A instead of -a.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

plugins/modules/system/mksysb.py

@ansibullbot ansibullbot added affects_2.10 bug This issue/PR relates to a bug community_review module module needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI needs_triage plugins plugin (any type) stale_ci CI is older than 7 days, rerun before merging system labels Aug 29, 2021
@kairoaraujo
Copy link

I will try to borrow a machine to run tests.

@felixfontein felixfontein added backport-3 check-before-release PR will be looked at again shortly before release and merged if possible. labels Aug 29, 2021
@russoz
Copy link
Collaborator Author

russoz commented Aug 30, 2021

@kairoaraujo it would be awesome if we could write some unit and integration tests for this and the other AIX-related modules. We will be more than happy to assist with that, and it will give us more confidence when making improvements in the future. Would you be interested and available for that? TIA

PS: Valeu! ;-)

@ansibullbot
Copy link
Collaborator

@felixfontein
Copy link
Collaborator

@russoz the diff now looks a bit strange, can you please rebase?

@russoz
Copy link
Collaborator Author

russoz commented Aug 30, 2021

@felixfontein done

@felixfontein
Copy link
Collaborator

@kairoaraujo any luck with testing this?

@ansibullbot ansibullbot removed the needs_ci This PR requires CI testing to be performed. Please close and re-open this PR to trigger CI label Sep 13, 2021
@felixfontein
Copy link
Collaborator

@kairoaraujo any update on testing?

@kairoaraujo
Copy link

@kairoaraujo, any update on testing?

Hi @felixfontein, my old P6 does not work anymore; I suspect the power supply. Anyway, I am looking if some colleague can borrow an AIX.

@mator
Copy link
Contributor

mator commented Oct 12, 2021

you could try on my AIX installation, but i need to fix it first as well =)

@felixfontein
Copy link
Collaborator

I don't really want to merge this without someone having tested it first (especially since this module has no tests run in CI). So if anyone could test this, that would be really great! :)

@kairoaraujo
Copy link

@felixfontein @mator

I am not sure if here is the best place to start this topic.

I had a look at what IBM is doing in https://github.com/IBM/ansible-power-aix.
I see many modules are covering what we do (or used to do) here in the Ansible project.

I am not sure how to handle it.
Keep the effort in here or move our efforts to the repository above?

@felixfontein
Copy link
Collaborator

That's a very good question. One problem with that collection is that it's not included in Ansible, so moving things there would mean that content is no longer there in Ansible. Basically our content would need to be deprecated and removed, and users told to install that collection and use the corresponding modules from it.

I'm not sure how many people actually use the AIX modules in this collection, and are interested in working on them (for maintaining them, or adding new features), or even in helping a possible transition.

If anyone is interested in reaching out to the maintainers of that collection to start a discussion with them, feel free to do so :)

@felixfontein
Copy link
Collaborator

I've just remembered this PR. I really hate to let it rot for too long. How about we merge it for community.general 5.0.0?

@felixfontein
Copy link
Collaborator

Ok, if nobody complains, I'll merge this beginning of next week. Let's get this out.

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Mar 14, 2022
@felixfontein felixfontein merged commit 4af7f49 into ansible-collections:main Mar 14, 2022
@patchback
Copy link

patchback bot commented Mar 14, 2022

Backport to stable-4: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-4/4af7f49ac0f4820093617b6ec3e8d1d6b23a5f62/pr-3295

Backported as #4353

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

patchback bot pushed a commit that referenced this pull request Mar 14, 2022
* Revamped module + fix bug in backup_dmapi_fs option

* added changelog fragment

* added copyright line

(cherry picked from commit 4af7f49)
@felixfontein
Copy link
Collaborator

@russoz thanks for your contribution!
@kairoaraujo @mator thanks for looking at this!

felixfontein pushed a commit that referenced this pull request Mar 14, 2022
…4353)

* Revamped module + fix bug in backup_dmapi_fs option

* added changelog fragment

* added copyright line

(cherry picked from commit 4af7f49)

Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>
@russoz russoz deleted the mksysb-revamp branch March 23, 2022 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug cloud community_review module_utils module_utils module module net_tools plugins plugin (any type) stale_ci CI is older than 7 days, rerun before merging system tests tests unit tests/unit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants