-
Notifications
You must be signed in to change notification settings - Fork 836
OSASINFRA-4437: openstack: add additional fields to support multiple VIPs #2976
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1194,6 +1194,28 @@ type OpenStackPlatformSpec struct { | |
| // +optional | ||
| IngressIPs []IP `json:"ingressIPs"` | ||
|
|
||
| // additionalAPIServerIPs is a list of additional IP addresses to contact | ||
| // the Kubernetes API server on separate networks. These are used when a | ||
| // user-managed load balancer exposes the API server on multiple networks | ||
| // simultaneously. Each entry must be a valid IP address. This field is | ||
| // only valid when loadBalancer.type is set to UserManaged. | ||
| // | ||
| // +kubebuilder:validation:MaxItems=10 | ||
| // +listType=atomic | ||
| // +optional | ||
| AdditionalAPIServerIPs []IP `json:"additionalAPIServerIPs,omitempty"` | ||
|
Comment on lines
+1203
to
+1206
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. Additional*ips missing featuregate marker New stable config/v1 API fields were added without a +openshift:enable:FeatureGate=<Name> marker immediately above them. This violates the requirement to gate new fields in stable APIs to preserve API compatibility expectations. Agent Prompt
Comment on lines
+1205
to
+1206
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2. Additional*ips omit behavior undocumented The new optional fields AdditionalAPIServerIPs and AdditionalIngressIPs do not document what happens when they are omitted. This makes API behavior unclear for clients and violates the required omission-behavior documentation rule. Agent Prompt
Comment on lines
+1203
to
+1206
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 4. maxitems=10 not documented The new list fields include +kubebuilder:validation:MaxItems=10 but the field comments do not mention this item-count constraint. This violates the requirement to document kubebuilder constraint markers in field comments. Agent Prompt
|
||
|
|
||
| // additionalIngressIPs is a list of additional IP addresses that route to | ||
| // the default ingress controller on separate networks. These are used when | ||
| // a user-managed load balancer exposes the ingress controller on multiple | ||
| // networks simultaneously. Each entry must be a valid IP address. This | ||
| // field is only valid when loadBalancer.type is set to UserManaged. | ||
| // | ||
| // +kubebuilder:validation:MaxItems=10 | ||
| // +listType=atomic | ||
| // +optional | ||
| AdditionalIngressIPs []IP `json:"additionalIngressIPs,omitempty"` | ||
|
|
||
|
Comment on lines
+1197
to
+1218
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- target type and nearby declarations ---'
rg -n -C 8 'AdditionalAPIServerIPs|AdditionalIngressIPs|type OpenStackPlatformSpec|type OpenStackPlatformStatus|LoadBalancer|DNSRecordsType' config/v1/types_infrastructure.go
printf '%s\n' '--- all related validation rules and field references ---'
rg -n -C 5 'additionalAPIServerIPs|additionalIngressIPs|loadBalancer\.type|platformSpec\.openstack|platformStatus\.openstack|controlPlaneTopology must match' .
printf '%s\n' '--- Infrastructure and ControllerConfig declarations ---'
rg -n -C 12 'type Infrastructure struct|type ControllerConfig struct|x-kubernetes-validations' config/v1Repository: openshift/api Length of output: 50370 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- target source matches only ---'
rg -n -C 12 'AdditionalAPIServerIPs|AdditionalIngressIPs|type OpenStackPlatformSpec|type OpenStackPlatformStatus|LoadBalancer|DNSRecordsType' config/v1/types_infrastructure.go
printf '%s\n' '--- Infrastructure and ControllerConfig source declarations ---'
rg -n -C 15 'type Infrastructure struct|type ControllerConfig struct|platformSpec|platformStatus|controlPlaneTopology must match' config/v1 --glob '*.go'
printf '%s\n' '--- current change summary ---'
git diff --stat -- config/v1/types_infrastructure.go
git diff -- config/v1/types_infrastructure.go | sed -n '1,220p'Repository: openshift/api Length of output: 50369 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- exact change ---'
git diff --unified=8 -- config/v1/types_infrastructure.go
printf '%s\n' '--- non-generated consumers and validators ---'
rg -n -C 6 'AdditionalAPIServerIPs|AdditionalIngressIPs|additionalAPIServerIPs|additionalIngressIPs' \
--glob '!config/v1/zz_generated.crd-manifests/**' \
--glob '!**/vendor/**' .
printf '%s\n' '--- relevant generated schema fragments ---'
python3 - <<'PY'
from pathlib import Path
for p in Path("config/v1/zz_generated.crd-manifests").glob("*.yaml"):
text = p.read_text(errors="replace")
hits = []
for needle in ("additionalAPIServerIPs:", "additionalIngressIPs:"):
start = 0
while True:
i = text.find(needle, start)
if i < 0:
break
line = text.count("\n", 0, i) + 1
block = text[i:i+900].splitlines()[:18]
hits.append((line, needle, block))
start = i + len(needle)
if hits:
print(f"FILE {p}")
for line, needle, block in hits:
print(f"LINE {line} {needle}")
print("\n".join(block))
PYRepository: openshift/api Length of output: 50370 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- target diff additions and removals ---'
git diff --unified=0 -- config/v1/types_infrastructure.go \
| rg '^(diff|@@|[+-][^+-])' | sed -n '1,180p'
printf '%s\n' '--- consumers in Go and validation markers ---'
rg -n -C 5 'AdditionalAPIServerIPs|AdditionalIngressIPs|additionalAPIServerIPs|additionalIngressIPs' \
config machineconfiguration --glob '*.go' --glob '!zz_generated/**' --glob '!vendor/**'
printf '%s\n' '--- ControllerConfig source shape ---'
rg -n -C 14 'type ControllerConfig struct|Infrastructure|platformSpec|platformStatus|controlPlaneTopology' \
machineconfiguration --glob '*.go' --glob '!zz_generated/**'
printf '%s\n' '--- Infrastructure CRD files ---'
find config/v1/zz_generated.crd-manifests config/v1/zz_generated.featuregated-crd-manifests \
-type f -iname '*infrastructures*' -print | head -20Repository: openshift/api Length of output: 23321 🏁 Script executed: #!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
source = Path("config/v1/types_infrastructure.go").read_text()
def type_body(name):
m = re.search(r"(?m)^type " + re.escape(name) + r"\s+struct\s*\{", source)
if not m:
raise SystemExit(f"missing {name}")
start = m.end()
depth = 1
i = start
while depth and i < len(source):
if source[i] == "{":
depth += 1
elif source[i] == "}":
depth -= 1
i += 1
return source[start:i-1]
for name in ("OpenStackPlatformSpec", "OpenStackPlatformStatus"):
body = type_body(name)
print(f"{name}:")
for field in ("LoadBalancer", "AdditionalAPIServerIPs", "AdditionalIngressIPs"):
print(f" {field}: {'yes' if re.search(r'(?m)^\\s*' + field + r'\\b', body) else 'no'}")
print(" loadBalancer.type validation markers:",
"yes" if "loadBalancer.type" in body else "no")
print("Infrastructure root spec/status validation markers:")
infra_head = source[source.index("type Infrastructure struct")-1000:source.index("type Infrastructure struct")]
for needle in ("self.spec.controlPlaneTopology", "self.status.controlPlaneTopology"):
print(f" {needle}: {'yes' if needle in infra_head else 'no'}")
print("Source validation markers mentioning additional fields:")
for line_no, line in enumerate(source.splitlines(), 1):
if ("additionalAPIServerIPs" in line or "additionalIngressIPs" in line) and "XValidation" in line:
print(f" {line_no}: {line.strip()}")
PY
printf '%s\n' '--- generated validation expressions in the default Infrastructure CRD ---'
rg -n -C 3 \
'additionalAPIServerIPs|additionalIngressIPs|spec\.controlPlaneTopology must match|self\.spec\.controlPlaneTopology|self\.status\.controlPlaneTopology' \
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-Default.crd.yamlRepository: openshift/api Length of output: 428 🏁 Script executed: #!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
source = Path("config/v1/types_infrastructure.go").read_text()
def type_body(name):
m = re.search(r"(?m)^type " + re.escape(name) + r"\s+struct\s*\{", source)
if not m:
raise SystemExit(f"missing {name}")
start = m.end()
depth = 1
i = start
while depth and i < len(source):
if source[i] == "{":
depth += 1
elif source[i] == "}":
depth -= 1
i += 1
return source[start:i-1]
for name in ("OpenStackPlatformSpec", "OpenStackPlatformStatus"):
body = type_body(name)
print(f"{name}:")
for field in ("LoadBalancer", "AdditionalAPIServerIPs", "AdditionalIngressIPs"):
present = bool(re.search(r"(?m)^\s*" + field + r"\b", body))
print(f" {field}: {'yes' if present else 'no'}")
print(" loadBalancer.type validation markers:",
"yes" if "loadBalancer.type" in body else "no")
print("Infrastructure root spec/status validation markers:")
infra_pos = source.index("type Infrastructure struct")
infra_head = source[max(0, infra_pos - 1200):infra_pos]
for needle in ("self.spec.controlPlaneTopology", "self.status.controlPlaneTopology"):
print(f" {needle}: {'yes' if needle in infra_head else 'no'}")
print("Source validation markers mentioning additional fields:")
for line_no, line in enumerate(source.splitlines(), 1):
if ("additionalAPIServerIPs" in line or "additionalIngressIPs" in line) and "XValidation" in line:
print(f" {line_no}: {line.strip()}")
PY
printf '%s\n' '--- generated validation expressions in the default Infrastructure CRD ---'
rg -n -C 3 \
'additionalAPIServerIPs|additionalIngressIPs|spec\.controlPlaneTopology must match|self\.spec\.controlPlaneTopology|self\.status\.controlPlaneTopology' \
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_infrastructures-Default.crd.yamlRepository: openshift/api Length of output: 3352 Enforce the
Add an 🤖 Prompt for AI Agents |
||
| // machineNetworks are IP networks used to connect all the OpenShift cluster | ||
| // nodes. Each network is provided in the CIDR format and should be IPv4 or IPv6, | ||
| // for example "10.0.0.0/8" or "fd00::/8". | ||
|
|
@@ -1248,6 +1270,30 @@ type OpenStackPlatformStatus struct { | |
| // +listType=atomic | ||
| IngressIPs []string `json:"ingressIPs"` | ||
|
|
||
| // additionalAPIServerIPs is a list of additional IP addresses to contact | ||
| // the Kubernetes API server on separate networks. These are used when a | ||
| // user-managed load balancer exposes the API server on multiple networks | ||
| // simultaneously. Each entry must be a valid IP address. This field is | ||
| // only valid when loadBalancer.type is set to UserManaged. | ||
| // | ||
| // +kubebuilder:validation:Format=ip | ||
| // +kubebuilder:validation:MaxItems=10 | ||
| // +listType=atomic | ||
| // +optional | ||
| AdditionalAPIServerIPs []string `json:"additionalAPIServerIPs,omitempty"` | ||
|
|
||
| // additionalIngressIPs is a list of additional IP addresses that route to | ||
| // the default ingress controller on separate networks. These are used when | ||
| // a user-managed load balancer exposes the ingress controller on multiple | ||
| // networks simultaneously. Each entry must be a valid IP address. This | ||
| // field is only valid when loadBalancer.type is set to UserManaged. | ||
| // | ||
| // +kubebuilder:validation:Format=ip | ||
| // +kubebuilder:validation:MaxItems=10 | ||
| // +listType=atomic | ||
| // +optional | ||
| AdditionalIngressIPs []string `json:"additionalIngressIPs,omitempty"` | ||
|
|
||
|
Comment on lines
+1273
to
+1296
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win Add CEL enforcement for the "loadBalancer.type" constraint on the status fields. Unlike the spec fields, Add a similar 🤖 Prompt for AI Agents |
||
| // nodeDNSIP is the IP address for the internal DNS used by the | ||
| // nodes. Unlike the one managed by the DNS operator, `NodeDNSIP` | ||
| // provides name resolution for the nodes themselves. There is no DNS-as-a-service for | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3. Missing xvalidation for loadbalancer.type
📜 Skill insight≡ CorrectnessAgent Prompt
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools