NO-JIRA: tests: fix variable expansion in replace-rt-kernel#163
NO-JIRA: tests: fix variable expansion in replace-rt-kernel#163dustymabe merged 1 commit intocoreos:mainfrom
Conversation
|
@dustymabe: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Code Review
This pull request correctly fixes a bug in the replace-rt-kernel test script where a bash array was not being fully expanded, resulting in an incomplete command. The change is correct and resolves the issue. I have added one suggestion to improve the code's robustness by quoting the array expansion, which is a shell scripting best practice.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dustymabe, travier The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
The code wasn't expanding the full array so we'd only get the first `--install kernel-rt-core` and not the full: --install kernel-rt-core --install kernel-rt-modules \ --install kernel-rt-modules-extra --install kernel-rt-modules-core
741df7b to
1771120
Compare
|
New changes are detected. LGTM label has been removed. |
|
@dustymabe: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
hmm. this seems to be a little more than just this problem.. this code does fix a bug but the test still fails. I think the real problem is that On CentOS 9 I see it doing more: |
|
Likely the culprit is coreos/rpm-ostree#5510 I put a draft PR on coreos/rpm-ostree#5558 this might need more iterations just trying to see if Opus can one shot the fix. |
|
You can safely just close this PR. The rpm-ostree PR: coreos/rpm-ostree#5558 resolves it without any test modification. |
Yes we can, but as written the code had a bug and this PR fixes it so I'm inclined to get it merged anyway. |
|
I'll go ahead and merge this since it fixes a bug and once we get a new rpm-ostree it should unblock the kernel rt replace test too. |
The code wasn't expanding the full array so we'd only get the first
--install kernel-rt-coreand not the full: