Add internalDNSRecords field #2460
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -183,6 +183,13 @@ const ( | |
| LoadBalancerTypeOpenShiftManagedDefault PlatformLoadBalancerType = "OpenShiftManagedDefault" | ||
| ) | ||
|
|
||
| type InternalDNSRecordsType string | ||
|
|
||
| const ( | ||
| InternalDNSRecordsDisabled InternalDNSRecordsType = "Disabled" | ||
| InternalDNSRecordsEnabled InternalDNSRecordsType = "Enabled" | ||
| ) | ||
|
|
||
| // 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 | ||
|
|
@@ -1026,6 +1033,15 @@ type BareMetalPlatformStatus struct { | |
| // +optional | ||
| LoadBalancer *BareMetalPlatformLoadBalancer `json:"loadBalancer,omitempty"` | ||
|
|
||
| // internalDNSRecords determines whether we deploy with internal records enabled for | ||
| // api, api-int, and ingress. | ||
|
Comment on lines
+1036
to
+1037
There was a problem hiding this comment. Please include validation constraints in the GoDoc here so that this is more end-user friendly. This is the text used in our generated API documentation and what users will see when they use something like Some good guidelines for things to take into consideration for inclusion in the GoDoc are here: https://github.com/openshift/enhancements/blob/master/dev-guide/api-conventions.md#write-user-readable-documentation-in-godoc |
||
| // +kubebuilder:validation:Optional | ||
|
There was a problem hiding this comment. This is essentially a duplicate of the |
||
| // +kubebuilder:validation:Enum=Enabled;Disabled | ||
| // +openshift:validation:featureGate=OnPremInternalDNSRecords | ||
|
There was a problem hiding this comment. AFAIK, this isn't a real marker. Let's remove this. There was a problem hiding this comment. Heh, yeah I was throwing stuff at the wall when I had trouble with the feature gate. |
||
| // +openshift:enable:FeatureGate=OnPremInternalDNSRecords | ||
| // +optional | ||
| InternalDNSRecords InternalDNSRecordsType `json:"internalDNSRecords"` | ||
|
There was a problem hiding this comment. Because this is an optional field and the zero value is invalid, this should have |
||
|
|
||
|
Comment on lines
+1036
to
+1044
There was a problem hiding this comment. This looks like we have only added this to the There was a problem hiding this comment. If I understand the EP correctly as well, this sounds like this should only be possible to set when Do we need some additional validation logic (maybe a CEL expression) to enforce that? There was a problem hiding this comment. Yes, this will be populated by the installer. I have a validation in the installer to ensure it isn't set when it shouldn't be, but I can move that here if it would be better. I should also note that this is only a partial version of the change. Because these are per-platform types we'll need to apply the same change to the other on-prem platforms once we know what it should look like. |
||
| // machineNetworks are IP networks used to connect all the OpenShift cluster nodes. | ||
| // +listType=atomic | ||
| // +kubebuilder:validation:MaxItems=32 | ||
|
|
||
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.
Normally we try to avoid the terminology
EnabledandDisabledwhere possible because the terms can often be overloaded and cause confusion.What if instead of naming the field this applies to
internalDNSRecords, what if we named it something likednsRecordsPolicy(or maybednsRecordsType? not sure which one is better) and we hadInternalandExternalas the options?