OCPBUGS-77078: Add MTU override for OVN-Kubernetes on hosted clusters#7942
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
openshift-ci-robot
added
jira/severity-important
|
@jparrill: This pull request references Jira Issue OCPBUGS-77078, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
No GitHub users were found matching the public email listed for the QA contact in Jira (yli2@redhat.com), skipping review request. 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. |
openshift-ci-robot
added
the
jira/valid-bug
openshift-ci
Bot
added
do-not-merge/needs-area
area/api
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Review skipped — only excluded labels are configured. (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new public field Sequence DiagramsequenceDiagram
actor User
participant CLI as Create CLI
participant Validator as Create Validator
participant API as HostedCluster API
participant Reconciler as Network Reconciler
participant OVN as OVN Network Operator
User->>CLI: run create with --ovn-kubernetes-mtu=<value>
CLI->>Validator: validate OVNKubernetesMTU with network-type and range
Validator-->>CLI: valid / invalid
alt valid (network-type == OVNKubernetes)
CLI->>API: create HostedCluster with OVNKubernetesConfig.MTU (int32)
API->>Reconciler: notify/reconcile HostedCluster
Reconciler->>OVN: set OVNKubernetes IPv4 MTU (convert int32 to *uint32)
OVN-->>Reconciler: confirm MTU applied
else invalid
Validator-->>CLI: return validation error
end
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
area/control-plane-operator
|
/label tide/merge-method-squash |
openshift-ci
Bot
added
the
tide/merge-method-squash
|
/auto-cc |
|
/test unit |
|
/test verify |
|
@jparrill: This pull request references Jira Issue OCPBUGS-77078, which is valid. 3 validation(s) were run on this bug
No GitHub users were found matching the public email listed for the QA contact in Jira (yli2@redhat.com), skipping review request. 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. |
|
Thanks! two small comments, lgtm otherwise |
openshift-ci
Bot
added
the
area/platform/aws
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/cluster/aws/create_test.go (1)
220-232: Consider adding explicit--network-type=OVNKubernetesfor clarity.The test relies on the default
NetworkTypebeingOVNKubernetes. While validation will pass, explicitly including the flag would:
- Match the pattern used in core validation tests
- Make the test intent clearer
- Protect against future changes to the default network type
💡 Suggested improvement
{ name: "minimal with OVNKubernetesMTU", args: []string{ "--name=example", "--sts-creds=" + credentialsFile, "--infra-json=" + infraFile, "--iam-json=" + iamFile, "--role-arn=fakeRoleARN", "--pull-secret=" + pullSecretFile, "--render-sensitive", + "--network-type=OVNKubernetes", "--ovn-kubernetes-mtu=1400", }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/cluster/aws/create_test.go` around lines 220 - 232, The test case named "minimal with OVNKubernetesMTU" relies on the default NetworkType; update its args slice to include the explicit flag "--network-type=OVNKubernetes" so the test intent is obvious and resilient to default changes — locate the test case with name "minimal with OVNKubernetesMTU" in create_test.go and add the "--network-type=OVNKubernetes" entry alongside the existing args (e.g., near "--ovn-kubernetes-mtu=1400").
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmd/cluster/aws/create_test.go`:
- Around line 220-232: The test case named "minimal with OVNKubernetesMTU"
relies on the default NetworkType; update its args slice to include the explicit
flag "--network-type=OVNKubernetes" so the test intent is obvious and resilient
to default changes — locate the test case with name "minimal with
OVNKubernetesMTU" in create_test.go and add the "--network-type=OVNKubernetes"
entry alongside the existing args (e.g., near "--ovn-kubernetes-mtu=1400").
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 663e1202-9aae-4cba-b86b-eaa1a8a78fb6
⛔ Files ignored due to path filters (1)
cmd/cluster/aws/testdata/zz_fixture_TestCreateCluster_minimal_with_OVNKubernetesMTU.yamlis excluded by!**/testdata/**
📒 Files selected for processing (3)
cmd/cluster/aws/create_test.gocmd/cluster/core/create.gocmd/cluster/core/create_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- cmd/cluster/core/create.go
- cmd/cluster/core/create_test.go
|
Let's hold on merging this until the CNO counterpart changes are in place if any is needed https://github.com/openshift/cluster-network-operator/blob/9d540eb7929f83879d3dafbcd1ae6cb1feca36b3/pkg/controller/operconfig/mtu_probe.go#L38-L42 |
|
Rebase due to conflicts |
|
/approve |
openshift-ci
Bot
added
the
approved
JoelSpeed
left a comment
JoelSpeed
left a comment
There was a problem hiding this comment.
/approve
For API review
|
Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enxebre, JoelSpeed, jparrill 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 |
|
/test e2e-aks |
|
/retest-required Non related infra failure |
|
/retest-required |
|
@jparrill: This pull request references Jira Issue OCPBUGS-77078, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: 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. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7942 +/- ##
==========================================
+ Coverage 26.56% 26.57% +0.01%
==========================================
Files 1087 1087
Lines 105041 105066 +25
==========================================
+ Hits 27901 27922 +21
- Misses 74731 74735 +4
Partials 2409 2409 🚀 New features to boost your workflow:
|
|
/verified by @wewang58 |
openshift-ci-robot
added
the
verified
|
@wewang58: 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. |
|
@jparrill: all tests passed! 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. |
|
@jparrill: Jira Issue OCPBUGS-77078: Some pull requests linked via external trackers have merged: The following pull request, linked via external tracker, has not merged: All associated pull requests must be merged or unlinked from the Jira bug in order for it to move to the next state. Once unlinked, request a bug refresh with Jira Issue OCPBUGS-77078 has not been moved to the MODIFIED state. This PR is marked as verified. If the remaining PRs listed above are marked as verified before merging, the issue will automatically be moved to VERIFIED after all of the changes from the PRs are available in an accepted nightly payload. 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. |
|
/jira backport release-4.21 |
|
@jparrill: The following backport issues have been created:
Queuing cherrypicks to the requested branches to be created after this PR merges: 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. |
|
@openshift-ci-robot: #7942 failed to apply on top of branch "release-4.21": 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 kubernetes-sigs/prow repository. |
Backport of PR openshift#7942 to release-4.21. Some non-commercial AWS regions do not support the full 9001 MTU that the cluster-network-operator assumes for AWS. This adds an optional, immutable mtu field to the HyperShift OVNKubernetesConfig API type, allowing users to override the tunnel interface MTU at cluster creation time. The field is propagated through ReconcileNetworkOperator to the operator.openshift.io/v1 Network resource, which the CNO already supports. When unset, the CNO continues to auto-detect the MTU. Also adds --ovn-kubernetes-mtu CLI flag and regenerates CRDs/docs. Squashed commits: * feat(api): add MTU field to OVNKubernetesConfig for hosted clusters * chore(generated): regenerate CRDs, clients, vendor, and docs * feat(cli): add --ovn-kubernetes-mtu flag to hypershift create cluster * chore(generated): add TestCreateCluster fixture for OVNKubernetesMTU Ref: https://issues.redhat.com/browse/OCPBUGS-77078 Ref: https://issues.redhat.com/browse/OCPBUGS-81489 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
Backport of PR openshift#7942 to release-4.21. Some non-commercial AWS regions do not support the full 9001 MTU that the cluster-network-operator assumes for AWS. This adds an optional, immutable mtu field to the HyperShift OVNKubernetesConfig API type, allowing users to override the tunnel interface MTU at cluster creation time. The field is propagated through ReconcileNetworkOperator to the operator.openshift.io/v1 Network resource, which the CNO already supports. When unset, the CNO continues to auto-detect the MTU. Also adds --ovn-kubernetes-mtu CLI flag and regenerates CRDs/docs. Squashed commits: * feat(api): add MTU field to OVNKubernetesConfig for hosted clusters * chore(generated): regenerate CRDs, clients, vendor, and docs * feat(cli): add --ovn-kubernetes-mtu flag to hypershift create cluster * chore(generated): add TestCreateCluster fixture for OVNKubernetesMTU Ref: https://issues.redhat.com/browse/OCPBUGS-77078 Ref: https://issues.redhat.com/browse/OCPBUGS-81489 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
Backport of PR openshift#7942 to release-4.21. Some non-commercial AWS regions do not support the full 9001 MTU that the cluster-network-operator assumes for AWS. This adds an optional, immutable mtu field to the HyperShift OVNKubernetesConfig API type, allowing users to override the tunnel interface MTU at cluster creation time. The field is propagated through ReconcileNetworkOperator to the operator.openshift.io/v1 Network resource, which the CNO already supports. When unset, the CNO continues to auto-detect the MTU. Also adds --ovn-kubernetes-mtu CLI flag and regenerates CRDs/docs. Squashed commits: * feat(api): add MTU field to OVNKubernetesConfig for hosted clusters * chore(generated): regenerate CRDs, clients, vendor, and docs * feat(cli): add --ovn-kubernetes-mtu flag to hypershift create cluster * chore(generated): add TestCreateCluster fixture for OVNKubernetesMTU Ref: https://issues.redhat.com/browse/OCPBUGS-77078 Ref: https://issues.redhat.com/browse/OCPBUGS-81489 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com>
|
/jira refresh |
|
@jparrill: Jira Issue Verification Checks: Jira Issue OCPBUGS-77078 Jira Issue OCPBUGS-77078 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 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. |
…openshift#7942) * feat(api): add MTU field to OVNKubernetesConfig for hosted clusters Some non-commercial AWS regions do not support the full 9001 MTU that the cluster-network-operator assumes for AWS. This adds an optional, immutable mtu field to the HyperShift OVNKubernetesConfig API type, allowing users to override the tunnel interface MTU at cluster creation time. The field is propagated through ReconcileNetworkOperator to the operator.openshift.io/v1 Network resource, which the CNO already supports. When unset, the CNO continues to auto-detect the MTU. Ref: https://issues.redhat.com/browse/OCPBUGS-77078 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com> * chore(generated): regenerate CRDs, clients, vendor, and docs Regenerate after adding the mtu field to OVNKubernetesConfig. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com> * feat(cli): add --ovn-kubernetes-mtu flag to hypershift create cluster Adds a new --ovn-kubernetes-mtu flag that allows users to specify the OVN-Kubernetes tunnel interface MTU when creating a hosted cluster. This is needed for non-commercial AWS regions that do not support the full 9001 MTU that the cluster-network-operator assumes. The flag is only valid when --network-type is OVNKubernetes and validation enforces this constraint. Ref: https://issues.redhat.com/browse/OCPBUGS-77078 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com> * chore(generated): add TestCreateCluster fixture for OVNKubernetesMTU Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com> --------- Signed-off-by: Juan Manuel Parrilla Madrid <jparrill@redhat.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
7 participants
What this PR does / why we need it:
Some non-commercial AWS regions (ISO/classified) do not support the full 9001 MTU that the cluster-network-operator assumes for AWS. This PR adds a way to override the OVN-Kubernetes tunnel interface MTU through the HostedCluster API and CLI.
Changes:
API: Adds an optional, immutable
mtufield toOVNKubernetesConfiginspec.operatorConfiguration.clusterNetworkOperator.ovnKubernetesConfig. When set, the value is propagated to theoperator.openshift.io/v1 Networkresource that the CNO reads. When unset, the CNO continues to auto-detect the MTU.Reconciliation: Propagates the MTU value from the HyperShift API to the CNO operator Network resource inside the hosted cluster via
ReconcileNetworkOperator().CLI: Adds
--ovn-kubernetes-mtuflag tohypershift create clusterandhcp create clustercommands. The flag is validated to only work with--network-type=OVNKubernetes.Which issue(s) this PR fixes:
Fixes https://issues.redhat.com/browse/OCPBUGS-77078
Related PR
Special notes for your reviewer:
MTUfield onoperator.openshift.io/v1 Network.spec.defaultNetwork.ovnKubernetesConfig.mtu— no CNO changes are needed.self == oldSelf) because changing tunnel MTU on a running cluster is a destructive operation.Checklist:
🤖 Generated with Claude Code via
/jira:solve OCPBUGS-77078Summary by CodeRabbit
New Features
Tests