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

bugfix for xfs_quota not properly initializing project quotas #5144

Conversation

MichaelRiss
Copy link

SUMMARY

xfs_quota is not properly initializing xfs project quota groups (#5143), this PR proposes a fix.
Fixes #5143

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

xfs_quota

ADDITIONAL INFORMATION

This is an attempt to re-submit #3350. Hopefully this time all tests succeed.


@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added bug This issue/PR relates to a bug module module new_contributor Help guide this first time contributor plugins plugin (any type) system needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Aug 22, 2022
@MichaelRiss
Copy link
Author

It looks like the tests about idempotency fail. I looked at the code of the module and I haven't seen any code which tries to compare the current state of the target system with the intended state of the target system and then returns result["changed"] = False if the target system is already correctly configured. In this sense the failing tests would not be a regression as the bug is already there. Is there any way to trigger the same tests but with the current main branch?

@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-4 labels Aug 22, 2022
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! I've added a first small comment on the changelog fragment.

@@ -0,0 +1,3 @@
---
bugfixes:
- xfs_quota - the call to "xfs_quota" to set up project quotas was missing the project name and hence failed (https://github.com/ansible-collections/community.general/issues/5143).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- xfs_quota - the call to "xfs_quota" to set up project quotas was missing the project name and hence failed (https://github.com/ansible-collections/community.general/issues/5143).
- xfs_quota - the call to ``xfs_quota`` to set up project quotas was missing the project name and hence failed (https://github.com/ansible-collections/community.general/issues/5143).

Copy link
Author

Choose a reason for hiding this comment

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

Will do. Out of curiosity, why is one quotation mark style preferred over the other one? Do these fragments get parsed later with some script/tool which needs this kind of style?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The strings in these fragments are interpreted as reStructuredText; they end up in CHANGELOG.rst (for exmaple https://github.com/ansible-collections/community.general/blob/stable-5/CHANGELOG.rst).

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the explanation! I read about .rst files but never worked with them so far.

@felixfontein
Copy link
Collaborator

Is there any way to trigger the same tests but with the current main branch?

These tests run (and pass) every night. https://dev.azure.com/ansible/community.general/_build/results?buildId=51843&view=results is such a run.

@MichaelRiss
Copy link
Author

I checked the past runs of test suite and they indeed succeed. I will need some time to understand how and why the tests succeed on the current main branch and why I have problems in my application with it.

@ansibullbot ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label Aug 31, 2022
@MichaelRiss
Copy link
Author

Trying to replicate the test suite, but it seems the xfs_quota integration test cannot be run locally.

@felixfontein
Copy link
Collaborator

In CI the xfs_quota tests are restricted to VMs since you cannot mount a file in a container when the container isn't privileged, which they are not in CI for security reasons.

What you can do is run it locally with --docker-privileged (i.e. something like ansible-test integration --docker centos7 -v --docker-privileged xfs_quota), that might work.

@felixfontein
Copy link
Collaborator

Please note that in #5461 the collection repository was restructured to remove the directory tree in plugins/modules/, and the corresponding tree in tests/unit/plugins/modules/. Your PR modifies files in this directory structure, and unfortunately now has some conflicts, potentially because of this. Please rebase with the current main branch to remove the conflicts.

@ansibullbot ansibullbot added the needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html label Nov 3, 2022
Copy link
Collaborator

@russoz russoz left a comment

Choose a reason for hiding this comment

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

Hi @MichaelRiss, thanks for the contribution! LGTM, but it needs a rebase

@russoz
Copy link
Collaborator

russoz commented Apr 14, 2023

By the way, instructions to rebasing: https://docs.ansible.com/ansible/latest/dev_guide/developing_rebasing.html

@felixfontein
Copy link
Collaborator

See also #6387.

@felixfontein felixfontein added the backport-7 Automatically create a backport for the stable-7 branch label May 10, 2023
@felixfontein
Copy link
Collaborator

needs_info

@ansibullbot ansibullbot added the needs_info This issue requires further information. Please answer any outstanding questions label May 10, 2023
@ansibullbot
Copy link
Collaborator

@MichaelRiss This pullrequest is waiting for your response. Please respond or the pullrequest will be closed.

click here for bot help

@ansibullbot
Copy link
Collaborator

@MichaelRiss You have not responded to information requests in this pullrequest so we will assume it no longer affects you. If you are still interested in this, please create a new pullrequest with the requested information.

click here for bot help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-7 Automatically create a backport for the stable-7 branch bug This issue/PR relates to a bug check-before-release PR will be looked at again shortly before release and merged if possible. module module needs_info This issue requires further information. Please answer any outstanding questions needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR new_contributor Help guide this first time contributor plugins plugin (any type) stale_ci CI is older than 7 days, rerun before merging system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

xfs_quota fails to initialize new project quotas
4 participants