Skip to content

OSASINFRA-4400: Add support for max_allowed_address_pairs OpenStack #3058

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
danchild wants to merge 2 commits into
base: master
Choose a base branch
from
Open

OSASINFRA-4400: Add support for max_allowed_address_pairs OpenStack #3058

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
67528a8
Pass OS max_allowed_address_pairs to CNCC via ConfigMap
danchild Jul 15, 2026
c860305
Cover cloud-network-config bootstrap and rendering
danchild Jul 16, 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
3 changes: 3 additions & 0 deletions bindata/cloud-network-config-controller/managed/controller.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,9 @@ spec:
-platform-aws-ca-override={{.PlatformAWSCAPath}} \
-platform-azure-environment={{.PlatformAzureEnvironment}} \
-secret-name cloud-network-config-controller-creds \
{{- if .OSMaxAllowedAddressPairs }}
-platform-os-max-allowed-address-pairs={{ .OSMaxAllowedAddressPairs }} \
{{- end }}
-kubeconfig /var/run/secrets/hosted_cluster/kubeconfig
env:
- name: CONTROLLER_NAMESPACE
Expand Down
18 changes: 12 additions & 6 deletions bindata/cloud-network-config-controller/self-hosted/controller.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,18 @@ spec:
image: {{.CloudNetworkConfigControllerImage}}
imagePullPolicy: IfNotPresent
command: ["/usr/bin/cloud-network-config-controller"]
args: [ "-platform-type", "{{.PlatformType}}",
"-platform-region={{.PlatformRegion}}",
"-platform-api-url={{.PlatformAPIURL}}",
"-platform-aws-ca-override={{.PlatformAWSCAPath}}",
"-platform-azure-environment={{.PlatformAzureEnvironment}}",
"-secret-name", "cloud-credentials"]
args:
- "-platform-type"
- "{{.PlatformType}}"
- "-platform-region={{.PlatformRegion}}"
- "-platform-api-url={{.PlatformAPIURL}}"
- "-platform-aws-ca-override={{.PlatformAWSCAPath}}"
- "-platform-azure-environment={{.PlatformAzureEnvironment}}"
- "-secret-name"
- "cloud-credentials"
{{- if .OSMaxAllowedAddressPairs }}
- "-platform-os-max-allowed-address-pairs={{ .OSMaxAllowedAddressPairs }}"
{{- end }}
Comment on lines +54 to +56

@coderabbitai coderabbitai Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Fix the YAML template syntax before merge.

The conditional actions on Lines 54 and 56 are outside YAML comments or scalar values. YAMLlint reports Line 55 with could not find expected ':'.

Place the template control actions in YAML comments without trim markers, then validate both rendered branches.

Proposed fix 
-{{- if .OSMaxAllowedAddressPairs }}
+# {{ if .OSMaxAllowedAddressPairs }}
         - "-platform-os-max-allowed-address-pairs={{ .OSMaxAllowedAddressPairs }}"
-{{- end }}
+# {{ end }}
🧰 Tools
🪛 YAMLlint (1.37.1)

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

(syntax)

🤖 Prompt for 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.

In `@bindata/cloud-network-config-controller/self-hosted/controller.yaml` around
lines 54 - 56, Fix the Helm template conditional around OSMaxAllowedAddressPairs
by placing the if/end actions inside YAML comments without trim markers, while
keeping the argument entry valid YAML in the rendered output. Validate both the
branch where OSMaxAllowedAddressPairs is set and the branch where it is absent,
using the existing template structure in controller.yaml.

Source: Linters/SAST tools

env:
- name: CONTROLLER_NAMESPACE
valueFrom:
Expand Down
8 changes: 8 additions & 0 deletions pkg/bootstrap/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,20 @@ type IPTablesAlerterBootstrapResult struct {
Enabled bool
}

// CloudNetworkConfigBootstrapResult contains bootstrap configuration
// read from the cloud-network-config ConfigMap.
type CloudNetworkConfigBootstrapResult struct {
OSMaxAllowedAddressPairs *int
}

type BootstrapResult struct {
Infra InfraStatus

OVN OVNBootstrapResult
IPTablesAlerter IPTablesAlerterBootstrapResult
TLSProfile TLSProfile

CloudNetworkConfig CloudNetworkConfigBootstrapResult
}

type InfraStatus struct {
Expand Down
39 changes: 39 additions & 0 deletions pkg/network/bootstrap.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ package network

import (
"context"
"fmt"
"strconv"

configv1 "github.com/openshift/api/config/v1"
operv1 "github.com/openshift/api/operator/v1"
"github.com/openshift/cluster-network-operator/pkg/bootstrap"
cnoclient "github.com/openshift/cluster-network-operator/pkg/client"
Expand Down Expand Up @@ -34,6 +37,14 @@ func Bootstrap(conf *operv1.Network, client cnoclient.Client) (*bootstrap.Bootst

out.IPTablesAlerter = iptablesAlerterBootstrap(client.ClientFor("").CRClient())

if infraStatus.PlatformType == configv1.OpenStackPlatformType {
cnc, err := cloudNetworkConfigBootstrap(client.ClientFor("").CRClient())
if err != nil {
return nil, err
}
out.CloudNetworkConfig = cnc
}

out.TLSProfile, err = GetTLSProfile(client, infraStatus.HostedControlPlane)
if err != nil {
return nil, err
Expand Down Expand Up @@ -67,3 +78,31 @@ func iptablesAlerterBootstrap(cl crclient.Reader) bootstrap.IPTablesAlerterBoots

return result
}

func cloudNetworkConfigBootstrap(cl crclient.Reader) (bootstrap.CloudNetworkConfigBootstrapResult, error) {
result := bootstrap.CloudNetworkConfigBootstrapResult{}

cm := &corev1.ConfigMap{}
if err := cl.Get(context.TODO(), types.NamespacedName{
Namespace: "openshift-network-operator",
Name: "cloud-network-config",
}, cm); err != nil {
if !apierrors.IsNotFound(err) {
return result, fmt.Errorf("error fetching cloud-network-config configmap: %w", err)
}
return result, nil
Comment on lines +86 to +93

@coderabbitai coderabbitai Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate and inspect the target file and related call sites without running repository code.
printf 'Target file lines:\n'
sed -n '1,150p' pkg/network/bootstrap.go

printf '\nCall sites for Bootstrap / cloudNetworkConfigBootstrap:\n'
rg -n "Bootstrap\\(|cloudNetworkConfigBootstrap\\(" pkg cmd test manifests 2>/dev/null || true

printf '\nFunction signatures and TODO context usages in bootstrap-related Go files:\n'
rg -n "context\\.TODO\\(|func .*Bootstrap|cloudNetworkConfigBootstrap" pkg/network/bootstrap.go pkg/network -g '*.go'

Repository: openshift/cluster-network-operator

Length of output: 13900

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'operconfig_controller relevant section:\n'
sed -n '290,350p' pkg/controller/operconfig/operconfig_controller.go

printf '\nReconcile-related imports and context handling in operconfig controller:\n'
sed -n '1,80p' pkg/controller/operconfig/operconfig_controller.go

printf '\nBootstrap tests around cloud network config:\n'
sed -n '210,250p' pkg/network/bootstrap_test.go
sed -n '420,485p' pkg/network/cloud_network_test.go

Repository: openshift/cluster-network-operator

Length of output: 9006

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Reconciler signature and Reconcile method header:\n'
rg -n "type ReconcileOperConfig|func \\(r \\*ReconcileOperConfig\\) Reconcile|func \\(r \\*ReconcileOperConfig\\)" pkg/controller/operconfig/operconfig_controller.go

Repository: openshift/cluster-network-operator

Length of output: 380

Thread the reconciliation context into the ConfigMap reads.

This OpenStack bootstrap path currently uses context.TODO() for cl.Get, so reconciliation cancellation/timeout is not applied. Add a context.Context parameter to Bootstrap, pass it into cloudNetworkConfigBootstrap, and use it for the ConfigMap request.

🤖 Prompt for 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.

In `@pkg/network/bootstrap.go` around lines 86 - 93, Update Bootstrap to accept a
context.Context parameter, thread that context through
cloudNetworkConfigBootstrap, and replace context.TODO() in its cl.Get ConfigMap
request with the provided reconciliation context.

Source: Path instructions

}

raw, ok := cm.Data["platform-os-max-allowed-address-pairs"]
if !ok {
return result, nil
}

val, err := strconv.Atoi(raw)
if err != nil {
return result, fmt.Errorf("error parsing cloud-network-config platform-os-max-allowed-address-pairs=%q: %w", raw, err)
}

result.OSMaxAllowedAddressPairs = &val
return result, nil
}
Comment thread
danchild marked this conversation as resolved.
176 changes: 176 additions & 0 deletions pkg/network/bootstrap_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,3 +215,179 @@ func TestBootstrap(t *testing.T) {
})
})
}

func TestBootstrapCloudNetworkConfig(t *testing.T) {
baseOperConfig := &operv1.Network{
ObjectMeta: metav1.ObjectMeta{Name: names.OPERATOR_CONFIG},
Spec: operv1.NetworkSpec{
DefaultNetwork: operv1.DefaultNetworkDefinition{
Type: operv1.NetworkTypeOVNKubernetes,
OVNKubernetesConfig: &operv1.OVNKubernetesConfig{
MTU: nil,
},
},
},
}

baseClientObjs := func(platformType configv1.PlatformType) []crclient.Object {
return []crclient.Object{
&configv1.Infrastructure{
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
Status: configv1.InfrastructureStatus{
PlatformStatus: &configv1.PlatformStatus{
Type: platformType,
},
},
},
&configv1.Proxy{
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
},
&corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: network.CLUSTER_CONFIG_NAME,
Namespace: network.CLUSTER_CONFIG_NAMESPACE,
},
Data: map[string]string{
"install-config": "controlPlane:\n replicas: 3\n",
},
},
&configv1.APIServer{
ObjectMeta: metav1.ObjectMeta{Name: "cluster"},
},
}
}

tests := []struct {
name string
platformType configv1.PlatformType
configMap *corev1.ConfigMap
expectValue *int
expectErr bool
}{
{
name: "skipped on non-OpenStack platform",
platformType: configv1.NonePlatformType,
configMap: &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "cloud-network-config",
Namespace: "openshift-network-operator",
},
Data: map[string]string{
"platform-os-max-allowed-address-pairs": "20",
},
},
expectValue: nil,
},
{
name: "ConfigMap absent",
platformType: configv1.OpenStackPlatformType,
configMap: nil,
expectValue: nil,
},
{
name: "key missing from ConfigMap",
platformType: configv1.OpenStackPlatformType,
configMap: &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "cloud-network-config",
Namespace: "openshift-network-operator",
},
Data: map[string]string{"other-key": "value"},
},
expectValue: nil,
},
{
name: "valid value 20",
platformType: configv1.OpenStackPlatformType,
configMap: &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "cloud-network-config",
Namespace: "openshift-network-operator",
},
Data: map[string]string{
"platform-os-max-allowed-address-pairs": "20",
},
},
expectValue: toPtr(20),
},
{
name: "zero value",
platformType: configv1.OpenStackPlatformType,
configMap: &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "cloud-network-config",
Namespace: "openshift-network-operator",
},
Data: map[string]string{
"platform-os-max-allowed-address-pairs": "0",
},
},
expectValue: toPtr(0),
},
{
name: "negative value",
platformType: configv1.OpenStackPlatformType,
configMap: &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "cloud-network-config",
Namespace: "openshift-network-operator",
},
Data: map[string]string{
"platform-os-max-allowed-address-pairs": "-5",
},
},
expectValue: toPtr(-5),
},
{
name: "non-integer value",
platformType: configv1.OpenStackPlatformType,
configMap: &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "cloud-network-config",
Namespace: "openshift-network-operator",
},
Data: map[string]string{
"platform-os-max-allowed-address-pairs": "abc",
},
},
expectValue: nil,
expectErr: true,
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
objs := baseClientObjs(tc.platformType)
if tc.configMap != nil {
objs = append(objs, tc.configMap)
}
client := fakeclient.NewFakeClient(objs...)

result, err := network.Bootstrap(baseOperConfig, client)
if tc.expectErr {
if err == nil {
t.Errorf("expected error, got nil")
}
return
}
if err != nil {
t.Fatalf("Bootstrap failed: %v", err)
}

got := result.CloudNetworkConfig.OSMaxAllowedAddressPairs
switch {
case tc.expectValue == nil && got != nil:
t.Errorf("expected nil, got %d", *got)
case tc.expectValue != nil && got == nil:
t.Errorf("expected %d, got nil", *tc.expectValue)
case tc.expectValue != nil && got != nil && *got != *tc.expectValue:
t.Errorf("expected %d, got %d", *tc.expectValue, *got)
}
})
}
}

// Convenience function to create pointers for tests only
func toPtr[t any](i t) *t {
return &i
}
10 changes: 10 additions & 0 deletions pkg/network/cloud_network.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package network

import (
"fmt"
"net"
"os"
"path/filepath"
Expand Down Expand Up @@ -49,6 +50,15 @@ func renderCloudNetworkConfigController(conf *operv1.NetworkSpec, bootstrapResul
data.Data["PlatformAzureEnvironment"] = ""
data.Data["PlatformAWSCAPath"] = ""

cnc := bootstrapResult.CloudNetworkConfig
if cnc.OSMaxAllowedAddressPairs != nil {
if *cnc.OSMaxAllowedAddressPairs <= 0 {
return nil, fmt.Errorf("invalid cloud-network-config: platform-os-max-allowed-address-pairs must be a non-zero, positive integer, got %d", *cnc.OSMaxAllowedAddressPairs)
}
}

data.Data["OSMaxAllowedAddressPairs"] = cnc.OSMaxAllowedAddressPairs

// AWS and azure allow for funky endpoint overriding.
// in different ways, of course.
apiurl := ""
Expand Down
Loading