Skip to content
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

OCPBUGS-48469: GCP: Update /etc/hosts file when ClusterHostedDNS is enabled #4800

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 28 additions & 0 deletions pkg/controller/template/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,7 @@ func renderTemplate(config RenderConfig, path string, b []byte) ([]byte, error)
funcs["cloudPlatformAPIIntLoadBalancerIPs"] = cloudPlatformAPIIntLoadBalancerIPs
funcs["cloudPlatformAPILoadBalancerIPs"] = cloudPlatformAPILoadBalancerIPs
funcs["cloudPlatformIngressLoadBalancerIPs"] = cloudPlatformIngressLoadBalancerIPs
funcs["cloudPlatformLBIPAvailable"] = cloudPlatformLBIPAvailable
funcs["join"] = strings.Join
tmpl, err := template.New(path).Funcs(funcs).Parse(string(b))
if err != nil {
Expand Down Expand Up @@ -777,6 +778,33 @@ func cloudPlatformIngressLoadBalancerIPs(cfg RenderConfig) (interface{}, error)
}
}

// cloudPlatformLBIPAvailable returns true when DNSType is set to `ClusterHosted`
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious, based on the comment I'd expect some check for clusterhosted in the function. I guess it's implicit since the service enablement is dependent on this field?

(I know we do the same elsewhere in the template rendering, so I'm fine with it as is)

// and LB IPs are provided as part of `PlatformStatus`.
func cloudPlatformLBIPAvailable(cfg RenderConfig) bool {
if cfg.Infra.Status.PlatformStatus != nil {
switch cfg.Infra.Status.PlatformStatus.Type {
case configv1.GCPPlatformType:
switch cloudPlatformLoadBalancerIPState(cfg) {
case availableLBIPState:
return true
default:
return false
}
case configv1.AWSPlatformType:
switch cloudPlatformLoadBalancerIPState(cfg) {
case availableLBIPState:
return true
default:
return false
}
default:
return false
}
} else {
return false
}
}

// cloudPlatformLoadBalancerIPState is a helper function that determines if
// LoadBalancer config has been set.
func cloudPlatformLoadBalancerIPState(cfg RenderConfig) LoadBalancerIPState {
Expand Down
2 changes: 2 additions & 0 deletions pkg/controller/template/render_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,8 @@ var (
"nutanix": "./test_data/controller_config_nutanix.yaml",
"network-forwarding-sdn": "./test_data/controller_config_forwarding_sdn.yaml",
"network-forwarding-ovn": "./test_data/controller_config_forwarding_ovn.yaml",
"gcp-custom-dns": "./test_data/controller_config_gcp_custom_dns.yaml",
"gcp-platform-default": "./test_data/controller_config_gcp_platform_default.yaml",
}
)

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
apiVersion: "machineconfigurations.openshift.io/v1"
kind: "ControllerConfig"
spec:
clusterDNSIP: "10.3.0.10"
cloudProviderConfig: ""
etcdInitialCount: 3
etcdCAData: ZHVtbXkgZXRjZC1jYQo=
rootCAData: ZHVtbXkgcm9vdC1jYQo=
pullSecret:
data: ZHVtbXkgZXRjZC1jYQo=
images:
etcd: image/etcd:1
setupEtcdEnv: image/setupEtcdEnv:1
infraImage: image/infraImage:1
kubeClientAgentImage: image/kubeClientAgentImage:1
infra:
apiVersion: config.openshift.io/v1
kind: Infrastructure
status:
apiServerInternalURI: https://api-int.my-test-cluster.installer.team.coreos.systems:6443
apiServerURL: https://api.my-test-cluster.installer.team.coreos.systems:6443
etcdDiscoveryDomain: my-test-cluster.installer.team.coreos.systems
infrastructureName: my-test-cluster
platformStatus:
type: "GCP"
gcp:
cloudLoadBalancerConfig:
dnsType: "ClusterHosted"
clusterHosted:
apiLoadBalancerIPs:
- 20.20.20.20
apiIntLoadBalancerIPs:
- 10.10.10.10
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
apiVersion: "machineconfigurations.openshift.io/v1"
kind: "ControllerConfig"
spec:
clusterDNSIP: "10.3.0.10"
cloudProviderConfig: ""
etcdInitialCount: 3
etcdCAData: ZHVtbXkgZXRjZC1jYQo=
rootCAData: ZHVtbXkgcm9vdC1jYQo=
pullSecret:
data: ZHVtbXkgZXRjZC1jYQo=
images:
etcd: image/etcd:1
setupEtcdEnv: image/setupEtcdEnv:1
infraImage: image/infraImage:1
kubeClientAgentImage: image/kubeClientAgentImage:1
infra:
apiVersion: config.openshift.io/v1
kind: Infrastructure
status:
apiServerInternalURI: https://api-int.my-test-cluster.installer.team.coreos.systems:6443
apiServerURL: https://api.my-test-cluster.installer.team.coreos.systems:6443
etcdDiscoveryDomain: my-test-cluster.installer.team.coreos.systems
infrastructureName: my-test-cluster
platformStatus:
type: "GCP"
gcp:
cloudLoadBalancerConfig:
dnsType: "PlatformDefault"
28 changes: 28 additions & 0 deletions pkg/operator/render.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ func (a *assetRenderer) addTemplateFuncs() {
funcs["cloudPlatformAPIIntLoadBalancerIPs"] = cloudPlatformAPIIntLoadBalancerIPs
funcs["cloudPlatformAPILoadBalancerIPs"] = cloudPlatformAPILoadBalancerIPs
funcs["cloudPlatformIngressLoadBalancerIPs"] = cloudPlatformIngressLoadBalancerIPs
funcs["cloudPlatformLBIPAvailable"] = cloudPlatformLBIPAvailable
funcs["join"] = strings.Join

a.tmpl = a.tmpl.Funcs(funcs)
Expand Down Expand Up @@ -461,6 +462,33 @@ func cloudPlatformIngressLoadBalancerIPs(cfg mcfgv1.ControllerConfigSpec) (inter
}
}

// cloudPlatformLBIPAvailable returns true when DNSType is set to `ClusterHosted`
// and LB IPs are provided as part of `PlatformStatus`.
func cloudPlatformLBIPAvailable(cfg mcfgv1.ControllerConfigSpec) bool {
if cfg.Infra.Status.PlatformStatus != nil {
switch cfg.Infra.Status.PlatformStatus.Type {
case configv1.GCPPlatformType:
switch cloudPlatformLoadBalancerIPState(cfg) {
case availableLBIPState:
return true
default:
return false
}
case configv1.AWSPlatformType:
switch cloudPlatformLoadBalancerIPState(cfg) {
case availableLBIPState:
return true
default:
return false
}
default:
return false
}
} else {
return false
}
}

// cloudPlatformLoadBalancerIPState is a helper function that determines if
// LoadBalancer config has been set.
func cloudPlatformLoadBalancerIPState(cfg mcfgv1.ControllerConfigSpec) LoadBalancerIPState {
Expand Down
20 changes: 20 additions & 0 deletions templates/common/gcp/files/usr-local-bin-update-etc-hosts.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
mode: 0755
path: "/usr/local/bin/update-etc-hosts"
contents:
Copy link
Member

Choose a reason for hiding this comment

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

Potentially a simpler way, avoiding the script and systemd service:

path: "/etc/hosts"
append:
  - inline: |
    {{ if and (eq .Infra.Status.PlatformStatus.Type "GCP") (.Infra.Status.PlatformStatus.GCP) (.Infra.Status.PlatformStatus.GCP.CloudLoadBalancerConfig) (eq .Infra.Status.PlatformStatus.GCP.CloudLoadBalancerConfig.DNSType "ClusterHosted") }}
    {{ $apiIntLBIPs := cloudPlatformAPIIntLoadBalancerIPs . }}
    {{ if len $apiIntLBIPs > 0 }}
    {{ $apiLBIPs := cloudPlatformAPILoadBalancerIPs . }}
    {{ if len $apiLBIPs > 0 }}{{ $apiLBIPs[0] }}{{ else }}{{ $apiIntLBIPs[0] }}{{ end }} {{ .Infra.Status.APIServerURL }}
    {{ $apiIntLBIPs[0] }} {{ .Infra.Status.APIServerInternalURL }}
    {{ end }}
    {{ end }}

Copy link
Contributor Author

@sadasu sadasu Jan 27, 2025

Choose a reason for hiding this comment

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

The systemd service is providing us a way to time the running of this script before kubelet.

inline: |
#!/bin/bash
apiLBIP=${1}
apiURL=${2}
apiIntLBIP=${3}
apiIntURL=${4}
if [ -z "$apiLBIP" ]; then
# apiLBIPs are not expected to be set on private clusters
apiLBIP=$apiIntLBIP
fi
mkdir -p /etc/conf.d
etc_hosts_config_filename="/etc/conf.d/etc-hosts.conf"
echo "${apiLBIP} ${apiURL%:*}" >> ${etc_hosts_config_filename}
echo "${apiIntLBIP} ${apiIntURL%:*}" >> ${etc_hosts_config_filename}
cat /etc/conf.d/etc-hosts.conf
cat /etc/conf.d/etc-hosts.conf >> /etc/hosts
echo "Done updating /etc/hosts"
28 changes: 28 additions & 0 deletions templates/common/gcp/units/gcp-update-etc-hosts.service.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
name: gcp-update-etc-hosts.service
enabled: {{if and (eq .Infra.Status.PlatformStatus.Type "GCP") (.Infra.Status.PlatformStatus.GCP) (.Infra.Status.PlatformStatus.GCP.CloudLoadBalancerConfig) (eq .Infra.Status.PlatformStatus.GCP.CloudLoadBalancerConfig.DNSType "ClusterHosted") }}true{{else}}false{{end}}
contents: |
[Unit]
Description=Update Default GCP /etc/hosts
# We don't need to do this on the firstboot
After=firstboot-osupdate.target
# Wait for NetworkManager to report it's online
After=NetworkManager-wait-online.service
# Run before kubelet
Before=kubelet-dependencies.target

[Service]
# Need oneshot to delay kubelet
Type=oneshot
ExecStart=/bin/bash -c " \
{{ if and (cloudPlatformLBIPAvailable .) (gt (len (cloudPlatformAPIIntLoadBalancerIPs .)) 0) }} \
apiIntLBIP={{ index (cloudPlatformAPIIntLoadBalancerIPs .) 0 }} \
{{ end }} \
{{ if and (cloudPlatformLBIPAvailable .) (gt (len (cloudPlatformAPILoadBalancerIPs .)) 0) }} \
apiLBIP={{ index (cloudPlatformAPILoadBalancerIPs .) 0 }} \
{{ end }} \
apiServerURL={{ .Infra.Status.APIServerURL }} \
apiServerIntURL={{ .Infra.Status.APIServerInternalURL }} \
/usr/local/bin/update-etc-hosts ${apiLBIP} ${apiServerURL} ${apiIntLBIP} ${apiServerIntURL}"

[Install]
RequiredBy=kubelet-dependencies.target