OPNET-595: machineconfiguration/v1alpha1: add BGPVIPConfig CRD - #2972
OPNET-595: machineconfiguration/v1alpha1: add BGPVIPConfig CRD#2972mkowalski wants to merge 1 commit into
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@mkowalski: This pull request references OPNET-595 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 epic to target the "5.0.0" version, but no target version was set. 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. |
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Hello @mkowalski! Some important instructions when contributing to openshift/api: |
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci
Bot
added
the
size/XXL
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdded the Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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.12.2)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@machineconfiguration/v1alpha1/types_bgpvipconfig.go`:
- Around line 139-144: Replace the Password string field in BGPVIPConfig with a
Kubernetes Secret reference, preserving optional configuration semantics. Update
both consumers of BGPVIPConfig authentication data to resolve the referenced
Secret and use its password value, removing all direct reads of the serialized
Password field.
- Around line 56-65: Update the validation rule on the communities field in
BGPVIPConfig to enforce a maximum of 65535 for both segments of two-part classic
communities while retaining the 4294967295 limit for three-part communities. In
machineconfiguration/v1alpha1/tests/bgpvipconfigs.machineconfiguration.openshift.io/BGPBasedVIPManagement.yaml
lines 154-176, change the test to reject 64512:4294967295 and add a valid
three-part boundary case.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 8b0254dc-b4ee-4057-a786-5d1d7b1a1750
⛔ Files ignored due to path filters (8)
machineconfiguration/v1alpha1/zz_generated.crd-manifests/0000_80_machine-config_01_bgpvipconfigs.crd.yamlis excluded by!**/zz_generated.crd-manifests/*machineconfiguration/v1alpha1/zz_generated.deepcopy.gois excluded by!**/zz_generated*machineconfiguration/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/zz_generated*machineconfiguration/v1alpha1/zz_generated.featuregated-crd-manifests/bgpvipconfigs.machineconfiguration.openshift.io/BGPBasedVIPManagement.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1alpha1/zz_generated.model_name.gois excluded by!**/zz_generated*machineconfiguration/v1alpha1/zz_generated.swagger_doc_generated.gois excluded by!**/zz_generated*openapi/generated_openapi/zz_generated.openapi.gois excluded by!openapi/**,!**/zz_generated*openapi/openapi.jsonis excluded by!openapi/**
📒 Files selected for processing (3)
machineconfiguration/v1alpha1/register.gomachineconfiguration/v1alpha1/tests/bgpvipconfigs.machineconfiguration.openshift.io/BGPBasedVIPManagement.yamlmachineconfiguration/v1alpha1/types_bgpvipconfig.go
997094c to
9a4d0b2
Compare
Assisted-By: Claude Fable 5 Signed-off-by: Mat Kowalski <mko@redhat.com>
9a4d0b2 to
60218f9
Compare
Typed, admission-validated configuration API for BGP-based VIP management (enhancement openshift/enhancements#1982, OPNET-595), gated on BGPBasedVIPManagement: a cluster-scoped singleton carrying the local ASN, the default BGP peer set, optional communities and per-host peer overrides, replacing the Dev Preview bgp-vip-config ConfigMap and the serialized-JSON ControllerConfigSpec.BGPVIPPeersJSON user surface (the JSON field remains as machine-config-operator internal transport). API conventions applied: Enabled/Disabled enums instead of booleans, integer-second timer fields (BGP wire-format uint16 seconds), no schema defaults (consumers default and godoc documents omitted behavior), list-map peers/overrides, CEL validation for the singleton name, peer IPs, timer relation and community segment ranges. Peer authentication is secret-only: passwordSecret references a kubernetes.io/basic-auth Secret in the openshift-config namespace ('password' key, 80-byte TCP MD5 limit) - passwords are never stored in this API; the shape mirrors frr-k8s's FRRConfiguration neighbor passwordSecret, which the cluster-network-operator maps it onto. Status carries observedGeneration and two conditions: Rendered (owned by machine-config-operator, set after the per-node peer configuration is applied to the ControllerConfig) and SessionsConfigured (owned by cluster-network-operator, set when the FRR session configuration has been rendered for application). Includes the integration test suite (validation matrix incl. dual-stack peers, host overrides, timer relation, community range and passwordSecret name validation). The consumers (installer, MCO, CNO) are implemented against the inline-password revision and were validated end to end on a live dual-stack baremetal cluster; their passwordSecret rework follows this API. Assisted-By: Claude Fable 5 Signed-off-by: Mat Kowalski <mko@redhat.com>
60218f9 to
22af5b1
Compare
2 participants
Typed, admission-validated configuration API for BGP-based VIP management (enhancement openshift/enhancements#1982), gated on
BGPBasedVIPManagement(gate added in #2923). Replaces the Dev Previewbgp-vip-configConfigMap and the serialized-JSONControllerConfigSpec.BGPVIPPeersJSONuser surface — the JSON field remains as MCO-internal transport, so templates and baremetal-runtimecfg are untouched.Shape
bgpvipconfigs.machineconfiguration.openshift.io/v1alpha1, cluster-scoped singletoncluster:localASN,defaultPeers(1–16, list-map bypeerAddress),communities(≤8, format + segment-range CEL),hostOverrides(≤256, list-map byhostname, replaces — not merges —defaultPeersfor the named node)peerAddress(isIP CEL),peerASN,passwordSecret(secret-only authentication — a name reference to akubernetes.io/basic-authSecret inopenshift-config,passwordkey, 80-byte TCP MD5 limit; no inline password field, and the shape mirrors frr-k8s'sFRRConfigurationneighborpasswordSecretthat CNO maps it onto),port,bfd/ebgpMultiHop(Enabled|Disabledenums),holdTimeSeconds/keepaliveTimeSeconds(0–65535, ≥3× relation CEL)observedGeneration+ conditionsRendered(MCO) andSessionsConfigured(CNO), written via SSA with distinct field managersConventions applied per dev-guide/api-conventions.md: no booleans, no pointers for optional fields, integer-second durations (also BGP's wire-format uint16 seconds), no schema defaults (configuration API — consumers default, godoc documents omitted behavior),
omitempty,omitzerostruct reference per the Go 1.24 guidance.Validation
Integration suite included (86 cases across the validation matrix incl. dual-stack peers, host overrides, timer relation, community segment range, passwordSecret name). The three consumers (installer render, MCO watch/serialize + NodeDisruptionPolicy, CNO FRRConfiguration render) are implemented and were validated end to end on a live dual-stack baremetal cluster against the previous revision of this API: byte-identical rendered peer configuration vs the ConfigMap path, day-2 peer edits propagating in ~45s with zero node disruption, and deletion of the Dev Preview ConfigMap with no effect. The consumers' passwordSecret rework follows this API; consumer PRs come once this merges (they currently vendor this branch).
Design doc with the full decision trail (Option A Infrastructure-spec placement vs this dedicated CRD): bgp-vip-demo spec
This PR description was generated using AI. Please verify before acting on it.