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: let container_execute to support rpm_format of installed_rpms #3559

Merged
merged 0 commits into from
Oct 24, 2022

Conversation

xiangce
Copy link
Contributor

@xiangce xiangce commented Oct 19, 2022

All Pull Requests:

Check all that apply:

  • Have you followed the guidelines in our Contributing document, including the instructions about commit messages?
  • Is this PR to correct an issue?
  • Is this PR an enhancement?

Complete Description of Additions/Changes:

  • fix the following issue:
    The rpm_format of installed_rpms includes % characters, which caused the collect_execute cannot collect it from the container correctly.
    the_cmd = ("/usr/bin/%s exec %s " + self.cmd) % e

    This patch fixes this issue with:
    the_cmd = "/usr/bin/%s exec %s %s" % (engine, cid, cmd)
  • and update the insights/util/autology/datasources.py correspondingly

@xiangce xiangce marked this pull request as ready for review October 19, 2022 04:04
@xiangce xiangce changed the title feat: make container_execute to support rpm_format of installed_rpms fix: make container_execute to support rpm_format of installed_rpms Oct 19, 2022
@xiangce xiangce changed the title fix: make container_execute to support rpm_format of installed_rpms fix: let container_execute to support rpm_format of installed_rpms Oct 19, 2022
@xiangce xiangce marked this pull request as draft October 19, 2022 09:58
@xiangce xiangce force-pushed the enhance_container_specs branch from 3d1a343 to 140e570 Compare October 19, 2022 11:27
@xiangce xiangce marked this pull request as ready for review October 19, 2022 11:40
@ryan-blakley
Copy link
Contributor

@xiangce everything looks good, but is there a way to possibly add a test for this?

Copy link
Contributor

@ryan-blakley ryan-blakley left a comment

Choose a reason for hiding this comment

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

Disregard my last comment, I just realized adding a test wouldn't be easy at all. Everything looks good to me.

@xiangce
Copy link
Contributor Author

xiangce commented Oct 24, 2022

@ryan-blakley - Thank you very much for reviewing. But you remind me that I forgot to add tests for these specs that I recently added. I raised PR #3563 to add the tests for them. Please have a look too.

@xiangce xiangce merged commit 3b812c9 into master Oct 24, 2022
@xiangce xiangce deleted the enhance_container_specs branch October 24, 2022 08:31
xiangce added a commit that referenced this pull request Oct 24, 2022
…3559)

* feat: make container_execute to support rpm_format of installed_rpms

- and update the insights/util/autology/datasources.py correspondingly

Signed-off-by: Xiangce Liu <[email protected]>

* only one path can be passed once

Signed-off-by: Xiangce Liu <[email protected]>

* update the logic more clear

Signed-off-by: Xiangce Liu <[email protected]>
(cherry picked from commit 3b812c9)
xiangce added a commit that referenced this pull request Sep 6, 2024
…3559)

* feat: make container_execute to support rpm_format of installed_rpms

- and update the insights/util/autology/datasources.py correspondingly

Signed-off-by: Xiangce Liu <[email protected]>

* only one path can be passed once

Signed-off-by: Xiangce Liu <[email protected]>

* update the logic more clear

Signed-off-by: Xiangce Liu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants