Skip to content

TELCODOCS-2064: Docs and RN: CNF-14082 SR-IOV Operator support for Intel C741 Emmitsburg Chipset #84760

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

Merged
merged 1 commit into from
Feb 12, 2025

Conversation

ogradyp
Copy link
Contributor

@ogradyp ogradyp commented Nov 11, 2024

https://issues.redhat.com/browse/TELCODOCS-2064: Docs and RN: CNF-14082 SR-IOV Operator support for Intel C741 Emmitsburg Chipset

Applies to OCP version : 4.18+

Preview: SR-IOV Network Operator config custom resource

Dev review completed by @cgoncalves
Dev review completed by @SchSeba
Peer review completed by @cbippley

Thank you.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Nov 11, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Nov 11, 2024

@ogradyp: This pull request references TELCODOCS-2064 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

…ameter

Version(s):

Issue:

Link to docs preview:

QE review:

  • QE has approved this change.

Additional information:

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.

@openshift-ci openshift-ci bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Nov 11, 2024
@ocpdocs-previewbot
Copy link

ocpdocs-previewbot commented Nov 11, 2024

@ogradyp ogradyp changed the title TELCODOCS-2064: Added the spec.featureGates.mellanoxFirmwareReset par… Docs and RN: CNF-14082 SR-IOV Operator support for Intel C741 Emmitsburg Chipset Nov 12, 2024
@openshift-ci-robot openshift-ci-robot removed the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Nov 12, 2024
@openshift-ci-robot
Copy link

@ogradyp: No Jira issue is referenced in the title of this pull request.
To reference a jira issue, add 'XYZ-NNN:' to the title of this pull request and request another refresh with /jira refresh.

In response to this:

…ameter

Version(s):

Issue:

Link to docs preview:

QE review:

  • QE has approved this change.

Additional information:

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.

Copy link
Contributor

@cgoncalves cgoncalves left a comment

Choose a reason for hiding this comment

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

Release note missing?

@@ -96,6 +96,10 @@ By default, this field is set to `2`.
|`boolean`
|Specifies whether to enable or disable the SR-IOV Network Operator metrics. By default, this field is set to `false`.

|`spec.featureGates.mellanoxFirmwareReset`
|`boolean`
|Specifies whether to reset the firmware in the SR-IOV Network Operator. By default, this field is set to `true`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Feel free to re-phrase my suggestion:

Suggested change
|Specifies whether to reset the firmware in the SR-IOV Network Operator. By default, this field is set to `true`.
|Specifies whether to reset the firmware on VF changes in the SR-IOV Network Operator. Certain chipsets (e.g., Intel C740 Series) do not completely power off the PCI-E devices, which is required to configure VFs on NVIDIA/Mellanox NICs. By default, this field is set to `false`.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @cgoncalves for your feedback. I have included, and slightly re-phrased, your suggestion.

@@ -96,6 +96,10 @@ By default, this field is set to `2`.
|`boolean`
|Specifies whether to enable or disable the SR-IOV Network Operator metrics. By default, this field is set to `false`.

|`spec.featureGates.mellanoxFirmwareReset`
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a Tech Preview feature. Can we have a TP note here somehow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @cgoncalves for your feedback. I have included the standard Technology Preview text snippet that appears with all Technology Preview features.

@ogradyp
Copy link
Contributor Author

ogradyp commented Nov 13, 2024

Release note missing?

Thanks @cgoncalves for your feedback. As the Release Note will be specific to the 4.18 stream, it will be included in a separate PR.

@cgoncalves
Copy link
Contributor

Thanks @ogradyp for the documentation! LGTM.

Please hold the merge. We're still finalizing the Tech Preview details internally.
/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 13, 2024
Copy link
Contributor

@SchSeba SchSeba left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 14, 2024
@ogradyp
Copy link
Contributor Author

ogradyp commented Nov 14, 2024

Thanks @ogradyp for the documentation! LGTM.

Please hold the merge. We're still finalizing the Tech Preview details internally. /hold

Thanks @cgoncalves for your approval. Sure, I will hold the merge. Please let me know as soon as the Tech Preview details are finalized.

@ogradyp ogradyp changed the title Docs and RN: CNF-14082 SR-IOV Operator support for Intel C741 Emmitsburg Chipset TELCODOCS-2064: Docs and RN: CNF-14082 SR-IOV Operator support for Intel C741 Emmitsburg Chipset Nov 25, 2024
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Nov 25, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Nov 25, 2024

@ogradyp: This pull request references TELCODOCS-2064 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set.

In response to this:

https://issues.redhat.com/browse/TELCODOCS-2064: Docs and RN: CNF-14082 SR-IOV Operator support for Intel C741 Emmitsburg Chipset

Applies to OCP version : 4.18+

Preview: SR-IOV Network Operator config custom resource

Dev review completed by @cgoncalves
Dev review completed by @SchSeba
Peer review completed by @StephenJamesSmith

Thank you.

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.

@cgoncalves
Copy link
Contributor

/unhold

Good to go from Engineering. Thank you for the patience in waiting this long.

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 7, 2025
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Feb 11, 2025
Copy link

openshift-ci bot commented Feb 11, 2025

New changes are detected. LGTM label has been removed.

@ogradyp
Copy link
Contributor Author

ogradyp commented Feb 11, 2025

/unhold

Good to go from Engineering. Thank you for the patience in waiting this long.

Thanks @cgoncalves for your update. I will send it for review and merging.

@ogradyp
Copy link
Contributor Author

ogradyp commented Feb 11, 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 Feb 11, 2025
@cbippley
Copy link
Contributor

/label peer-review-in-progress
/remove-label peer-review-needed

@openshift-ci openshift-ci bot 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 Feb 11, 2025
Copy link
Contributor

@cbippley cbippley left a comment

Choose a reason for hiding this comment

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

One nit, LGTM!

@@ -96,6 +96,13 @@ By default, this field is set to `2`.
|`boolean`
|Specifies whether to enable or disable the SR-IOV Network Operator metrics. By default, this field is set to `false`.

|`spec.featureGates.mellanoxFirmwareReset`
|`boolean`
|Specifies whether to reset the firmware on virtual function (VF) changes in the SR-IOV Network Operator. Some chipsets (for example, Intel C740 Series) do not completely power off the PCI-E devices, which is required to configure VFs on NVIDIA/Mellanox NICs. By default, this field is set to `false`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
|Specifies whether to reset the firmware on virtual function (VF) changes in the SR-IOV Network Operator. Some chipsets (for example, Intel C740 Series) do not completely power off the PCI-E devices, which is required to configure VFs on NVIDIA/Mellanox NICs. By default, this field is set to `false`.
|Specifies whether to reset the firmware on virtual function (VF) changes in the SR-IOV Network Operator. Some chipsets, such as the Intel C740 Series, do not completely power off the PCI-E devices, which is required to configure VFs on NVIDIA/Mellanox NICs. By default, this field is set to `false`.

Avoiding parenthesis: https://www.ibm.com/docs/en/ibm-style?topic=punctuation-parentheses
Using "such as" instead of "for example": https://www.ibm.com/docs/en/ibm-style?topic=format-examples

@cbippley
Copy link
Contributor

/label peer-review-done
/remove-label peer-review-in-progress

@openshift-ci openshift-ci bot 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 Feb 11, 2025
…ameter

TELCODOCS-2064: Dev feedback applied

TELCODOCS-2064: Technology Preview snippet text added

TELCODOCS-2064: Peer review feedback applied
Copy link

openshift-ci bot commented Feb 12, 2025

@ogradyp: 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.

@ogradyp
Copy link
Contributor Author

ogradyp commented Feb 12, 2025

One nit, LGTM!

Thanks @cbippley for your review and feedback. I have applied your suggestion.

@ogradyp
Copy link
Contributor Author

ogradyp commented Feb 12, 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 Feb 12, 2025
@openshift-ci-robot
Copy link

openshift-ci-robot commented Feb 12, 2025

@ogradyp: This pull request references TELCODOCS-2064 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.19.0" version, but no target version was set.

In response to this:

https://issues.redhat.com/browse/TELCODOCS-2064: Docs and RN: CNF-14082 SR-IOV Operator support for Intel C741 Emmitsburg Chipset

Applies to OCP version : 4.18+

Preview: SR-IOV Network Operator config custom resource

Dev review completed by @cgoncalves
Dev review completed by @SchSeba
Peer review completed by @cbippley

Thank you.

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.

@xenolinux xenolinux added merge-review-in-progress Signifies that the merge review team is reviewing this PR and removed merge-review-needed Signifies that the merge review team needs to review this PR labels Feb 12, 2025
Copy link
Contributor

@xenolinux xenolinux left a comment

Choose a reason for hiding this comment

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

LGTM Merging

@xenolinux xenolinux added this to the Planned for 4.18 GA milestone Feb 12, 2025
@xenolinux xenolinux merged commit 312dd87 into openshift:main Feb 12, 2025
2 checks passed
@xenolinux
Copy link
Contributor

/cherrypick enterprise-4.18

@xenolinux xenolinux removed the merge-review-in-progress Signifies that the merge review team is reviewing this PR label Feb 12, 2025
@openshift-cherrypick-robot

@xenolinux: new pull request created: #88453

In response to this:

/cherrypick enterprise-4.18

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch/enterprise-4.18 jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. peer-review-done Signifies that the peer review team has reviewed this PR size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants