Skip to content

[OCPBUGS-56165]IBMZ changes for Multipath support for ABI #93336

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 14, 2025
@lg-rh
Copy link
Author

lg-rh commented May 14, 2025

/ok-to-test

@openshift-ci openshift-ci bot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label May 14, 2025
@lg-rh
Copy link
Author

lg-rh commented May 14, 2025

/ok-to-test

Copy link
Contributor

@skopacz1 skopacz1 left a comment

Choose a reason for hiding this comment

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

Left some suggestions for you to try, hopefully these will fix the issues you're having. Let me know if you have any questions!

Comment on lines +39 to +50
[NOTE]
====
For FCP multipath configurations, provide two disks instead of one.
====
+
.Example
[source,yaml]
----
rd.zfcp=<adapter1>,<wwpn1>,<lun1> \
rd.zfcp=<adapter2>,<wwpn2>,<lun2>
----
Copy link
Contributor

Choose a reason for hiding this comment

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

If you look in the preview of this PR, the addition of this note and code block breaks the formatting/order of callouts that follow this. You may want to try something like wrapping this content in an open delimiter block (see this page for more details), which will sort of put all of this content in a separate "block" of asciidoc text.

Suggested change
[NOTE]
====
For FCP multipath configurations, provide two disks instead of one.
====
+
.Example
[source,yaml]
----
rd.zfcp=<adapter1>,<wwpn1>,<lun1> \
rd.zfcp=<adapter2>,<wwpn2>,<lun2>
----
--
[NOTE]
====
For FCP multipath configurations, provide two disks instead of one.
====
+
.Example
[source,yaml]
----
rd.zfcp=<adapter1>,<wwpn1>,<lun1> \
rd.zfcp=<adapter2>,<wwpn2>,<lun2>
----
--

Comment on lines +44 to +56
[NOTE]
====
For FCP multipath configurations, provide two disks instead of one.
====
+
.Example
[source,yaml]
----
rd.zfcp=<adapter1>,<wwpn1>,<lun1> \
rd.zfcp=<adapter2>,<wwpn2>,<lun2>
----
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to the other comment, you would also want to try wrapping this content in the -- block:

Suggested change
[NOTE]
====
For FCP multipath configurations, provide two disks instead of one.
====
+
.Example
[source,yaml]
----
rd.zfcp=<adapter1>,<wwpn1>,<lun1> \
rd.zfcp=<adapter2>,<wwpn2>,<lun2>
----
[NOTE]
====
For FCP multipath configurations, provide two disks instead of one.
====
+
--
.Example
[source,yaml]
----
rd.zfcp=<adapter1>,<wwpn1>,<lun1> \
rd.zfcp=<adapter2>,<wwpn2>,<lun2>
----
--

Copy link
Author

Choose a reason for hiding this comment

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

Let me try it out these suggestions

Comment on lines 195 to 198
[NOTE]
====
This parameter is mandatory for FCP multipath configurations on {ibm-z-name}.
====
Copy link
Contributor

Choose a reason for hiding this comment

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

Same thing here:

Suggested change
[NOTE]
====
This parameter is mandatory for FCP multipath configurations on {ibm-z-name}.
====
--
[NOTE]
====
This parameter is mandatory for FCP multipath configurations on {ibm-z-name}.
====
--

@lg-rh lg-rh force-pushed the ibmz_abi_4.19 branch 3 times, most recently from 4799f44 to d26b764 Compare May 15, 2025 06:51
Copy link

openshift-ci bot commented May 15, 2025

@lg-rh: all tests passed!

Full PR test history. Your PR dashboard.

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 kubernetes-sigs/prow repository. I understand the commands that are listed here.

@Neeraj8418
Copy link

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 15, 2025
@werled
Copy link

werled commented May 16, 2025

/lgtm

@lg-rh
Copy link
Author

lg-rh commented May 16, 2025

/label peer-review-needed

@openshift-ci openshift-ci bot added the peer-review-needed Signifies that the peer review team needs to review this PR label May 16, 2025
@lpettyjo lpettyjo added peer-review-in-progress Signifies that the peer review team is reviewing this PR and removed peer-review-needed Signifies that the peer review team needs to review this PR labels May 16, 2025
@lpettyjo lpettyjo self-requested a review May 16, 2025 11:11
@lpettyjo lpettyjo added this to the Planned for 4.19 GA milestone May 16, 2025
Copy link
Contributor

@lpettyjo lpettyjo left a comment

Choose a reason for hiding this comment

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

LGTM

@lpettyjo lpettyjo added peer-review-done Signifies that the peer review team has reviewed this PR and removed peer-review-in-progress Signifies that the peer review team is reviewing this PR labels May 16, 2025
@lg-rh
Copy link
Author

lg-rh commented May 16, 2025

/label merge-review-needed

@openshift-ci openshift-ci bot added the merge-review-needed Signifies that the merge review team needs to review this PR label May 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-4.19 lgtm Indicates that a PR is ready to be merged. merge-review-needed Signifies that the merge review team needs to review this PR ok-to-test Indicates a non-member PR verified by an org member that is safe to test. peer-review-done Signifies that the peer review team has reviewed this PR size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants