OCPBUGS-78576: UPSTREAM: 136913: Update the csi-driver-host-path version to 1.17.1#2629
OCPBUGS-78576: UPSTREAM: 136913: Update the csi-driver-host-path version to 1.17.1#2629Phaow wants to merge 1 commit intoopenshift:masterfrom
Conversation
openshift-ci-robot
added
the
backports/validated-commits
openshift-ci-robot
added
jira/severity-important
|
@Phaow: This pull request references Jira Issue OCPBUGS-78576, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
The bug has been updated to refer to the pull request using the external bug tracker. 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. |
|
@Phaow: the contents of this pull request could be automatically validated. The following commits are valid:
Comment |
WalkthroughMultiple CSI hostpath plugin image tags were updated to v1.17.1 in test manifests; the DRA test driver pod image was made unpinned and deployment code was changed to compute and patch the hostpathplugin image dynamically during setup. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
|
Hi @jsafrane , could you help take a look when you get a chance? Thank you!^^ |
|
/assign @jsafrane |
|
/lgtm |
|
/test images |
|
/retest |
|
/retest-required |
1 similar comment
|
/retest-required |
|
/retest |
|
/verified by CI |
openshift-ci-robot
added
the
verified
|
@Phaow: This PR has been marked as verified by 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. |
|
/retest-required |
1 similar comment
|
/retest-required |
|
e2e-aws-ovn-serial-2of2 fails with:
But this PR does not add / change busybox image |
|
Threre are many flaky tests ... , when I check the job history and found fixes are in progress ->
|
|
/retest |
|
/retest |
1 similar comment
|
/retest |
openshift-ci-robot
removed
the
verified
|
New changes are detected. LGTM label has been removed. |
|
@Phaow: the contents of this pull request could be automatically validated. The following commits are valid:
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jsafrane, Phaow The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/retest |
|
@Phaow: the contents of this pull request could be automatically validated. The following commits are valid:
Comment |
|
@Phaow: the contents of this pull request could be automatically validated. The following commits are valid:
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/dra/utils/deploy.go`:
- Around line 482-487: The code uses lexicographic string comparison to pick the
newest hostPath tag; replace that with proper semantic version comparison using
golang.org/x/mod/semver: import "golang.org/x/mod/semver" and change the
condition to treat empty hostPathVersion as older or when
semver.Compare(hostPathVersion, version) < 0 (ensure inputs are valid semver,
e.g. semver.IsValid(hostPathVersion) and semver.IsValid(version) or normalize
with a "v" prefix) so hostPathVersion = version only when hostPathVersion is
empty or semver.Compare(hostPathVersion, version) < 0; keep references to the
hostPathVersion and version variables unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 141ac495-d582-400e-ac1c-dc38cf994d7a
📒 Files selected for processing (2)
test/e2e/dra/utils/deploy.gotest/e2e/testing-manifests/dra/dra-test-driver-proxy.yaml
| // "Dumb" string comparison is good enough for e.g. v1.16.1 < v1.17.0. | ||
| // It seems unlikely that any major/patch will need more than one digit | ||
| // or that version grow beyond 99. | ||
| if hostPathVersion == "" || hostPathVersion < version { | ||
| hostPathVersion = version | ||
| } |
There was a problem hiding this comment.
Don't use lexicographic ordering for the "latest" hostpath tag.
hostPathVersion < version will misorder semantic versions once a component reaches two digits (v1.17.9 sorts above v1.17.10). That means this helper can silently select an older hostpath image even though its job here is to follow the newest storage-csi version.
Suggested fix
- // "Dumb" string comparison is good enough for e.g. v1.16.1 < v1.17.0.
- // It seems unlikely that any major/patch will need more than one digit
- // or that version grow beyond 99.
- if hostPathVersion == "" || hostPathVersion < version {
+ if hostPathVersion == "" || semver.Compare(version, hostPathVersion) > 0 {
hostPathVersion = version
}import "golang.org/x/mod/semver"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/e2e/dra/utils/deploy.go` around lines 482 - 487, The code uses
lexicographic string comparison to pick the newest hostPath tag; replace that
with proper semantic version comparison using golang.org/x/mod/semver: import
"golang.org/x/mod/semver" and change the condition to treat empty
hostPathVersion as older or when semver.Compare(hostPathVersion, version) < 0
(ensure inputs are valid semver, e.g. semver.IsValid(hostPathVersion) and
semver.IsValid(version) or normalize with a "v" prefix) so hostPathVersion =
version only when hostPathVersion is empty or semver.Compare(hostPathVersion,
version) < 0; keep references to the hostPathVersion and version variables
unchanged.
|
@Phaow: This pull request references Jira Issue OCPBUGS-78576, which is valid. 3 validation(s) were run on this bug
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. |
|
/retest |
|
@Phaow: the contents of this pull request could be automatically validated. The following commits are valid:
Comment |
|
@Phaow: This pull request references Jira Issue OCPBUGS-78576, which is valid. 3 validation(s) were run on this bug
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. |
|
/test e2e-aws-ovn-crun |
|
/retest |
|
/test e2e-aws-ovn-techpreview |
|
@Phaow: The following tests 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. |
|
Closing, please open a separate request if this work is still required. |
|
@Phaow: This pull request references Jira Issue OCPBUGS-78576. The bug has been updated to no longer refer to the pull request using the external bug tracker. 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. |
7 participants
What type of PR is this?
/kind bug
What this PR does / why we need it:
Which issue(s) this PR is related to:
Fixes https://redhat.atlassian.net/browse/OCPBUGS-78576.
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: