Skip to content

OPNET-780: Add BGPBasedVIPManagement feature gate and BGP VIP management fields #2923

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mkowalski wants to merge 2 commits into
base: master
Choose a base branch
from
Open

OPNET-780: Add BGPBasedVIPManagement feature gate and BGP VIP management fields #2923

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b092743
config/v1: add BGPBasedVIPManagement feature gate and VIPManagement f…
mkowalski Jul 9, 2026
3199351
machineconfiguration/v1: add BGPVIPPeersJSON to ControllerConfigSpec
mkowalski Jul 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 102 additions & 0 deletions config/v1/tests/infrastructures.config.openshift.io/BGPBasedVIPManagement.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
apiVersion: apiextensions.k8s.io/v1 # Hack because controller-gen complains if we don't have this
name: "BGPBasedVIPManagement Infrastructure"
crdName: infrastructures.config.openshift.io
featureGates:
- BGPBasedVIPManagement
tests:

@JoelSpeed JoelSpeed Jul 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These validations are testing the enum value behaves as expected, I think we trust enum values. Probably better to save integration tests for validation like the immutability written in CEL, I'm much more interested in checking that works personally

@mkowalski mkowalski Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's fair, added test for CEL immutability and removed testing enums

onUpdate:
- name: Should allow setting vipManagement to BGP on BareMetal
initial: |
apiVersion: config.openshift.io/v1
kind: Infrastructure
spec:
platformSpec:
baremetal: {}
type: BareMetal
updated: |
apiVersion: config.openshift.io/v1
kind: Infrastructure
spec:
platformSpec:
baremetal: {}
type: BareMetal
status:
platform: BareMetal
platformStatus:
baremetal:
vipManagement: BGP
type: BareMetal
expected: |
apiVersion: config.openshift.io/v1
kind: Infrastructure
spec:
platformSpec:
baremetal: {}
type: BareMetal
status:
platform: BareMetal
platformStatus:
baremetal:
vipManagement: BGP
loadBalancer:
type: OpenShiftManagedDefault
type: BareMetal
cpuPartitioning: None
infrastructureTopology: HighlyAvailable
controlPlaneTopology: HighlyAvailable
- name: Should reject changing vipManagement from BGP to Keepalived
initial: |
apiVersion: config.openshift.io/v1
kind: Infrastructure
spec:
platformSpec:
baremetal: {}
type: BareMetal
status:
platform: BareMetal
platformStatus:
baremetal:
vipManagement: BGP
type: BareMetal
updated: |
apiVersion: config.openshift.io/v1
kind: Infrastructure
spec:
platformSpec:
baremetal: {}
type: BareMetal
status:
platform: BareMetal
platformStatus:
baremetal:
vipManagement: Keepalived
type: BareMetal
expectedStatusError: 'platformStatus.baremetal.vipManagement: Invalid value: "string": vipManagement is immutable once set'
- name: Should reject changing vipManagement from Keepalived to BGP
initial: |
apiVersion: config.openshift.io/v1
kind: Infrastructure
spec:
platformSpec:
baremetal: {}
type: BareMetal
status:
platform: BareMetal
platformStatus:
baremetal:
vipManagement: Keepalived
type: BareMetal
updated: |
apiVersion: config.openshift.io/v1
kind: Infrastructure
spec:
platformSpec:
baremetal: {}
type: BareMetal
status:
platform: BareMetal
platformStatus:
baremetal:
vipManagement: BGP
type: BareMetal
expectedStatusError: 'platformStatus.baremetal.vipManagement: Invalid value: "string": vipManagement is immutable once set'
30 changes: 30 additions & 0 deletions config/v1/types_infrastructure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,21 @@ const (
DNSRecordsTypeInternal DNSRecordsType = "Internal"
)

// VIPManagementType defines which mechanism manages the API and Ingress
// VIPs on an on-premise cluster.
// +kubebuilder:validation:Enum=Keepalived;BGP
// +enum
type VIPManagementType string

const (
// VIPManagementTypeKeepalived means the VIPs are managed by the default
// keepalived/VRRP mechanism.
VIPManagementTypeKeepalived VIPManagementType = "Keepalived"
// VIPManagementTypeBGP means the VIPs are advertised via BGP by kube-vip
// (Routing Table Mode) and frr-k8s running as static pods.
VIPManagementTypeBGP VIPManagementType = "BGP"
)

// PlatformType is a specific supported infrastructure provider.
// +kubebuilder:validation:Enum="";AWS;Azure;BareMetal;GCP;Libvirt;OpenStack;None;VSphere;oVirt;IBMCloud;KubeVirt;EquinixMetal;PowerVS;AlibabaCloud;Nutanix;External
type PlatformType string
Expand Down Expand Up @@ -1074,6 +1089,21 @@ type BareMetalPlatformStatus struct {
// +optional
LoadBalancer *BareMetalPlatformLoadBalancer `json:"loadBalancer,omitempty"`

// vipManagement indicates which VIP management mechanism is active
// on this cluster.
// Allowed values are `Keepalived`, `BGP`, and omitted.
// Once set to a non-empty value, this field is immutable.
// When set to `BGP`, kube-vip (Routing Table Mode) and frr-k8s are
// deployed as static pods to advertise VIPs via BGP, replacing the
// default keepalived/VRRP mechanism.
// When set to `Keepalived`, the default keepalived-based VIP
// management is used.
// When omitted, the default keepalived-based VIP management is used.
// +kubebuilder:validation:XValidation:rule="oldSelf == '' || self == oldSelf",message="vipManagement is immutable once set"
// +openshift:enable:FeatureGate=BGPBasedVIPManagement
// +optional
VIPManagement VIPManagementType `json:"vipManagement,omitempty"`
Comment thread
JoelSpeed marked this conversation as resolved.
Comment on lines +1092 to +1105

@JoelSpeed JoelSpeed Jul 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The documentation for BGP suggests that Keepalived to BGP is an acceptable transition? If I set this to keepalived explicitly, can I not later move it to BGP?

@mkowalski mkowalski Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We will do transition from Keepalived to BGP in the future but not in DevPreview

@JoelSpeed JoelSpeed Jul 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IIUC, setting BGP day 2 is effectively doing Keepalived to BGP already no? So we allow and have to test that path already right? Or is there something I'm missing, what extra work do you need to do to verify that an explicit Keepalived migration to explicit BGP is safe?


// dnsRecordsType determines whether records for api, api-int, and ingress
// are provided by the internal DNS service or externally.
// Allowed values are `Internal`, `External`, and omitted.
Expand Down
2 changes: 1 addition & 1 deletion config/v1/types_infrastructure_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (

const (
infraCRDDefaultFilePath = "0000_10_config-operator_01_infrastructures-Default.crd.yaml"
infraCRDTestPreviewFilePath = "0000_10_config-operator_01_infrastructures-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml"
infraCRDTestPreviewFilePath = "0000_10_config-operator_01_infrastructures-TechPreviewNoUpgrade.crd.yaml"
)

// TestInfrastructureStatusDefault verifies that the infrastructure CR status does not have default value
Expand Down
19 changes: 19 additions & 0 deletions ...-manifests/0000_10_config-operator_01_infrastructures-Hypershift-CustomNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1785,6 +1785,25 @@ spec:
datacenter DNS, a DNS service is hosted as a static pod to serve those hostnames
to the nodes in the cluster.
type: string
vipManagement:
description: |-
vipManagement indicates which VIP management mechanism is active
on this cluster.
Allowed values are `Keepalived`, `BGP`, and omitted.
Once set to a non-empty value, this field is immutable.
When set to `BGP`, kube-vip (Routing Table Mode) and frr-k8s are
deployed as static pods to advertise VIPs via BGP, replacing the
default keepalived/VRRP mechanism.
When set to `Keepalived`, the default keepalived-based VIP
management is used.
When omitted, the default keepalived-based VIP management is used.
enum:
- Keepalived
- BGP
type: string
x-kubernetes-validations:
- message: vipManagement is immutable once set
rule: oldSelf == '' || self == oldSelf
type: object
x-kubernetes-validations:
- message: dnsRecordsType may only be set to External when loadBalancer.type
Expand Down
19 changes: 19 additions & 0 deletions ...ifests/0000_10_config-operator_01_infrastructures-Hypershift-DevPreviewNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1770,6 +1770,25 @@ spec:
datacenter DNS, a DNS service is hosted as a static pod to serve those hostnames
to the nodes in the cluster.
type: string
vipManagement:
description: |-
vipManagement indicates which VIP management mechanism is active
on this cluster.
Allowed values are `Keepalived`, `BGP`, and omitted.
Once set to a non-empty value, this field is immutable.
When set to `BGP`, kube-vip (Routing Table Mode) and frr-k8s are
deployed as static pods to advertise VIPs via BGP, replacing the
default keepalived/VRRP mechanism.
When set to `Keepalived`, the default keepalived-based VIP
management is used.
When omitted, the default keepalived-based VIP management is used.
enum:
- Keepalived
- BGP
type: string
x-kubernetes-validations:
- message: vipManagement is immutable once set
rule: oldSelf == '' || self == oldSelf
type: object
x-kubernetes-validations:
- message: dnsRecordsType may only be set to External when loadBalancer.type
Expand Down
19 changes: 19 additions & 0 deletions ...nifests/0000_10_config-operator_01_infrastructures-SelfManagedHA-CustomNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1785,6 +1785,25 @@ spec:
datacenter DNS, a DNS service is hosted as a static pod to serve those hostnames
to the nodes in the cluster.
type: string
vipManagement:
description: |-
vipManagement indicates which VIP management mechanism is active
on this cluster.
Allowed values are `Keepalived`, `BGP`, and omitted.
Once set to a non-empty value, this field is immutable.
When set to `BGP`, kube-vip (Routing Table Mode) and frr-k8s are
deployed as static pods to advertise VIPs via BGP, replacing the
default keepalived/VRRP mechanism.
When set to `Keepalived`, the default keepalived-based VIP
management is used.
When omitted, the default keepalived-based VIP management is used.
enum:
- Keepalived
- BGP
type: string
x-kubernetes-validations:
- message: vipManagement is immutable once set
rule: oldSelf == '' || self == oldSelf
type: object
x-kubernetes-validations:
- message: dnsRecordsType may only be set to External when loadBalancer.type
Expand Down
19 changes: 19 additions & 0 deletions ...sts/0000_10_config-operator_01_infrastructures-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1785,6 +1785,25 @@ spec:
datacenter DNS, a DNS service is hosted as a static pod to serve those hostnames
to the nodes in the cluster.
type: string
vipManagement:
description: |-
vipManagement indicates which VIP management mechanism is active
on this cluster.
Allowed values are `Keepalived`, `BGP`, and omitted.
Once set to a non-empty value, this field is immutable.
When set to `BGP`, kube-vip (Routing Table Mode) and frr-k8s are
deployed as static pods to advertise VIPs via BGP, replacing the
default keepalived/VRRP mechanism.
When set to `Keepalived`, the default keepalived-based VIP
management is used.
When omitted, the default keepalived-based VIP management is used.
enum:
- Keepalived
- BGP
type: string
x-kubernetes-validations:
- message: vipManagement is immutable once set
rule: oldSelf == '' || self == oldSelf
type: object
x-kubernetes-validations:
- message: dnsRecordsType may only be set to External when loadBalancer.type
Expand Down
1 change: 1 addition & 0 deletions ...lfManagedHA-TechPreviewNoUpgrade.crd.yaml → ...astructures-TechPreviewNoUpgrade.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ metadata:
annotations:
api-approved.openshift.io: https://github.com/openshift/api/pull/470
api.openshift.io/merged-by-featuregates: "true"
include.release.openshift.io/ibm-cloud-managed: "true"
include.release.openshift.io/self-managed-high-availability: "true"
release.openshift.io/bootstrap-required: "true"
release.openshift.io/feature-set: TechPreviewNoUpgrade
Expand Down
1 change: 1 addition & 0 deletions config/v1/zz_generated.featuregated-crd-manifests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,7 @@ infrastructures.config.openshift.io:
- AWSClusterHostedDNSInstall
- AWSDualStackInstall
- AzureDualStackInstall
- BGPBasedVIPManagement
- DualReplica
- DyanmicServiceEndpointIBMCloud
- MutableTopology
Expand Down
Loading