CORENET-6100: [WIP] Add support for no-overlay mode

[ricky-rav]

No description provided.

[coderabbitai]

Walkthrough

Adds templated no‑overlay and managed BGP options to OVN config, FRR‑k8s OperatorPKI and readiness/bootstrap handling, Network CRD updates, dependency replace directives, MTU validation and gating logic, render signature changes, and accompanying unit tests and manifest tweaks.

Changes

Cohort / File(s)	Summary
OVN Kubernetes templates bindata/network/ovn-kubernetes/managed/004-config.yaml, bindata/network/ovn-kubernetes/self-hosted/004-config.yaml	Add conditional transport rendering (DefaultNetworkTransport), a no-overlay block guarded by NoOverlayEnabled with optional outbound-snat and routing, and a bgp.managed block guarded by NoOverlayManagedEnabled with optional as-number and topology.
FRR‑k8s manifests bindata/network/frr-k8s/003-pki.yaml, bindata/network/frr-k8s/frr-k8s.yaml, bindata/network/frr-k8s/monitor.yaml, bindata/network/frr-k8s/node-status-cleaner.yaml, bindata/network/frr-k8s/webhook.yaml	Add two OperatorPKI resources (webhook, metrics); change secret names and service annotations; inject KUBERNETES_SERVICE_HOST/KUBERNETES_SERVICE_PORT env vars; add toleration; set ServiceMonitor TLS to insecureSkipVerify: true; make ValidatingWebhookConfiguration conditional on CA bundle.
Go module go.mod	Update github.com/openshift/api version and add replace directives mapping github.com/openshift/api and github.com/openshift/client-go to forked module versions.
OVN no‑overlay logic & rendering pkg/network/ovn_kubernetes.go, pkg/network/render.go, pkg/network/render_test.go, pkg/network/ovn_kubernetes_test.go	Populate no‑overlay template fields, map BGP managed settings, add isFeatureGateEnabled helper, implement ValidateMTUForNoOverlay, add FRR CA bundle fetcher and FRR readiness gating (isFRRWebhookReady, shouldSkipOVNKUntilFRRReady), update render callsites/signatures and tests, and standardize error message casing.
Operator config controller pkg/controller/operconfig/operconfig_controller.go	After filling defaults, validate MTU for no‑overlay against host MTU; on invalid MTU mark operator config degraded with InvalidOperatorConfig and return (early exit).
FRR readiness tests pkg/network/frr_readiness_test.go	Add unit tests for FRR webhook readiness and for decision logic to skip OVN until FRR is ready, using fake EndpointSlice fixtures.
Network CRD manifests manifests/0000_70_network_01_networks-*.crd.yaml manifests/0000_70_network_01_networks-Default.crd.yaml, ...-DevPreviewNoUpgrade.crd.yaml, ...-TechPreviewNoUpgrade.crd.yaml, ...-CustomNoUpgrade.crd.yaml, ...-OKD.crd.yaml	Add/replace large auto‑generated Network CRD OpenAPI v3 schemas (cluster‑scoped Network CRD variants). OKD variant adds a metadata label.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

• Pay attention to:
  • MTU validation logic and operator controller early‑exit behavior.
  • Feature‑gate checks and new isFeatureGateEnabled helper.
  • Templating parity for no‑overlay and bgp.managed across OVN config files.
  • FRR bootstrap/readiness: OperatorPKI resources, CA bundle retrieval, and conditional webhook rendering.
  • Updated render function signatures and corresponding test updates.
  • go.mod replace directives to confirm intentional forks/versions.
  • Large auto‑generated CRD manifests: scan for accidental manual edits.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.5.0)

Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions
The command is terminated due to an error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[openshift-ci-robot]

@ricky-rav: This pull request references CORENET-6100 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.21.0" version, but no target version was set.

Details

In 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.

[ricky-rav]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ricky-rav
Once this PR has been reviewed and has the lgtm label, please assign kyrtapz for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

pkg/network/ovn_kubernetes.go (1)

99-99: Consider separating error message capitalization changes.

The error message capitalization changes (converting initial capital letters to lowercase) are stylistic improvements unrelated to the NoOverlay feature. While consistency is valuable, mixing these changes with feature work:

• Makes the diff harder to review
• Increases the risk of merge conflicts
• Complicates git blame and bisecting if issues arise

Consider moving these changes to a separate commit or PR focused solely on error message style consistency.

Also applies to: 998-998, 1038-1038, 1042-1042, 1047-1047, 1051-1051, 1056-1056, 1060-1060, 1066-1066, 1071-1071, 1306-1306, 1311-1311, 1317-1317, 1352-1352, 1381-1381, 1410-1410, 2099-2099

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 41be31e and 0d8bdf2.

⛔ Files ignored due to path filters (31)

• go.sum is excluded by !**/*.sum
• vendor/github.com/openshift/api/config/v1/types_infrastructure.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/config/v1/types_node.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/config/v1/types_scheduling.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yaml is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/console/v1/types.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/console/v1/zz_generated.swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/features.md is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/features/features.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machine/v1beta1/types_awsprovider.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machine/v1beta1/zz_generated.deepcopy.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machine/v1beta1/zz_generated.swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machineconfiguration/v1/types.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machineconfiguration/v1/types_machineconfignode.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machineconfiguration/v1/zz_generated.deepcopy.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machineconfiguration/v1/zz_generated.featuregated-crd-manifests.yaml is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machineconfiguration/v1/zz_generated.swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machineconfiguration/v1alpha1/register.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machineconfiguration/v1alpha1/types_pinnedimageset.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machineconfiguration/v1alpha1/zz_generated.deepcopy.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machineconfiguration/v1alpha1/zz_generated.featuregated-crd-manifests.yaml is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machineconfiguration/v1alpha1/zz_generated.swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/operator/v1/types_ingress.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/operator/v1/types_network.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers.crd.yaml is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_70_network_01_networks.crd.yaml is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/operator/v1/zz_generated.deepcopy.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/operator/v1/zz_generated.featuregated-crd-manifests.yaml is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/operator/v1/zz_generated.swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
• vendor/modules.txt is excluded by !vendor/**, !**/vendor/**

📒 Files selected for processing (4)

• bindata/network/ovn-kubernetes/managed/004-config.yaml (4 hunks)
• bindata/network/ovn-kubernetes/self-hosted/004-config.yaml (2 hunks)
• go.mod (1 hunks)
• pkg/network/ovn_kubernetes.go (9 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

• bindata/network/ovn-kubernetes/managed/004-config.yaml
• go.mod
• bindata/network/ovn-kubernetes/self-hosted/004-config.yaml
• pkg/network/ovn_kubernetes.go

🔇 Additional comments (2)

bindata/network/ovn-kubernetes/self-hosted/004-config.yaml (1)

16-18: LGTM!

The template changes for NoOverlay mode configuration are well-structured. The conditional blocks properly guard the new configuration sections, and the template variables follow existing naming conventions.

Also applies to: 68-87

bindata/network/ovn-kubernetes/managed/004-config.yaml (1)

16-18: LGTM!

The managed configuration changes mirror the self-hosted configuration and maintain consistency across both ConfigMap instances (for node and control plane). The template structure is correct and follows existing patterns.

Also applies to: 58-77, 121-123, 169-188

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Remove fork dependency before merging.

This replace directive points to a personal fork (github.com/ricky-rav/api). Fork dependencies create supply chain risks, break reproducible builds, and should never be merged to production code. Before removing the [WIP] tag, ensure the necessary API changes are merged upstream to github.com/openshift/api and update this dependency to point to the official repository.

🤖 Prompt for AI Agents

In go.mod around line 178, the replace directive points to a personal fork
(github.com/ricky-rav/api) which must be removed before merging; replace that
directive with the official module (github.com/openshift/api) after the upstream
changes are merged, remove the replace line, update the module version to the
appropriate tag/commit from the official repo, then run go mod tidy and re-run
CI/tests to ensure all imports resolve and builds remain reproducible.

[ricky-rav]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 2

♻️ Duplicate comments (1)

go.mod (1)

178-180: Remove fork dependencies before merging.

These replace directives were already flagged in previous reviews. As noted, fork dependencies create supply chain risks and must be removed before this PR is merged.

🧹 Nitpick comments (2)

pkg/network/frr_readiness_test.go (1)

41-61: Consider adding test case for nil Ready condition pointer.

The test covers Ready: ptr.To(false) but not the case where Ready is nil. In the isFRRWebhookReady function (render.go line 72), the code checks endpoint.Conditions.Ready != nil && *endpoint.Conditions.Ready. If Ready is nil, the endpoint would not be counted as ready, which is correct behavior, but having an explicit test case would document this expectation.

{
    name: "endpoint slice with endpoint but Ready is nil",
    endpointSlice: &discoveryv1.EndpointSlice{
        ObjectMeta: metav1.ObjectMeta{
            Name:      frrK8sWebhookService + "-abc",
            Namespace: frrK8sNamespace,
            Labels: map[string]string{
                "kubernetes.io/service-name": frrK8sWebhookService,
            },
        },
        Endpoints: []discoveryv1.Endpoint{
            {
                Addresses:  []string{"10.0.0.1"},
                Conditions: discoveryv1.EndpointConditions{},
            },
        },
    },
    expected: false,
},

pkg/network/ovn_kubernetes_test.go (1)

4484-4889: Tighten no‑overlay rendering tests by using or removing unused expectations

In TestRenderOVNKubernetesNoOverlay, the testcase struct defines several expectation fields (expectNoOverlayEnabled, expectNoOverlayRouting, expectNoOverlaySNAT, expectManagedEnabled, expectManagedTopology, expectManagedASNumber, expectErr) that are never asserted, so the tests only effectively check for presence/absence of transport="no-overlay" and basic object existence. This can give a false sense of coverage and makes the test harder to follow.

Consider either:

• Wiring these fields into concrete assertions (e.g., checking rendered config content, annotations, or FRR/BGP‑related resources), or
• Removing the unused fields and narrowing the test to what it actually validates today.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 0d8bdf2 and c162e54.

⛔ Files ignored due to path filters (110)

• go.sum is excluded by !**/*.sum
• vendor/github.com/openshift/api/AGENTS.md is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/config/v1/register.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/config/v1/types_feature.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/config/v1/types_image_policy.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/config/v1/types_infrastructure.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/config/v1/types_insights.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/config/v1/types_node.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/config/v1/types_scheduling.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/config/v1/zz_generated.deepcopy.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/config/v1/zz_generated.featuregated-crd-manifests.yaml is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/config/v1/zz_generated.swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/config/v1alpha1/types_cluster_monitoring.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/config/v1alpha1/zz_generated.featuregated-crd-manifests.yaml is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/console/v1/types.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/console/v1/zz_generated.swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/features.md is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/features/features.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machine/v1beta1/types_awsprovider.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machine/v1beta1/zz_generated.deepcopy.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machine/v1beta1/zz_generated.swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machineconfiguration/v1/types.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machineconfiguration/v1/types_machineconfignode.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machineconfiguration/v1/zz_generated.deepcopy.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machineconfiguration/v1/zz_generated.featuregated-crd-manifests.yaml is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machineconfiguration/v1/zz_generated.swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machineconfiguration/v1alpha1/register.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machineconfiguration/v1alpha1/types.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machineconfiguration/v1alpha1/types_internalreleaseimage.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machineconfiguration/v1alpha1/types_osimagestream.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machineconfiguration/v1alpha1/types_pinnedimageset.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machineconfiguration/v1alpha1/zz_generated.deepcopy.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machineconfiguration/v1alpha1/zz_generated.featuregated-crd-manifests.yaml is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/machineconfiguration/v1alpha1/zz_generated.swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/operator/v1/types_ingress.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/operator/v1/types_network.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_12_etcd_01_etcds-OKD.crd.yaml is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_20_kube-apiserver_01_kubeapiservers-OKD.crd.yaml is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_csi-driver_01_clustercsidrivers-OKD.crd.yaml is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers.crd.yaml is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_70_network_01_networks-CustomNoUpgrade.crd.yaml is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_70_network_01_networks-Default.crd.yaml is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_70_network_01_networks-DevPreviewNoUpgrade.crd.yaml is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_70_network_01_networks-OKD.crd.yaml is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_70_network_01_networks-TechPreviewNoUpgrade.crd.yaml is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/operator/v1/zz_generated.crd-manifests/0000_80_machine-config_01_machineconfigurations-OKD.crd.yaml is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/operator/v1/zz_generated.deepcopy.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/operator/v1/zz_generated.featuregated-crd-manifests.yaml is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/api/operator/v1/zz_generated.swagger_doc_generated.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/awsplatformstatus.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/azureplatformstatus.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/baremetalplatformstatus.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/custom.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gatherconfig.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gathererconfig.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gatherers.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gcpplatformstatus.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/gcpserviceendpoint.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/insightsdatagatherspec.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/nutanixplatformstatus.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/openstackplatformstatus.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/ovirtplatformstatus.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/persistentvolumeclaimreference.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/persistentvolumeconfig.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/storage.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/applyconfigurations/config/v1/vsphereplatformstatus.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/applyconfigurations/internal/internal.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1/config_client.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1/generated_expansion.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/clientset/versioned/typed/config/v1/insightsdatagather.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/informers/externalversions/config/v1/insightsdatagather.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/informers/externalversions/config/v1/interface.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/informers/externalversions/generic.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/listers/config/v1/expansion_generated.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/config/listers/config/v1/insightsdatagather.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/internal/internal.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1/machineconfignodestatus.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1/machineconfignodestatusinternalreleaseimage.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1/machineconfignodestatusinternalreleaseimageref.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1/machineconfigpoolspec.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1/machineconfigpoolstatus.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1/osimagestreamreference.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1alpha1/internalreleaseimage.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1alpha1/internalreleaseimagebundlestatus.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1alpha1/internalreleaseimageref.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1alpha1/internalreleaseimagespec.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1alpha1/internalreleaseimagestatus.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1alpha1/osimagestream.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1alpha1/osimagestreamset.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1alpha1/osimagestreamstatus.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/applyconfigurations/machineconfiguration/v1alpha1/pinnedimageref.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/clientset/versioned/typed/machineconfiguration/v1alpha1/generated_expansion.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/clientset/versioned/typed/machineconfiguration/v1alpha1/internalreleaseimage.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/clientset/versioned/typed/machineconfiguration/v1alpha1/machineconfiguration_client.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/clientset/versioned/typed/machineconfiguration/v1alpha1/osimagestream.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/informers/externalversions/generic.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/informers/externalversions/machineconfiguration/v1alpha1/interface.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/informers/externalversions/machineconfiguration/v1alpha1/internalreleaseimage.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/informers/externalversions/machineconfiguration/v1alpha1/osimagestream.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/listers/machineconfiguration/v1alpha1/expansion_generated.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/listers/machineconfiguration/v1alpha1/internalreleaseimage.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/machineconfiguration/listers/machineconfiguration/v1alpha1/osimagestream.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/operator/applyconfigurations/internal/internal.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/bgpmanagedconfig.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/ingresscontrollerspec.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/ingresscontrollertuningoptions.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/nooverlayoptions.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/operator/applyconfigurations/operator/v1/ovnkubernetesconfig.go is excluded by !vendor/**, !**/vendor/**
• vendor/github.com/openshift/client-go/operator/applyconfigurations/utils.go is excluded by !vendor/**, !**/vendor/**
• vendor/modules.txt is excluded by !vendor/**, !**/vendor/**

📒 Files selected for processing (19)

• bindata/network/frr-k8s/003-pki.yaml (1 hunks)
• bindata/network/frr-k8s/frr-k8s.yaml (4 hunks)
• bindata/network/frr-k8s/monitor.yaml (1 hunks)
• bindata/network/frr-k8s/node-status-cleaner.yaml (2 hunks)
• bindata/network/frr-k8s/webhook.yaml (2 hunks)
• bindata/network/ovn-kubernetes/managed/004-config.yaml (4 hunks)
• bindata/network/ovn-kubernetes/self-hosted/004-config.yaml (2 hunks)
• go.mod (2 hunks)
• manifests/0000_70_network_01_networks-CustomNoUpgrade.crd.yaml (1 hunks)
• manifests/0000_70_network_01_networks-Default.crd.yaml (1 hunks)
• manifests/0000_70_network_01_networks-DevPreviewNoUpgrade.crd.yaml (1 hunks)
• manifests/0000_70_network_01_networks-OKD.crd.yaml (1 hunks)
• manifests/0000_70_network_01_networks-TechPreviewNoUpgrade.crd.yaml (1 hunks)
• pkg/controller/operconfig/operconfig_controller.go (1 hunks)
• pkg/network/frr_readiness_test.go (1 hunks)
• pkg/network/ovn_kubernetes.go (14 hunks)
• pkg/network/ovn_kubernetes_test.go (11 hunks)
• pkg/network/render.go (9 hunks)
• pkg/network/render_test.go (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• bindata/network/ovn-kubernetes/self-hosted/004-config.yaml

🧰 Additional context used

📓 Path-based instructions (1)

**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

• manifests/0000_70_network_01_networks-Default.crd.yaml
• manifests/0000_70_network_01_networks-TechPreviewNoUpgrade.crd.yaml
• bindata/network/frr-k8s/003-pki.yaml
• pkg/controller/operconfig/operconfig_controller.go
• go.mod
• bindata/network/ovn-kubernetes/managed/004-config.yaml
• bindata/network/frr-k8s/webhook.yaml
• bindata/network/frr-k8s/node-status-cleaner.yaml
• bindata/network/frr-k8s/frr-k8s.yaml
• bindata/network/frr-k8s/monitor.yaml
• pkg/network/frr_readiness_test.go
• manifests/0000_70_network_01_networks-OKD.crd.yaml
• manifests/0000_70_network_01_networks-DevPreviewNoUpgrade.crd.yaml
• pkg/network/render_test.go
• manifests/0000_70_network_01_networks-CustomNoUpgrade.crd.yaml
• pkg/network/render.go
• pkg/network/ovn_kubernetes.go
• pkg/network/ovn_kubernetes_test.go

🪛 YAMLlint (1.37.1)

bindata/network/frr-k8s/webhook.yaml

[error] 13-13: syntax error: could not find expected ':'

(syntax)

🔇 Additional comments (29)

pkg/controller/operconfig/operconfig_controller.go (1)

305-311: LGTM! MTU validation correctly positioned.

The no-overlay MTU validation is appropriately placed after FillDefaults and follows the existing error handling pattern. Early exit on validation failure prevents invalid configurations from being applied.

manifests/0000_70_network_01_networks-OKD.crd.yaml (1)

10-10: LGTM! Standard feature-set label addition.

The OKD feature-set label aligns with other CRD variants and enables proper feature differentiation.

bindata/network/frr-k8s/node-status-cleaner.yaml (3)

33-36: LGTM! API server connectivity configuration.

The KUBERNETES_SERVICE_HOST and KUBERNETES_SERVICE_PORT environment variables enable direct API server connectivity, supporting hosted control plane and other deployment scenarios.

---

84-86: LGTM! Appropriate toleration for cleanup workload.

The not-ready toleration allows the status cleaner to operate during node startup or issues, which is appropriate for a reconciliation component.

---

91-91: LGTM! Certificate secret naming cleanup.

The secret name change to frr-k8s-webhook-cert aligns with the OperatorPKI-based certificate management introduced in this PR.

bindata/network/frr-k8s/frr-k8s.yaml (3)

54-54: LGTM! Certificate naming improvement.

The secret name change to frr-k8s-metrics-cert is more specific and aligns with the OperatorPKI-based certificate management.

---

103-106: LGTM! API server connectivity for controller.

The environment variables enable direct API server connectivity for the frr-k8s controller, consistent with other containers in this PR.

---

266-269: LGTM! API server connectivity for status container.

The environment variables enable the frr-status container to connect to the API server, consistent with other FRR-k8s components.

bindata/network/ovn-kubernetes/managed/004-config.yaml (3)

16-18: LGTM! Conditional transport configuration.

The transport setting is properly gated and enables no-overlay mode configuration while maintaining backward compatibility with existing deployments.

Also applies to: 121-123

---

58-67: LGTM! No-overlay configuration block.

The [no-overlay] section properly introduces outbound-snat and routing settings, correctly gated by the NoOverlayEnabled flag to ensure feature isolation.

Also applies to: 169-178

---

68-77: LGTM! Managed BGP configuration.

The [bgp.managed] section introduces AS number and topology settings for managed BGP mode, appropriately gated by NoOverlayManagedEnabled flag.

Also applies to: 179-188

bindata/network/frr-k8s/webhook.yaml (1)

12-40: LGTM! Improved webhook CA bundle management.

The change from annotation-based to explicit template-based CA bundle injection is cleaner and integrates properly with the OperatorPKI resources. The conditional rendering ensures the webhook is only configured when the CA bundle is available.

Note: The YAMLlint syntax error is a false positive—it doesn't recognize Go template directives.

pkg/network/render_test.go (2)

641-641: LGTM! Test expectations updated for OperatorPKI resources.

The expected count correctly reflects the two additional OperatorPKI components (webhook and metrics certificates) introduced for FRR-k8s. The inline comment clearly documents this change.

---

647-647: LGTM! Test call updated for new function signature.

The additional parameters (nil for extras, fakeBootstrapResult() for bootstrap data) are appropriate for test context and align with the broader rendering changes that require bootstrap information.

manifests/0000_70_network_01_networks-CustomNoUpgrade.crd.yaml (1)

1-11: LGTM - Auto-generated CRD with comprehensive schema for no-overlay mode.

The CRD properly defines the new no-overlay mode fields (defaultNetworkTransport, defaultNetworkNoOverlayOptions, bgpManagedConfig) with appropriate validation rules and immutability constraints. The validation CEL expressions look correct for enforcing configuration dependencies.

Note: There are minor typos in descriptions (e.g., "currrently" at line 198, "ommitted" at lines 547, 576, 593, 621, 698), but since this file is auto-generated, these should be fixed in the source API definitions.

pkg/network/frr_readiness_test.go (1)

135-330: LGTM - Comprehensive test coverage for shouldSkipOVNKUntilFRRReady.

The test cases cover the key decision points well:

• OVNK already running → don't skip
• No FRR provider → don't skip
• RouteAdvertisements disabled → don't skip
• No OVNKubernetesConfig → don't skip
• All conditions met + FRR ready → don't skip
• All conditions met + FRR not ready → skip
• FRR not ready with empty endpoints → skip

pkg/network/render.go (4)

52-84: LGTM - Well-documented FRR webhook readiness check.

The function correctly:

1. Lists EndpointSlices by service label
2. Handles errors gracefully with logging
3. Counts only endpoints with Ready: true
4. Returns boolean indicating readiness

The logging provides good observability for debugging bootstrap issues.

---

86-132: LGTM - Clear logic for OVNK/FRR coordination.

The function's guard conditions are well-structured and the documentation clearly explains when OVNK rendering is skipped. This prevents the race condition where OVNK starts before FRR webhook is ready.

---

1094-1098: The concern about panic is factually incorrect.

Go map access for a missing key returns the zero value (an empty APIServer{}), not a panic. Additionally, APIServers is always initialized as an empty map in platform.InfraStatus() (line 67 of pkg/platform/platform.go), so the map itself is never nil. Accessing bootstrapResult.Infra.APIServers[bootstrap.APIServerDefault] without a nil check on the map is safe.

If the key doesn't exist, the code would receive an empty APIServer{Host: "", Port: ""}, which is a separate concern about data validation rather than a runtime panic.

Likely an incorrect or invalid review comment.

---

723-729: Clarify that progressing=true status update doesn't trigger faster re-reconciliation.

The controller returns progressing=true and calls SetProgressing() to update status conditions, but this doesn't affect the requeue interval. The reconciler always returns with RequeueAfter: ResyncPeriod (3 minutes), regardless of whether render is progressing. Event-driven watches on Network resources and ConfigMaps can trigger faster reconciliation if those objects change, but the progressing status itself doesn't accelerate requeue. The 3-minute baseline resync is appropriate for this scenario.

bindata/network/frr-k8s/003-pki.yaml (1)

1-26: LGTM - OperatorPKI resources solve bootstrap chicken-and-egg problem.

The comment clearly explains the rationale: service-ca depends on CNI being ready, but the FRR webhook must be ready before OVN-Kubernetes starts when RouteAdvertisements are enabled. Using OperatorPKI breaks this circular dependency.

The targetCert.commonName values correctly match the service DNS names (<service>.<namespace>.svc).

#!/bin/bash
# Verify the OperatorPKI controller creates the expected resources
rg -n "frr-k8s-webhook" pkg/ --type go -A2 -B2

manifests/0000_70_network_01_networks-Default.crd.yaml (1)

1-1046: Auto-generated CRD looks structurally correct.

This file is marked as auto-generated and follows standard Kubernetes CRD patterns. The schema definitions align with the operator API types.

pkg/network/ovn_kubernetes.go (5)

337-386: No-overlay mode configuration looks well-structured.

The no-overlay configuration logic properly:

• Guards on feature gate before processing
• Converts API values to lowercase for ovn-kubernetes config
• Handles BGPManagedConfig separately for cluster-wide BGP settings
• Includes the default case for unsupported topologies (addressing prior review feedback)

---

1145-1177: MTU validation for no-overlay mode is well-implemented.

The function properly handles edge cases:

• Returns early when not applicable (no config, not no-overlay mode, no MTU set)
• Gracefully handles unknown host MTU with a warning
• Provides a clear error message when MTU exceeds host MTU

---

1264-1270: MTU default calculation correctly handles no-overlay mode.

In no-overlay mode, there's no Geneve encapsulation overhead, so using the full host MTU is correct. The overlay path remains unchanged.

---

2236-2244: Good defensive helper to avoid panics on unknown feature gates.

This prevents runtime panics when checking feature gates that may not be registered in the cluster's feature gate configuration.

---

100-100: Error string casing aligns with Go conventions.

The lowercase error messages follow Go's convention that error strings should not be capitalized.

pkg/network/ovn_kubernetes_test.go (1)

76-84: Consistent wiring of FeatureGateNoOverlayMode through test feature‑gate helpers

Adding apifeatures.FeatureGateNoOverlayMode to getDefaultFeatureGates, the knownFeatureGates slice, and the helper gate sets (noFeatureGates, udnFeatureGate, preDefUDNFeatureGates) keeps the tests aligned with the new feature flag and allows explicit enable/disable scenarios without changing existing behaviors.

Also applies to: 929-936, 3829-3836, 4103-4140

manifests/0000_70_network_01_networks-TechPreviewNoUpgrade.crd.yaml (1)

277-741: No‑overlay/BGP schema and CEL invariants for TechPreview look coherent

The ovnKubernetesConfig extensions for no‑overlay (defaultNetworkTransport, defaultNetworkNoOverlayOptions) and managed BGP (bgpManagedConfig) are wired with clear CEL validations: required‑when‑used, sensible defaults (e.g., ASN), and immutability/non‑removal once configured. This should push most invalid combinations (e.g., NoOverlay without options, Managed routing without BGP config, or flipping transport back to empty) to API‑time rather than runtime.

Also applies to: 1019-1036

[ricky-rav]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 0

♻️ Duplicate comments (1)

pkg/network/ovn_kubernetes.go (1)

369-386: Add validation: BGPManagedConfig requires NoOverlayRouting to be "managed".

When BGPManagedConfig is specified (line 369), there's no validation that NoOverlayRouting is set to "managed". BGP managed configuration is cluster-wide and applies to networks using no-overlay mode with managed routing, so these settings should be consistent.

Consider adding validation to ensure NoOverlayRouting is "managed" when BGPManagedConfig is present:

 	// BGP managed configuration is cluster-wide and applies to any network (default or CUDN)
 	// using no-overlay mode with managed routing.
 	// BGPTopology is required when BGPManagedConfig is specified.
 	if noOverlayFeatureEnabled && c.BGPManagedConfig.BGPTopology != "" {
+		// Validate that routing is set to managed when BGPManagedConfig is present
+		if data.Data["NoOverlayRouting"] != "managed" {
+			return nil, progressing, fmt.Errorf("BGPManagedConfig requires NoOverlayRouting to be set to 'managed'")
+		}
+
 		data.Data["NoOverlayManagedEnabled"] = true
 		klog.V(2).Infof("BGP managed configuration enabled for no-overlay mode")

🧹 Nitpick comments (1)

pkg/network/render_test.go (1)

641-649: FRR capability test adjustment looks consistent with new resources

Updating the FRR-capability expectation to 20 objects and wiring the new renderAdditionalRoutingCapabilities parameters (passing nil client and fakeBootstrapResult()) is coherent with the added OperatorPKI resources and conditional VWC rendering. Just ensure the implementation gracefully handles a nil client on FRR paths (the test will expose any panic here).

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between c162e54 and d653c92.

📒 Files selected for processing (18)

• bindata/network/frr-k8s/003-pki.yaml (1 hunks)
• bindata/network/frr-k8s/frr-k8s.yaml (4 hunks)
• bindata/network/frr-k8s/monitor.yaml (1 hunks)
• bindata/network/frr-k8s/node-status-cleaner.yaml (2 hunks)
• bindata/network/frr-k8s/webhook.yaml (2 hunks)
• bindata/network/ovn-kubernetes/managed/004-config.yaml (4 hunks)
• bindata/network/ovn-kubernetes/self-hosted/004-config.yaml (2 hunks)
• manifests/0000_70_network_01_networks-CustomNoUpgrade.crd.yaml (1 hunks)
• manifests/0000_70_network_01_networks-Default.crd.yaml (1 hunks)
• manifests/0000_70_network_01_networks-DevPreviewNoUpgrade.crd.yaml (1 hunks)
• manifests/0000_70_network_01_networks-OKD.crd.yaml (1 hunks)
• manifests/0000_70_network_01_networks-TechPreviewNoUpgrade.crd.yaml (1 hunks)
• pkg/controller/operconfig/operconfig_controller.go (1 hunks)
• pkg/network/frr_readiness_test.go (1 hunks)
• pkg/network/ovn_kubernetes.go (14 hunks)
• pkg/network/ovn_kubernetes_test.go (11 hunks)
• pkg/network/render.go (9 hunks)
• pkg/network/render_test.go (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (6)

• bindata/network/ovn-kubernetes/self-hosted/004-config.yaml
• pkg/network/frr_readiness_test.go
• bindata/network/frr-k8s/frr-k8s.yaml
• pkg/controller/operconfig/operconfig_controller.go
• bindata/network/frr-k8s/node-status-cleaner.yaml
• bindata/network/frr-k8s/monitor.yaml

🧰 Additional context used

📓 Path-based instructions (1)

**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

• bindata/network/ovn-kubernetes/managed/004-config.yaml
• bindata/network/frr-k8s/webhook.yaml
• manifests/0000_70_network_01_networks-DevPreviewNoUpgrade.crd.yaml
• pkg/network/ovn_kubernetes.go
• manifests/0000_70_network_01_networks-CustomNoUpgrade.crd.yaml
• manifests/0000_70_network_01_networks-OKD.crd.yaml
• pkg/network/render.go
• pkg/network/ovn_kubernetes_test.go
• manifests/0000_70_network_01_networks-Default.crd.yaml
• bindata/network/frr-k8s/003-pki.yaml
• pkg/network/render_test.go
• manifests/0000_70_network_01_networks-TechPreviewNoUpgrade.crd.yaml

🧬 Code graph analysis (3)

pkg/network/ovn_kubernetes.go (2)

pkg/hypershift/hypershift.go (1)

• NewHyperShiftConfig (106-125)

pkg/util/util.go (1)

• OVN_CONTROL_PLANE (19-19)

pkg/network/render.go (2)

pkg/client/types.go (1)

• Client (19-31)

pkg/bootstrap/types.go (2)

• BootstrapResult (97-102)
• APIServerDefault (157-157)

pkg/network/ovn_kubernetes_test.go (1)

pkg/network/ovn_kubernetes.go (4)

• OVN_NODE_SELECTOR_DEFAULT_DPU_HOST (63-63)
• OVN_NODE_SELECTOR_DEFAULT_DPU (64-64)
• OVN_NODE_SELECTOR_DEFAULT_SMART_NIC (65-65)
• ValidateMTUForNoOverlay (1149-1177)

🪛 YAMLlint (1.37.1)

bindata/network/frr-k8s/webhook.yaml

[error] 13-13: syntax error: could not find expected ':'

(syntax)

🔇 Additional comments (16)

manifests/0000_70_network_01_networks-OKD.crd.yaml (1)

5-11: OKD feature‑set annotation is consistent and low‑risk

Adding release.openshift.io/feature-set: OKD to the CRD metadata cleanly gates this variant without impacting the CRD’s functional schema.

manifests/0000_70_network_01_networks-CustomNoUpgrade.crd.yaml (1)

240-746: No‑overlay / BGP schema and validations look coherent

The new defaultNetworkTransport, defaultNetworkNoOverlayOptions, and bgpManagedConfig fields, together with their x-kubernetes-validations (immutability, required‑when‑Managed, and NoOverlay gating), form a consistent contract and should prevent misconfiguration of no‑overlay/BGP mode without adding runtime risk.

manifests/0000_70_network_01_networks-DevPreviewNoUpgrade.crd.yaml (1)

1-1046: DevPreview Network CRD definition matches Default semantics

This DevPreviewNoUpgrade Network CRD mirrors the Default variant’s schema and validations, differing only in the feature‑set annotation, which is appropriate for feature‑gated exposure.

manifests/0000_70_network_01_networks-Default.crd.yaml (1)

1-1046: Default Network CRD schema and validations look sound

The new Network CRD for the Default feature set provides a comprehensive, consistent OpenAPI schema and CEL validations (FRR/routeAdvertisements, ipForwarding, migration MTU, etc.) without introducing obvious correctness or security issues.

bindata/network/frr-k8s/webhook.yaml (1)

12-40: Conditional webhook rendering on CA bundle is appropriate

Guarding the ValidatingWebhookConfiguration on .FRRK8sWebhookCABundle and wiring that into caBundle matches the new OperatorPKI-based flow and avoids deploying a broken webhook before CA material is ready.

bindata/network/ovn-kubernetes/managed/004-config.yaml (1)

16-18: No‑overlay and BGP managed blocks are wired correctly into ovnkube.conf

The new transport, [no-overlay], and [bgp.managed] sections are conditionally rendered and symmetric between node/master configs, which matches the no-overlay design and is exercised by the new tests in ovn_kubernetes_test.go. No structural or templating issues stand out.

Also applies to: 58-77, 121-123, 169-188

bindata/network/frr-k8s/003-pki.yaml (1)

1-26: OperatorPKI resources correctly model webhook and metrics TLS

Using OperatorPKI for frr-k8s-webhook and frr-k8s-metrics with service FQDN common names is consistent with avoiding service-ca during bootstrap and with the new webhook CA-bundle handling.

pkg/network/ovn_kubernetes_test.go (1)

76-84: No‑overlay feature‑gate wiring and tests are comprehensive and consistent

Including FeatureGateNoOverlayMode in the default/known feature‑gate sets and adding the new no‑overlay tests (TestRenderOVNKubernetesNoOverlay, defaults/MTU, and TestValidateMTUForNoOverlay) gives good coverage of:

• transport selection and feature‑gate off behavior,
• rendered [no-overlay] and [bgp.managed] sections and value casing,
• MTU defaulting and validation vs host MTU.

The expectations line up with the CRD schema defaults and the ValidateMTUForNoOverlay implementation, and existing tests keep NoOverlay disabled where needed via custom feature‑gate sets.

Also applies to: 929-936, 4483-4922

pkg/network/ovn_kubernetes.go (3)

1145-1177: LGTM: ValidateMTUForNoOverlay is well-structured.

The function correctly validates MTU constraints for no-overlay mode:

• Handles nil config and unset MTU gracefully
• Skips validation when not applicable (non-no-overlay mode)
• Logs a warning instead of failing when hostMTU is unknown
• Validates that MTU doesn't exceed host MTU (correct for no-overlay mode with no encapsulation overhead)

---

1264-1270: LGTM: MTU calculation correctly handles no-overlay mode.

The logic appropriately differentiates MTU calculation:

• No-overlay mode: Uses host MTU directly (no encapsulation overhead)
• Overlay mode: Subtracts encapsulation overhead from host MTU

This aligns with the no-overlay design where packets are not encapsulated.

---

2235-2244: LGTM: Safe feature gate check pattern.

The helper function prevents panics by checking if a feature gate is known before calling Enabled(). This is a good defensive programming pattern that ensures stability when new feature gates are introduced or when feature gate configurations are incomplete.

pkg/network/render.go (5)

52-84: LGTM: FRR webhook readiness check is robust.

The function correctly uses EndpointSlices to check webhook readiness:

• Handles errors gracefully with logging
• Checks for nil conditions before accessing Ready field
• Counts ready endpoints across all EndpointSlices
• Provides diagnostic logging for troubleshooting

This prevents race conditions where OVNK attempts to use the webhook before it's ready.

---

86-132: LGTM: Gating logic correctly prevents race condition.

The function appropriately gates OVNK rendering to prevent race conditions:

• Only applies to fresh installs (OVNK not yet running)
• Requires all conditions to be met (FRR enabled, RouteAdvertisements enabled, webhook not ready)
• Does not interfere with upgrades or existing deployments
• Provides clear logging for operational visibility

This ensures OVNK waits for FRR webhook readiness only when necessary.

---

723-729: LGTM: Integration point correctly handles rendering skip.

The pre-check is properly integrated:

• Placed before rendering logic to prevent unnecessary work
• Returns progressing=true to signal CNO to continue reconciling
• Only applies to OVNKubernetes network type
• Clear comment explains the race condition being prevented

This ensures CNO retries until FRR webhook is ready before rendering OVNK.

---

1077-1102: LGTM: FRR rendering data correctly populated.

The function signature update and data population are appropriate:

• Added client and bootstrapResult parameters enable necessary data fetching
• API server host/port injection solves bootstrap connectivity issue (service IP not routable before CNI is running)
• Webhook CA bundle fetched via dedicated function
• Clear comment explains the bootstrap connectivity challenge

This ensures FRR pods can connect to the API server during initial deployment.

---

1115-1142: LGTM: CA bundle retrieval correctly handles unavailability.

The function safely handles the CA bundle lifecycle:

• Returns empty string when ConfigMap or CA bundle is not yet available
• Base64 encodes the CA bundle for ValidatingWebhookConfiguration
• Provides diagnostic logging

Based on past review discussion, the webhook.yaml template conditionally renders the ValidatingWebhookConfiguration only when FRRK8sWebhookCABundle is non-empty. This design ensures:

• Service is created immediately (outside conditional)
• ValidatingWebhookConfiguration is deferred until CA bundle is available
• No invalid webhook configuration is created

[openshift-ci]

@ricky-rav: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws-ovn-upgrade-ipsec	d653c92	link	true	/test e2e-aws-ovn-upgrade-ipsec
ci/prow/e2e-ovn-ipsec-step-registry	d653c92	link	true	/test e2e-ovn-ipsec-step-registry
ci/prow/okd-scos-images	d653c92	link	true	/test okd-scos-images
ci/prow/e2e-metal-ipi-ovn-dualstack-bgp-local-gw	d653c92	link	true	/test e2e-metal-ipi-ovn-dualstack-bgp-local-gw
ci/prow/security	d653c92	link	false	/test security
ci/prow/frrk8s-e2e	d653c92	link	false	/test frrk8s-e2e
ci/prow/e2e-aws-ovn-upgrade	d653c92	link	true	/test e2e-aws-ovn-upgrade

Full PR test history. Your PR dashboard.

Details

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.