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

feat(source): optional exclusion of unschedulable nodes #5045

Open
wants to merge 4 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
1 change: 1 addition & 0 deletions docs/flags.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
| `--[no-]traefik-disable-legacy` | Disable listeners on Resources under the traefik.containo.us API Group |
| `--[no-]traefik-disable-new` | Disable listeners on Resources under the traefik.io API Group |
| `--nat64-networks=NAT64-NETWORKS` | Adding an A record for each AAAA record in NAT64-enabled networks; specify multiple times for multiple possible nets (optional) |
| `--[no-]exclude-unschedulable` | Exclude nodes that are considered unschedulable (default: true) |
| `--[no-]expose-internal-ipv6` | When using the node source, expose internal IPv6 addresses (optional). Default is true. |
| `--provider=provider` | The DNS provider where the DNS records will be created (required, options: akamai, alibabacloud, aws, aws-sd, azure, azure-dns, azure-private-dns, civo, cloudflare, coredns, digitalocean, dnsimple, exoscale, gandi, godaddy, google, ibmcloud, inmemory, linode, ns1, oci, ovh, pdns, pihole, plural, rfc2136, scaleway, skydns, tencentcloud, transip, ultradns, webhook) |
| `--provider-cache-time=0s` | The time to cache the DNS provider record list requests. |
Expand Down
5 changes: 3 additions & 2 deletions docs/sources/nodes.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ The node source adds an `A` record per each node `externalIP` (if not found, any
It also adds an `AAAA` record per each node IPv6 `internalIP`. Refer to the [IPv6 Behavior](#ipv6-behavior) section for more details.
The TTL of the records can be set with the `external-dns.alpha.kubernetes.io/ttl` node annotation.

Nodes marked as **Unschedulable** as per [core/v1/NodeSpec](https://pkg.go.dev/k8s.io/[email protected]/core/v1#NodeSpec) are excluded.
This avoid exposing Unhealthy, NotReady or SchedulingDisabled (cordon) nodes.
Nodes marked as **Unschedulable** as per [core/v1/NodeSpec](https://pkg.go.dev/k8s.io/[email protected]/core/v1#NodeSpec) are excluded by default.
As such, no DNS records are created for Unhealthy, NotReady or SchedulingDisabled (cordon) nodes (and existing ones are removed).
In case you want to override the default, for example if you manage per-host DNS records via ExternalDNS, you can specify `--no-exclude-unschedulable` to always expose nodes no matter their status.

## IPv6 Behavior

Expand Down
1 change: 1 addition & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ func main() {
ResolveLoadBalancerHostname: cfg.ResolveServiceLoadBalancerHostname,
TraefikDisableLegacy: cfg.TraefikDisableLegacy,
TraefikDisableNew: cfg.TraefikDisableNew,
ExcludeUnschedulable: cfg.ExcludeUnschedulable,
ExposeInternalIPv6: cfg.ExposeInternalIPV6,
}

Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/externaldns/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ type Config struct {
TraefikDisableLegacy bool
TraefikDisableNew bool
NAT64Networks []string
ExcludeUnschedulable bool
}

var defaultConfig = &Config{
Expand Down Expand Up @@ -376,6 +377,7 @@ var defaultConfig = &Config{
TraefikDisableLegacy: false,
TraefikDisableNew: false,
NAT64Networks: []string{},
ExcludeUnschedulable: true,
}

// NewConfig returns new Config object
Expand Down Expand Up @@ -483,6 +485,7 @@ func App(cfg *Config) *kingpin.Application {
app.Flag("traefik-disable-legacy", "Disable listeners on Resources under the traefik.containo.us API Group").Default(strconv.FormatBool(defaultConfig.TraefikDisableLegacy)).BoolVar(&cfg.TraefikDisableLegacy)
app.Flag("traefik-disable-new", "Disable listeners on Resources under the traefik.io API Group").Default(strconv.FormatBool(defaultConfig.TraefikDisableNew)).BoolVar(&cfg.TraefikDisableNew)
app.Flag("nat64-networks", "Adding an A record for each AAAA record in NAT64-enabled networks; specify multiple times for multiple possible nets (optional)").StringsVar(&cfg.NAT64Networks)
app.Flag("exclude-unschedulable", "Exclude nodes that are considered unschedulable (default: true)").Default(strconv.FormatBool(defaultConfig.ExcludeUnschedulable)).BoolVar(&cfg.ExcludeUnschedulable)
app.Flag("expose-internal-ipv6", "When using the node source, expose internal IPv6 addresses (optional). Default is true.").BoolVar(&cfg.ExposeInternalIPV6)

// Flags related to providers
Expand Down
4 changes: 4 additions & 0 deletions pkg/apis/externaldns/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ var (
WebhookProviderURL: "http://localhost:8888",
WebhookProviderReadTimeout: 5 * time.Second,
WebhookProviderWriteTimeout: 10 * time.Second,
ExcludeUnschedulable: true,
}

overriddenConfig = &Config{
Expand Down Expand Up @@ -245,6 +246,7 @@ var (
WebhookProviderURL: "http://localhost:8888",
WebhookProviderReadTimeout: 5 * time.Second,
WebhookProviderWriteTimeout: 10 * time.Second,
ExcludeUnschedulable: false,
}
)

Expand Down Expand Up @@ -383,6 +385,7 @@ func TestParseFlags(t *testing.T) {
"--managed-record-types=AAAA",
"--managed-record-types=CNAME",
"--managed-record-types=NS",
"--no-exclude-unschedulable",
"--rfc2136-batch-change-size=100",
"--rfc2136-load-balancing-strategy=round-robin",
"--rfc2136-host=rfc2136-host1",
Expand Down Expand Up @@ -501,6 +504,7 @@ func TestParseFlags(t *testing.T) {
"EXTERNAL_DNS_TRANSIP_KEYFILE": "/path/to/transip.key",
"EXTERNAL_DNS_DIGITALOCEAN_API_PAGE_SIZE": "100",
"EXTERNAL_DNS_MANAGED_RECORD_TYPES": "A\nAAAA\nCNAME\nNS",
"EXTERNAL_DNS_EXCLUDE_UNSCHEDULABLE": "false",
"EXTERNAL_DNS_RFC2136_BATCH_CHANGE_SIZE": "100",
"EXTERNAL_DNS_RFC2136_LOAD_BALANCING_STRATEGY": "round-robin",
"EXTERNAL_DNS_RFC2136_HOST": "rfc2136-host1\nrfc2136-host2",
Expand Down
30 changes: 16 additions & 14 deletions source/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,17 @@ import (
)

type nodeSource struct {
client kubernetes.Interface
annotationFilter string
fqdnTemplate *template.Template
nodeInformer coreinformers.NodeInformer
labelSelector labels.Selector
exposeInternalIPV6 bool
client kubernetes.Interface
annotationFilter string
fqdnTemplate *template.Template
nodeInformer coreinformers.NodeInformer
labelSelector labels.Selector
excludeUnschedulable bool
exposeInternalIPV6 bool
}

// NewNodeSource creates a new nodeSource with the given config.
func NewNodeSource(ctx context.Context, kubeClient kubernetes.Interface, annotationFilter, fqdnTemplate string, labelSelector labels.Selector, exposeInternalIPv6 bool) (Source, error) {
func NewNodeSource(ctx context.Context, kubeClient kubernetes.Interface, annotationFilter, fqdnTemplate string, labelSelector labels.Selector, exposeInternalIPv6 bool, excludeUnschedulable bool) (Source, error) {
tmpl, err := parseTemplate(fqdnTemplate)
if err != nil {
return nil, err
Expand Down Expand Up @@ -71,12 +72,13 @@ func NewNodeSource(ctx context.Context, kubeClient kubernetes.Interface, annotat
}

return &nodeSource{
client: kubeClient,
annotationFilter: annotationFilter,
fqdnTemplate: tmpl,
nodeInformer: nodeInformer,
labelSelector: labelSelector,
exposeInternalIPV6: exposeInternalIPv6,
client: kubeClient,
annotationFilter: annotationFilter,
fqdnTemplate: tmpl,
nodeInformer: nodeInformer,
labelSelector: labelSelector,
excludeUnschedulable: excludeUnschedulable,
exposeInternalIPV6: exposeInternalIPv6,
}, nil
}

Expand Down Expand Up @@ -104,7 +106,7 @@ func (ns *nodeSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, erro
continue
}

if node.Spec.Unschedulable {
if node.Spec.Unschedulable && ns.excludeUnschedulable {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add tests that capture debug logging?

func LogsToBuffer(level log.Level, t *testing.T) *bytes.Buffer {

You could search for example usages in the code.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry for the late response, but I've been pretty busy the last couple weeks.

I've now added an example of how this could be implemented in the existing test method (which would allow the other testcases to check for logs, as well. Not quite ideal, as this requires disabling parallelism for these testcases, but the alternative would be having to duplicate more of the test logic, which I'm not really a fan of, either.

log.Debugf("Skipping node %s because it is unschedulable", node.Name)
continue
}
Expand Down
100 changes: 70 additions & 30 deletions source/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package source

import (
"bytes"
"context"
log "github.com/sirupsen/logrus"
"sigs.k8s.io/external-dns/internal/testutils"
Expand Down Expand Up @@ -81,6 +82,7 @@ func testNodeSourceNewNodeSource(t *testing.T) {
ti.fqdnTemplate,
labels.Everything(),
true,
true,
)

if ti.expectError {
Expand All @@ -97,18 +99,21 @@ func testNodeSourceEndpoints(t *testing.T) {
t.Parallel()

for _, tc := range []struct {
title string
annotationFilter string
labelSelector string
fqdnTemplate string
nodeName string
nodeAddresses []v1.NodeAddress
labels map[string]string
annotations map[string]string
exposeInternalIPv6 bool // default to true for this version. Change later when the next minor version is released.
unschedulable bool // default to false
expected []*endpoint.Endpoint
expectError bool
title string
annotationFilter string
labelSelector string
fqdnTemplate string
nodeName string
nodeAddresses []v1.NodeAddress
labels map[string]string
annotations map[string]string
excludeUnschedulable bool // default to false
exposeInternalIPv6 bool // default to true for this version. Change later when the next minor version is released.
unschedulable bool // default to false
expected []*endpoint.Endpoint
expectError bool
expectedLogs []string
expectedAbsentLogs []string
}{
{
title: "node with short hostname returns one endpoint",
Expand Down Expand Up @@ -361,16 +366,40 @@ func testNodeSourceEndpoints(t *testing.T) {
},
},
{
title: "unschedulable node return nothing",
nodeName: "node1",
exposeInternalIPv6: true,
nodeAddresses: []v1.NodeAddress{{Type: v1.NodeExternalIP, Address: "1.2.3.4"}},
unschedulable: true,
expected: []*endpoint.Endpoint{},
title: "unschedulable node return nothing with excludeUnschedulable=true",
nodeName: "node1",
exposeInternalIPv6: true,
nodeAddresses: []v1.NodeAddress{{Type: v1.NodeExternalIP, Address: "1.2.3.4"}},
unschedulable: true,
excludeUnschedulable: true,
expected: []*endpoint.Endpoint{},
expectedLogs: []string{
"Skipping node node1 because it is unschedulable",
},
},
{
title: "unschedulable node returns node with excludeUnschedulable=false",
nodeName: "node1",
nodeAddresses: []v1.NodeAddress{{Type: v1.NodeExternalIP, Address: "1.2.3.4"}},
unschedulable: true,
excludeUnschedulable: false,
expected: []*endpoint.Endpoint{
{RecordType: "A", DNSName: "node1", Targets: endpoint.Targets{"1.2.3.4"}},
},
expectedAbsentLogs: []string{
"Skipping node node1 because it is unschedulable",
},
},
} {
tc := tc
t.Run(tc.title, func(t *testing.T) {
var buf *bytes.Buffer
if len(tc.expectedLogs) == 0 && len(tc.expectedAbsentLogs) == 0 {
t.Parallel()
} else {
buf = testutils.LogsToBuffer(log.DebugLevel, t)
}

labelSelector := labels.Everything()
if tc.labelSelector != "" {
var err error
Expand Down Expand Up @@ -406,6 +435,7 @@ func testNodeSourceEndpoints(t *testing.T) {
tc.fqdnTemplate,
labelSelector,
tc.exposeInternalIPv6,
tc.excludeUnschedulable,
)
require.NoError(t, err)

Expand All @@ -418,24 +448,33 @@ func testNodeSourceEndpoints(t *testing.T) {

// Validate returned endpoints against desired endpoints.
validateEndpoints(t, endpoints, tc.expected)

for _, entry := range tc.expectedLogs {
assert.Contains(t, buf.String(), entry)
}

for _, entry := range tc.expectedAbsentLogs {
assert.NotContains(t, buf.String(), entry)
}
})
}
}

func testNodeEndpointsWithIPv6(t *testing.T) {
for _, tc := range []struct {
title string
annotationFilter string
labelSelector string
fqdnTemplate string
nodeName string
nodeAddresses []v1.NodeAddress
labels map[string]string
annotations map[string]string
exposeInternalIPv6 bool // default to true for this version. Change later when the next minor version is released.
unschedulable bool // default to false
expected []*endpoint.Endpoint
expectError bool
title string
annotationFilter string
labelSelector string
fqdnTemplate string
nodeName string
nodeAddresses []v1.NodeAddress
labels map[string]string
annotations map[string]string
excludeUnschedulable bool // defaults to false
exposeInternalIPv6 bool // default to true for this version. Change later when the next minor version is released.
unschedulable bool // default to false
expected []*endpoint.Endpoint
expectError bool
}{
{
title: "node with only internal IPs should return internal IPvs irrespective of exposeInternalIPv6",
Expand Down Expand Up @@ -516,6 +555,7 @@ func testNodeEndpointsWithIPv6(t *testing.T) {
tc.fqdnTemplate,
labelSelector,
tc.exposeInternalIPv6,
tc.excludeUnschedulable,
)
require.NoError(t, err)

Expand Down
3 changes: 2 additions & 1 deletion source/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ type Config struct {
ResolveLoadBalancerHostname bool
TraefikDisableLegacy bool
TraefikDisableNew bool
ExcludeUnschedulable bool
ExposeInternalIPv6 bool
}

Expand Down Expand Up @@ -217,7 +218,7 @@ func BuildWithConfig(ctx context.Context, source string, p ClientGenerator, cfg
if err != nil {
return nil, err
}
return NewNodeSource(ctx, client, cfg.AnnotationFilter, cfg.FQDNTemplate, cfg.LabelFilter, cfg.ExposeInternalIPv6)
return NewNodeSource(ctx, client, cfg.AnnotationFilter, cfg.FQDNTemplate, cfg.LabelFilter, cfg.ExposeInternalIPv6, cfg.ExcludeUnschedulable)
case "service":
client, err := p.KubeClient()
if err != nil {
Expand Down
Loading