source node: Skip unschedulable nodes #4761

n-Arno · 2024-09-19T15:42:39Z

Skip unschedulable nodes to avoid adding nodes to a DNS round robin entry.

Added in source/node.go a simple test regarding Node.Spec.Unschedulable.

Fixes #ISSUE

Checklist

Unit tests updated
End user documentation updated

linux-foundation-easycla · 2024-09-19T15:42:44Z

The committers listed above are authorized under a signed CLA.

✅ login: n-Arno / name: Arnaud A (326ee0b)

k8s-ci-robot · 2024-09-19T15:42:48Z

Welcome @n-Arno!

It looks like this is your first PR to kubernetes-sigs/external-dns 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/external-dns has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2024-09-19T15:42:49Z

Hi @n-Arno. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

n-Arno · 2024-09-19T19:53:36Z

Added Unit test and comment in docs/sources/nodes.md

mloiseleur · 2024-09-20T06:12:50Z

/ok-to-test

mloiseleur · 2024-09-20T06:20:32Z

Thanks 👍, this make sense.
I'll be happy to lgtm this PR once the linter is fixed.

n-Arno · 2024-09-20T19:19:42Z

missing gofmt done + rebase in a single commit 😄

mloiseleur · 2024-09-22T14:08:19Z

/lgtm

szuecs · 2024-09-30T07:12:47Z

docs/sources/nodes.md

@@ -7,6 +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`.
 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/api@v0.31.1/core/v1#NodeSpec) are excluded.
+This avoid exposing Unhealthy, NotReady or SchedulingDisabled (cordon) nodes.


the referenced doc is more clear, citing also the default seems to be important:

// Unschedulable controls node schedulability of new pods. By default, node is schedulable. // More info: https://kubernetes.io/docs/concepts/nodes/node/#manual-node-administration // +optional Unschedulable [bool](https://pkg.go.dev/builtin#bool) `json:"unschedulable,omitempty" protobuf:"varint,4,opt,name=unschedulable"`

Thanks for the info, i'll add this 😄

szuecs · 2024-09-30T09:06:53Z

source/node.go

@@ -102,6 +102,11 @@ func (ns *nodeSource) Endpoints(ctx context.Context) ([]*endpoint.Endpoint, erro
 			continue
 		}

+		if node.Spec.Unschedulable {
+			log.Debugf("Skipping node %s because it is unschedulable", node.Name)


I wonder if this should not be more prominent than debug, maybe info?

I initially went for Debug since the other "skip" step also does it in Debug:

external-dns/source/node.go

Line 100 in 45257fb

log.Debugf("Skipping node %s because controller value does not match, found: %s, required: %s",

If Info is better, i'll go for this.

Let's have it like this

szuecs · 2024-10-10T13:55:58Z

/approve

k8s-ci-robot · 2024-10-10T13:56:06Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: szuecs

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [szuecs]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot requested review from mloiseleur and szuecs September 19, 2024 15:42

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Sep 19, 2024

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Sep 19, 2024

k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Sep 19, 2024

n-Arno mentioned this pull request Sep 19, 2024

Handling of Node NotReady #1112

Closed

n-Arno force-pushed the master branch from 00d25e1 to 1290922 Compare September 19, 2024 17:43

n-Arno force-pushed the master branch from 1d6be42 to fcfa271 Compare September 19, 2024 19:51

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Sep 19, 2024

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Sep 20, 2024

source node: Skip unschedulable nodes

326ee0b

n-Arno force-pushed the master branch from fcfa271 to 326ee0b Compare September 20, 2024 19:17

k8s-ci-robot assigned mloiseleur Sep 22, 2024

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 22, 2024

szuecs reviewed Sep 30, 2024

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 10, 2024

k8s-ci-robot merged commit 2b93938 into kubernetes-sigs:master Oct 10, 2024
13 checks passed

jbalonso mentioned this pull request Jan 5, 2025

Configurable handling of node NotReady #4991

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

source node: Skip unschedulable nodes #4761

source node: Skip unschedulable nodes #4761

n-Arno commented Sep 19, 2024 •

edited

Loading

linux-foundation-easycla bot commented Sep 19, 2024 •

edited

Loading

k8s-ci-robot commented Sep 19, 2024

k8s-ci-robot commented Sep 19, 2024

n-Arno commented Sep 19, 2024

mloiseleur commented Sep 20, 2024

mloiseleur commented Sep 20, 2024

n-Arno commented Sep 20, 2024

mloiseleur commented Sep 22, 2024

szuecs Sep 30, 2024

n-Arno Oct 2, 2024

szuecs Sep 30, 2024

n-Arno Oct 2, 2024

szuecs Oct 10, 2024

szuecs commented Oct 10, 2024

k8s-ci-robot commented Oct 10, 2024

source node: Skip unschedulable nodes #4761

source node: Skip unschedulable nodes #4761

Conversation

n-Arno commented Sep 19, 2024 • edited Loading

linux-foundation-easycla bot commented Sep 19, 2024 • edited Loading

k8s-ci-robot commented Sep 19, 2024

k8s-ci-robot commented Sep 19, 2024

n-Arno commented Sep 19, 2024

mloiseleur commented Sep 20, 2024

mloiseleur commented Sep 20, 2024

n-Arno commented Sep 20, 2024

mloiseleur commented Sep 22, 2024

szuecs Sep 30, 2024

Choose a reason for hiding this comment

n-Arno Oct 2, 2024

Choose a reason for hiding this comment

szuecs Sep 30, 2024

Choose a reason for hiding this comment

n-Arno Oct 2, 2024

Choose a reason for hiding this comment

szuecs Oct 10, 2024

Choose a reason for hiding this comment

szuecs commented Oct 10, 2024

k8s-ci-robot commented Oct 10, 2024

n-Arno commented Sep 19, 2024 •

edited

Loading

linux-foundation-easycla bot commented Sep 19, 2024 •

edited

Loading