feat(event): emit events for ingress,svc,pod,node,crd#6099
feat(event): emit events for ingress,svc,pod,node,crd#6099k8s-ci-robot merged 43 commits intokubernetes-sigs:masterfrom
Conversation
…t client creation Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
…t client creation Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
…t client creation Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
…t client creation Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
…t client creation Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
…t client creation Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
…t client creation Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
…t client creation Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
Pull Request Test Coverage Report for Build 23117764731Details
💛 - Coveralls |
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
580bcfd to
e3247d8
Compare
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
d34dddd to
4334924
Compare
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
4334924 to
ebcb2ad
Compare
|
/test pull-external-dns-unit-test |
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
|
/test pull-external-dns-unit-test |
| func TestEmit_RecordError(t *testing.T) { | ||
| refObj := &events.ObjectReference{} | ||
| changes := plan.Changes{ | ||
| Create: []*endpoint.Endpoint{ | ||
| endpoint.NewEndpoint("one.example.com", endpoint.RecordTypeA, "10.10.10.0"). | ||
| WithRefObject(refObj), | ||
| }, | ||
| UpdateNew: []*endpoint.Endpoint{ | ||
| endpoint.NewEndpoint("two.example.com", endpoint.RecordTypeA, "10.10.10.1"). | ||
| WithRefObject(refObj), | ||
| }, | ||
| } | ||
| emitter := fake.NewFakeEventEmitter() | ||
|
|
||
| emitChangeEvent(emitter, changes, events.RecordError) | ||
|
|
||
| emitter.AssertCalled(t, "Add", events.NewEventFromEndpoint(changes.Create[0], events.ActionCreate, events.RecordError)) | ||
| emitter.AssertCalled(t, "Add", events.NewEventFromEndpoint(changes.UpdateNew[0], events.ActionUpdate, events.RecordError)) | ||
| emitter.AssertNumberOfCalls(t, "Add", 2) | ||
| } |
There was a problem hiding this comment.
Why no test for ``Delete` changes too?
There was a problem hiding this comment.
Yep, looks like missed that gap. Deletes have unique behavior: the reason is overridden. On success they emit RecordDeleted (not RecordReady); on failure they emit RecordError. This is a deliberate branch that differs from Create/Update.
Restructured test, and added new sub-case to explicitly documents the contract
| } | ||
| } | ||
|
|
||
| func TestMergeEndpoints_RefObjects(t *testing.T) { |
There was a problem hiding this comment.
Why not test this in TestMergeEndpoints()?
There was a problem hiding this comment.
Same as #6099 (comment). This is cosmetics at the moment, but I share pros/cons for merging
Pros of merging:
- One place to look for all MergeEndpoints behavior
- Reduces test function count
Cons:
- Table struct becomes a union with optional fields or changes entirely - hurts readability of the simple cases
- assert.ElementsMatch can't verify RefObject: you'd need per-case assertion logic regardless
- Mixes two concerns: target merging vs. metadata preservation after merge
- TestMergeEndpointsLogging is already split out for the same reason (third concern: side effects)
TestMergeEndpoints owns output shape, TestMergeEndpoints_RefObjects owns metadata fidelity, TestMergeEndpointsLogging owns side effects. Each has its own assertion style that would conflict if combined.
| } | ||
| } | ||
|
|
||
| func TestDedupSource_RefObjects(t *testing.T) { |
There was a problem hiding this comment.
Why not include this in testDedupEndpoints()?
There was a problem hiding this comment.
Techically, it could be done. I could say this is more a preference how to unit tests in isolation, keep tests focused etc
The outcome is same --> Different concerns:
- TestDedupEndpoints: which endpoints survive dedup (DNS name + target identity)
- TestDedupSource_RefObjects: which metadata is preserved on the survivor (RefObject fidelity)
- The expected field is different
When test cases start to require slightly different setup, you’ve crossed a concern boundary and should keep them in separate tests. In follow up TestDedupSource_RefObjects may become a bit more complex as well, as deduplicated endpoint may have multiple references to kubernets objects.
Co-authored-by: vflaux <38909103+vflaux@users.noreply.github.com>
Signed-off-by: ivan katliarchuk <ivan.katliarchuk@gmail.com>
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ivankatliarchuk The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What does it do ?
Added emit events for
Interesting statistic for this PR
Tested
Events emitted for sources

Capture record errors

Follow-up
Motivation
Adding support for event emitting
More
require: #6076
require: #6119