Skip to content

Commit fbd4bc5

Browse files
peterhonederraoulbhatia
authored andcommitted
fixed review comments
- incorporated changes from PR #3468 to allow for arbitrary annotations to be used no matter the hostNetwork property's value - updated docs to make clear how the annotations are used for pods
1 parent 365c0b0 commit fbd4bc5

File tree

4 files changed

+59
-48
lines changed

4 files changed

+59
-48
lines changed

docs/annotations/annotations.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,8 @@ Specifies the domain for the resource's DNS records.
6464
Multiple hostnames can be specified through a comma-separated list, e.g.
6565
`svc.mydomain1.com,svc.mydomain2.com`.
6666

67+
For `Pods`, uses the `Pod`'s `Status.PodIP`, unless they are `hostNetwork: true` in which case the NodeExternalIP is used for IPv4 and NodeInternalIP for IPv6.
68+
6769
## external-dns.alpha.kubernetes.io/ingress-hostname-source
6870

6971
Specifies where to get the domain for an `Ingress` resource.
@@ -80,7 +82,7 @@ Specifies the domain for the resource's DNS records that are for use from intern
8082

8183
For `Services` of type `LoadBalancer`, uses the `Service`'s `ClusterIP`.
8284

83-
For `Pods`, uses the `Pod`'s `Status.PodIP`.
85+
For `Pods`, uses the `Pod`'s `Status.PodIP`, unless they are `hostNetwork: true` in which case the NodeExternalIP is used for IPv4 and NodeInternalIP for IPv6.
8486

8587
## external-dns.alpha.kubernetes.io/target
8688

docs/tutorials/kops-dns-controller.md

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,9 @@ The DNS record mappings try to "do the right thing", but what this means is diff
2121

2222
### Pods
2323

24-
For the external annotation, ExternalDNS will map a HostNetwork=true Pod to the external IPs of the Node.
24+
For the external annotation, ExternalDNS will map a Pod to the external IPs of the Node.
2525

26-
For the internal annotation, ExternalDNS will map a HostNetwork=true Pod to the internal IPs of the Node.
27-
28-
ExternalDNS ignore Pods that are not HostNetwork=true
26+
For the internal annotation, ExternalDNS will map a Pod to the internal IPs of the Node.
2927

3028
Annotations added to Pods will always result in an A record being created.
3129

source/pod.go

Lines changed: 39 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ func (ps *podSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, error
8585
for _, pod := range pods {
8686
targets := getTargetsFromTargetAnnotation(pod.Annotations)
8787

88-
// accept hostname annotations that point to the pod's IP, no matter if the pod spec uses the host network
88+
// accept internal hostname annotations that point to the pod's IP
8989
if domainAnnotation, ok := pod.Annotations[internalHostnameAnnotationKey]; ok {
9090
domainList := splitHostnameAnnotation(domainAnnotation)
9191
for _, domain := range domainList {
@@ -99,46 +99,44 @@ func (ps *podSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, error
9999
}
100100
}
101101

102-
// accept annotations that point to the node's external IP (or IPv6 NodeInternalIP) only for pods using the host network
103-
if pod.Spec.HostNetwork {
104-
if domainAnnotation, ok := pod.Annotations[hostnameAnnotationKey]; ok {
105-
domainList := splitHostnameAnnotation(domainAnnotation)
106-
for _, domain := range domainList {
107-
if len(targets) == 0 {
108-
node, _ := ps.nodeInformer.Lister().Get(pod.Spec.NodeName)
109-
for _, address := range node.Status.Addresses {
110-
recordType := suitableType(address.Address)
111-
// IPv6 addresses are labeled as NodeInternalIP despite being usable externally as well.
112-
if address.Type == corev1.NodeExternalIP || (address.Type == corev1.NodeInternalIP && recordType == endpoint.RecordTypeAAAA) {
113-
addToEndpointMap(endpointMap, domain, recordType, address.Address)
114-
}
115-
}
116-
} else {
117-
for _, target := range targets {
118-
addToEndpointMap(endpointMap, domain, suitableType(target), target)
102+
// accept internal hostname annotations that point to the pod's IP (or IPv6 NodeInternalIP)
103+
if domainAnnotation, ok := pod.Annotations[hostnameAnnotationKey]; ok {
104+
domainList := splitHostnameAnnotation(domainAnnotation)
105+
for _, domain := range domainList {
106+
if len(targets) == 0 {
107+
node, _ := ps.nodeInformer.Lister().Get(pod.Spec.NodeName)
108+
for _, address := range node.Status.Addresses {
109+
recordType := suitableType(address.Address)
110+
// IPv6 addresses are labeled as NodeInternalIP despite being usable externally as well.
111+
if address.Type == corev1.NodeExternalIP || (address.Type == corev1.NodeInternalIP && recordType == endpoint.RecordTypeAAAA) {
112+
addToEndpointMap(endpointMap, domain, recordType, address.Address)
119113
}
120114
}
115+
} else {
116+
for _, target := range targets {
117+
addToEndpointMap(endpointMap, domain, suitableType(target), target)
118+
}
121119
}
122120
}
121+
}
123122

124-
if ps.compatibility == "kops-dns-controller" {
125-
if domainAnnotation, ok := pod.Annotations[kopsDNSControllerInternalHostnameAnnotationKey]; ok {
126-
domainList := splitHostnameAnnotation(domainAnnotation)
127-
for _, domain := range domainList {
128-
addToEndpointMap(endpointMap, domain, suitableType(pod.Status.PodIP), pod.Status.PodIP)
129-
}
123+
if ps.compatibility == "kops-dns-controller" {
124+
if domainAnnotation, ok := pod.Annotations[kopsDNSControllerInternalHostnameAnnotationKey]; ok {
125+
domainList := splitHostnameAnnotation(domainAnnotation)
126+
for _, domain := range domainList {
127+
addToEndpointMap(endpointMap, domain, suitableType(pod.Status.PodIP), pod.Status.PodIP)
130128
}
129+
}
131130

132-
if domainAnnotation, ok := pod.Annotations[kopsDNSControllerHostnameAnnotationKey]; ok {
133-
domainList := splitHostnameAnnotation(domainAnnotation)
134-
for _, domain := range domainList {
135-
node, _ := ps.nodeInformer.Lister().Get(pod.Spec.NodeName)
136-
for _, address := range node.Status.Addresses {
137-
recordType := suitableType(address.Address)
138-
// IPv6 addresses are labeled as NodeInternalIP despite being usable externally as well.
139-
if address.Type == corev1.NodeExternalIP || (address.Type == corev1.NodeInternalIP && recordType == endpoint.RecordTypeAAAA) {
140-
addToEndpointMap(endpointMap, domain, recordType, address.Address)
141-
}
131+
if domainAnnotation, ok := pod.Annotations[kopsDNSControllerHostnameAnnotationKey]; ok {
132+
domainList := splitHostnameAnnotation(domainAnnotation)
133+
for _, domain := range domainList {
134+
node, _ := ps.nodeInformer.Lister().Get(pod.Spec.NodeName)
135+
for _, address := range node.Status.Addresses {
136+
recordType := suitableType(address.Address)
137+
// IPv6 addresses are labeled as NodeInternalIP despite being usable externally as well.
138+
if address.Type == corev1.NodeExternalIP || (address.Type == corev1.NodeInternalIP && recordType == endpoint.RecordTypeAAAA) {
139+
addToEndpointMap(endpointMap, domain, recordType, address.Address)
142140
}
143141
}
144142
}
@@ -160,5 +158,12 @@ func addToEndpointMap(endpointMap map[endpoint.EndpointKey][]string, domain stri
160158
if _, ok := endpointMap[key]; !ok {
161159
endpointMap[key] = []string{}
162160
}
161+
162+
// check that only unique addresses are added
163+
for _, existingAddress := range endpointMap[key] {
164+
if existingAddress == address {
165+
return
166+
}
167+
}
163168
endpointMap[key] = append(endpointMap[key], address)
164169
}

source/pod_test.go

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ func TestPodSource(t *testing.T) {
116116
"kops-dns-controller",
117117
[]*endpoint.Endpoint{
118118
{DNSName: "a.foo.example.org", Targets: endpoint.Targets{"54.10.11.1", "54.10.11.2"}, RecordType: endpoint.RecordTypeA},
119-
{DNSName: "internal.a.foo.example.org", Targets: endpoint.Targets{"10.0.1.1", "10.0.1.2"}, RecordType: endpoint.RecordTypeA},
119+
{DNSName: "internal.a.foo.example.org", Targets: endpoint.Targets{"10.0.1.1", "10.0.1.2", "10.0.1.3"}, RecordType: endpoint.RecordTypeA},
120120
},
121121
false,
122122
[]*corev1.Node{
@@ -178,7 +178,9 @@ func TestPodSource(t *testing.T) {
178178
PodIP: "10.0.1.2",
179179
},
180180
},
181-
// this pod must be ignored because it's not in the host network and kops requires this
181+
// non-HostNetwork pod
182+
// - internal hostname annotation will use the PodIP
183+
// - external hostname annotation will use the NodeExternalIP
182184
{
183185
ObjectMeta: metav1.ObjectMeta{
184186
Name: "my-pod3",
@@ -264,7 +266,9 @@ func TestPodSource(t *testing.T) {
264266
PodIP: "2001:DB8::2",
265267
},
266268
},
267-
// this pod's internal hostname annotation must not be ignored even though it's not in the host network
269+
// this pod's hostname annotations (both internal and not) must not be ignored even though it's not in the host network
270+
// - internal hostname annotation uses the PodIP
271+
// - external hostname annotation uses the NodeInternalIP
268272
{
269273
ObjectMeta: metav1.ObjectMeta{
270274
Name: "my-pod3",
@@ -290,7 +294,7 @@ func TestPodSource(t *testing.T) {
290294
"kops-dns-controller",
291295
[]*endpoint.Endpoint{
292296
{DNSName: "a.foo.example.org", Targets: endpoint.Targets{"2001:DB8::1", "2001:DB8::2"}, RecordType: endpoint.RecordTypeAAAA},
293-
{DNSName: "internal.a.foo.example.org", Targets: endpoint.Targets{"2001:DB8::1", "2001:DB8::2"}, RecordType: endpoint.RecordTypeAAAA},
297+
{DNSName: "internal.a.foo.example.org", Targets: endpoint.Targets{"2001:DB8::1", "2001:DB8::2", "2001:DB8::3"}, RecordType: endpoint.RecordTypeAAAA},
294298
},
295299
false,
296300
[]*corev1.Node{
@@ -350,7 +354,9 @@ func TestPodSource(t *testing.T) {
350354
PodIP: "2001:DB8::2",
351355
},
352356
},
353-
// this pod must be ignored because it's not in the host network and kops requires this (no matter the type of hostname annotation)
357+
// this pod's hostname annotations (both internal and not) must not be ignored even though it's not in the host network
358+
// - internal hostname annotation uses the PodIP
359+
// - external hostname annotation uses the NodeInternalIP
354360
{
355361
ObjectMeta: metav1.ObjectMeta{
356362
Name: "my-pod3",
@@ -375,7 +381,7 @@ func TestPodSource(t *testing.T) {
375381
"",
376382
"",
377383
[]*endpoint.Endpoint{
378-
{DNSName: "a.foo.example.org", Targets: endpoint.Targets{"208.1.2.1", "208.1.2.2"}, RecordType: endpoint.RecordTypeA},
384+
{DNSName: "a.foo.example.org", Targets: endpoint.Targets{"208.1.2.1", "208.1.2.2", "208.1.2.3"}, RecordType: endpoint.RecordTypeA},
379385
{DNSName: "internal.a.foo.example.org", Targets: endpoint.Targets{"208.1.2.1", "208.1.2.2", "208.1.2.3"}, RecordType: endpoint.RecordTypeA},
380386
},
381387
false,
@@ -440,7 +446,7 @@ func TestPodSource(t *testing.T) {
440446
PodIP: "10.0.1.2",
441447
},
442448
},
443-
// this pod's internal hostname annotation must not be ignored even though it's not in the host network (even when using a custom target annotation)
449+
// test non-HostNetwork pod
444450
{
445451
ObjectMeta: metav1.ObjectMeta{
446452
Name: "my-pod3",
@@ -532,11 +538,11 @@ func TestPodSource(t *testing.T) {
532538
},
533539
},
534540
{
535-
"internal hostname annotations of pods with hostNetwore=false should still be added as valid records",
541+
"hostname annotations of pods with hostNetwork=false should still be added as valid records",
536542
"",
537543
"",
538544
[]*endpoint.Endpoint{
539-
{DNSName: "a.foo.example.org", Targets: endpoint.Targets{"54.10.11.1"}, RecordType: endpoint.RecordTypeA},
545+
{DNSName: "a.foo.example.org", Targets: endpoint.Targets{"54.10.11.1", "54.10.11.2"}, RecordType: endpoint.RecordTypeA},
540546
{DNSName: "internal.a.foo.example.org", Targets: endpoint.Targets{"10.0.1.1", "100.0.1.2"}, RecordType: endpoint.RecordTypeA},
541547
// this annotation is part of a comma separated hostname list in the annotation
542548
{DNSName: "internal.b.foo.example.org", Targets: endpoint.Targets{"10.0.1.1"}, RecordType: endpoint.RecordTypeA},

0 commit comments

Comments
 (0)