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

Fix running of Ansible playbooks shipped with tmt #3126

Merged
merged 2 commits into from
Sep 3, 2024

Conversation

falconizmi
Copy link
Collaborator

@falconizmi falconizmi commented Aug 1, 2024

Plugins have to mangle playbook paths to appease a piece of code that
expected paths to always be relative and under the fmf root. This is not
true for playbooks shipped with plugins that are bundled with tmt, like
prepare/feature. Therefore changing the code to support plugins that
may never be under the fmf root.

Pull Request Checklist

  • implement the feature
  • write the documentation
  • extend the test coverage
  • update the specification
  • adjust plugin docstring
  • mention the version
  • include a release note

@falconizmi falconizmi self-assigned this Aug 1, 2024
@falconizmi falconizmi added ansible status | blocking other work An important pull request, blocking other pull requests or issues labels Aug 1, 2024
@falconizmi
Copy link
Collaborator Author

This PR blocks #2985

@psss psss added this to the 1.36 milestone Aug 7, 2024
@thrix thrix removed the status | blocking other work An important pull request, blocking other pull requests or issues label Aug 13, 2024
@falconizmi falconizmi changed the title Draft: Support running ansible using feature outside fmf root Support running ansible using feature outside fmf root Aug 13, 2024
tmt/steps/prepare/feature.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@martinhoyer martinhoyer left a comment

Choose a reason for hiding this comment

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

+1 to @happz's comments

@therazix
Copy link
Collaborator

It looks like this PR also fixes the #3032.

@happz
Copy link
Collaborator

happz commented Aug 30, 2024

It looks like this PR also fixes the #3032.

Hm, yes and no. Yes, I think this PR might fix #3032, because the plugin and tmt will now use a better way when speaking about paths of playbooks that are shipped with tmt. But also no, because the cause is not the one mentioned in #3032, i.e. a missing fmf root - that's actually a second issue. prepare/epel had to perform some black magic mangling of its playbook paths, which did involve fmf root, and this PR removes this magic, therefore fmf root is no longer involved, therefore prepare/feature will work. But not because tmt suddenly learned to run any playbook without having an fmf root - when it comes to user-provided playbooks, like those for prepare/ansible or finish/ansible, they will still fail outside of fmf root. We discussed both issues with @falconizmi and agreed on addressing them both, in separate PRs.

@thrix thrix added the priority | must high priority, must be included in the next release label Aug 30, 2024
@thrix
Copy link
Collaborator

thrix commented Aug 30, 2024

marked as must, users are hitting this in Testing Farm

@happz happz added the ci | full test Pull request is ready for the full test execution label Aug 30, 2024
@happz
Copy link
Collaborator

happz commented Sep 1, 2024

Assuming @falconizmi was not available on such late notice on Friday afternoon, I tried to address the comments above. I think there will be failed tests, I'll check them in the morning.

@happz
Copy link
Collaborator

happz commented Sep 3, 2024

And before the merge, $SUBJ & commit message should be updated - it's not fixing "outside fmf root" issue, it's fixing "feature can't run its own playbooks".

Done.

Ismail Ibrahim Quwarah and others added 2 commits September 3, 2024 11:43
Plugins have to mangle playbook paths to appease a piece of code that
expected paths to always be relative and under the fmf root. This is not
true for playbooks shipped with plugins that are bundled with tmt, like
`prepare/feature`. Therefore changing the code to support plugins that
may never be under the fmf root.
@happz happz force-pushed the iquwarah-tmt-feature-ansible branch from 26d311a to 4296815 Compare September 3, 2024 09:50
@happz happz changed the title Support running ansible using feature outside fmf root Fix running of Ansible playbooks shipped with tmt Sep 3, 2024
@happz happz added the status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish. label Sep 3, 2024
@happz happz merged commit 74c4f5a into main Sep 3, 2024
22 checks passed
@happz happz deleted the iquwarah-tmt-feature-ansible branch September 3, 2024 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ansible ci | full test Pull request is ready for the full test execution priority | must high priority, must be included in the next release status | ready for merge The only missing piece is to do the rebase the current 'main' and let the CI finish.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants