Skip to content

Conversation

ivankatliarchuk
Copy link
Contributor

@ivankatliarchuk ivankatliarchuk commented Jul 17, 2025

What does it do ?

Note: safe to merge as it does not break any current functionality, and only supported with fake source

  • Publish k8s events. Scoped to fake source, to keep change size relatively small.
    • fake events do not have any object reference
  • currently only supported successful events, as to support failed cases need more effort
  • emit only specific events, similar to --service-type-filter logic
  • events emit relevant docs

I'm slicing this feature into multiple PRs, as there is a lot to add + tests are usually 3x lines vs actual logic + docs + smoke testing is simpler when delivered in chunks

High Level Diagram

sequenceDiagram
  participant Source
  participant ReferenceObject
  participant Endpoint
  participant Plan
  participant Event
  participant Provider
  participant EventEmitter


  loop Process each Endpoint
    Source->>Endpoint: Add Endpoint with Reference
  end

  Endpoint-->>ReferenceObject: Contains
  Plan-->>Endpoint: Contains multiple

  loop Apply Changes
    Plan->>Provider: Apply Endpoint Changes
    Provider-->>Endpoint: Label with Skip/Success/Failed
    Provider-->>Plan: Return Result
  end


  loop Process each Event
    Provider->>Plan: Label with Skip/Success/Failed
    Plan-->>Event: Construct Event
    Event-->>ReferenceObject: Contains
    Event->>EventEmitter: Emit
  end
Loading

Follow-up:

  • Provider a reliable way to label each endpoint with Success, Skip, Failed so that we could emit correct event per endpiont. This may take time.
  • improved unit-test coverage
  • metrics
  • helm changes
  • support CRD DNSEndpoint as source
  • support all other sources
  • externalise method func (ec *Controller) run(ctx context.Context) with metrics instrumentation
  • instrument kubeclient with metrics centrally

Motivation

Fixes #646
Relates #5243. The change will enable us to raise events for CRD changes

More

  • Yes, this PR title follows Conventional Commits
  • Yes, I added unit tests
  • Yes, I updated end user documentation accordingly
  • Smoke tests results
  • Update what does it do with references if any

Example Fake event initiated for core-dns pod.
Screenshot 2025-07-20 at 16 43 03

At the moment plan is not aware where or not changes applied or skipped, or failed for certain Endpoints. This will be added in follow-up PR

example, skipped endpoint creation due to domain is not supported
Screenshot 2025-07-20 at 16 30 50

Example Event in Json format

{
            "action": "Created",
            "apiVersion": "v1",
            "eventTime": "2025-07-20T15:29:02.768054Z",
            "firstTimestamp": null,
            "involvedObject": {
                "apiVersion": "v1",
                "kind": "Pod",
                "name": "coredns-668d6bf9bc-2d2ds",
                "namespace": "kube-system",
                "uid": "558f5d67-f678-4581-b0bd-0f8c031aec75"
            },
            "kind": "Event",
            "lastTimestamp": null,
            "message": "record:buzd.example.com, owner:fake, type:A, targets:192.0.2.190",
            "metadata": {
                "creationTimestamp": "2025-07-20T15:29:01Z",
                "name": "coredns-668d6bf9bc-2d2ds.1853ff42e5f27ef0",
                "namespace": "kube-system",
                "resourceVersion": "3792",
                "uid": "3e9fe0e3-b5b1-4381-8dca-24fc00a06357"
            },
            "reason": "RecordReady",
            "related": {
                "apiVersion": "v1",
                "kind": "Pod",
                "name": "coredns-668d6bf9bc-2d2ds",
                "namespace": "kube-system",
                "uid": "558f5d67-f678-4581-b0bd-0f8c031aec75"
            },
            "reportingComponent": "external-dns-Ivans-MacBook-Pro.local",
            "reportingInstance": "external-dns",
            "source": {},
            "type": "Normal"
        },

@ivankatliarchuk ivankatliarchuk marked this pull request as draft July 17, 2025 10:49
@k8s-ci-robot k8s-ci-robot added the controller Issues or PRs related to the controller label Jul 17, 2025
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. plan Issues or PRs related to external-dns plan source do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 17, 2025
@ivankatliarchuk ivankatliarchuk force-pushed the feat-emit-events-for-changes branch 3 times, most recently from 80243c1 to 029c972 Compare July 17, 2025 11:11
@ivankatliarchuk ivankatliarchuk force-pushed the feat-emit-events-for-changes branch 2 times, most recently from 42071de to 3f6008f Compare July 18, 2025 17:46
@k8s-ci-robot k8s-ci-robot added the apis Issues or PRs related to API change label Jul 18, 2025
Signed-off-by: ivan katliarchuk <[email protected]>
@ivankatliarchuk ivankatliarchuk force-pushed the feat-emit-events-for-changes branch from 3f6008f to dcbcf99 Compare July 18, 2025 17:51
@ivankatliarchuk ivankatliarchuk force-pushed the feat-emit-events-for-changes branch from 4942eb4 to 772e5de Compare July 19, 2025 07:13
@ivankatliarchuk ivankatliarchuk force-pushed the feat-emit-events-for-changes branch 4 times, most recently from fa0af46 to 9973da2 Compare July 20, 2025 13:45
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 20, 2025
@ivankatliarchuk ivankatliarchuk force-pushed the feat-emit-events-for-changes branch from 9973da2 to 7eab91c Compare July 20, 2025 14:00
Signed-off-by: ivan katliarchuk <[email protected]>
@ivankatliarchuk ivankatliarchuk force-pushed the feat-emit-events-for-changes branch from 7eab91c to f57baa4 Compare July 20, 2025 14:46
@ivankatliarchuk
Copy link
Contributor Author

Tested once again. Seems like working

@ivankatliarchuk
Copy link
Contributor Author

Heh silly things missed ;-)

Copy link
Collaborator

@mloiseleur mloiseleur left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 16, 2025
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 16, 2025
@ivankatliarchuk
Copy link
Contributor Author

rebase and resolved merge conflicts

Copy link
Collaborator

@mloiseleur mloiseleur left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 20, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mloiseleur

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:

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 k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 20, 2025
@k8s-ci-robot k8s-ci-robot merged commit e9983a5 into kubernetes-sigs:master Aug 20, 2025
14 checks passed
@ivankatliarchuk ivankatliarchuk deleted the feat-emit-events-for-changes branch August 20, 2025 11:13
@vflaux
Copy link
Contributor

vflaux commented Aug 24, 2025

This PR cause a panic at:

func (ec *Controller) Add(events ...Event) {
if ec.queue.Len() >= ec.maxQueuedEvents {

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1f3cb3d]

goroutine 1 [running]:
sigs.k8s.io/external-dns/pkg/events.(*Controller).Add(0x0, {0xc0008756b0, 0x1, 0x1})
	/external-dns/pkg/events/controller.go:137 +0x3d
sigs.k8s.io/external-dns/controller.emitChangeEvent({0x79f0500, 0x0}, {{0xc0005b4438, 0x1, 0x1}, {0xa7370e0, 0x0, 0x0}, {0xa7370e0, 0x0, ...}, ...}, ...)
	/external-dns/controller/events.go:32 +0x207
sigs.k8s.io/external-dns/controller.(*Controller).RunOnce(0xc0006fd110, {0x7a3fe78, 0xc00094a990})
	/external-dns/controller/controller.go:250 +0xdc5
sigs.k8s.io/external-dns/controller.(*Controller).Run(0xc0006fd110, {0x7a3feb0, 0xc000357630})
	/external-dns/controller/controller.go:347 +0x113
sigs.k8s.io/external-dns/controller.Execute()
	/external-dns/controller/execute.go:144 +0xe7a
main.main()
	/external-dns/main.go:26 +0xf

ec is actually nil.
There is a check for nil here:

func emitChangeEvent(e events.EventEmitter, ch plan.Changes, reason events.Reason) {
if e == nil {
return
}

But we receive a typed pointer to nil, not nil directly.

This value come from here:

eventsCfg := events.NewConfig(
events.WithKubeConfig(cfg.KubeConfig, cfg.APIServerURL, cfg.RequestTimeout),
events.WithEmitEvents(cfg.EmitEvents),
events.WithDryRun(cfg.DryRun))
var eventsCtrl *events.Controller
if eventsCfg.IsEnabled() {
eventsCtrl, err = events.NewEventController(eventsCfg)
if err != nil {
log.Fatal(err)
}
eventsCtrl.Run(ctx)
}
return &Controller{
Source: src,
Registry: reg,
Policy: policy,
Interval: cfg.Interval,
DomainFilter: filter,
ManagedRecordTypes: cfg.ManagedDNSRecordTypes,
ExcludeRecordTypes: cfg.ExcludeDNSRecordTypes,
MinEventSyncInterval: cfg.MinEventSyncInterval,
EventController: eventsCtrl,
}, nil

Passing an nil *events.Controller to an interface field is not the same as passing nil to this field. Which is quite confusing.

This is shown in debugger:

  • Passing a nil *events.Controller: image
  • Not assigning any value: image

troll-os pushed a commit to FiligranHQ/external-dns that referenced this pull request Aug 28, 2025
* feat(events): publish k8s events

Signed-off-by: ivan katliarchuk <[email protected]>

* feat(events): publish k8s events

Signed-off-by: ivan katliarchuk <[email protected]>

* feat(events): publish k8s events

Signed-off-by: ivan katliarchuk <[email protected]>

* feat(events): publish k8s events

Signed-off-by: ivan katliarchuk <[email protected]>

* feat(events): publish k8s events

Signed-off-by: ivan katliarchuk <[email protected]>

* feat(events): raise k8s events with fake provider

Signed-off-by: ivan katliarchuk <[email protected]>

* feat(events): raise k8s events with fake provider

Signed-off-by: ivan katliarchuk <[email protected]>

* feat(events): raise k8s events with fake provider

Signed-off-by: ivan katliarchuk <[email protected]>

* feat(events): raise k8s events with fake provider

Signed-off-by: ivan katliarchuk <[email protected]>

* feat(events): raise k8s events with fake provider

Co-authored-by: Michel Loiseleur <[email protected]>

* feat(events): raise k8s events with fake provider

Signed-off-by: ivan katliarchuk <[email protected]>

* feat(events): raise k8s events with fake provide

Co-authored-by: Michel Loiseleur <[email protected]>

* feat(events): raise k8s events with fake provider

Signed-off-by: ivan katliarchuk <[email protected]>

* feat(events): raise k8s events with fake provider

Signed-off-by: ivan katliarchuk <[email protected]>

---------

Signed-off-by: ivan katliarchuk <[email protected]>
Co-authored-by: Michel Loiseleur <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
apis Issues or PRs related to API change approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. controller Issues or PRs related to the controller docs lgtm "Looks good to me", indicates that a PR is ready to be merged. plan Issues or PRs related to external-dns plan provider Issues or PRs related to a provider size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. source
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Emit Kubernetes events for changes
4 participants