test: controller run() and successfully shutdown #3639

szuecs · 2023-05-26T21:19:58Z

test: controller run() should do somthing and successfully shutdown

Signed-off-by: Sandor Szuecs <sandor.szuecs@zalando.de>

szuecs · 2023-05-26T21:22:17Z

/ok-to-test

szuecs · 2023-05-26T21:33:25Z

/lgtm

k8s-ci-robot · 2023-05-26T21:33:26Z

@szuecs: you cannot LGTM your own PR.

In response to this:

/lgtm

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/test-infra repository.

controller/controller_test.go

johngmyers · 2023-06-01T05:40:48Z

controller/controller_test.go

+	source := new(testutils.MockSource)
+	cfg := externaldns.NewConfig()
+	cfg.ManagedDNSRecordTypes = []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME}
+	source.On("Endpoints").Return([]*endpoint.Endpoint{


Possibly extract the setup and validation code that is common between this and TestRunOnce?

mloiseleur · 2023-10-27T11:31:42Z

@szuecs Wdyt of @johngmyers comments ?

fix: race as suggested Signed-off-by: Sandor Szücs <sandor.szuecs@zalando.de>

szuecs · 2024-01-05T19:12:45Z

@mloiseleur changed the code to fix all comments by @johngmyers

mloiseleur · 2024-01-06T09:28:33Z

controller/controller_test.go

+		ctrl.Run(ctx)
+		close(stopped)
+	}()
+	time.Sleep(1500 * time.Millisecond)


It's often a source of flakyness to introduce time.sleep in a test.
Do you think it's possible to use a readiness-like approach ?

while you are right, it's not really possible to do so.
cancel() starts shutdown (triggers being non-ready) and we want to wait for an iteration of external-dns.
We could of course also create a hook inside external-dns control loop, but that's a bit too much for only testing IMO.

mloiseleur · 2024-01-15T07:21:43Z

/lgtm

mloiseleur · 2024-01-15T07:22:08Z

/assign @Raffo
for final review

k8s-triage-robot · 2024-04-14T07:37:12Z

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

Mark this PR as fresh with /remove-lifecycle stale
Close this PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

szuecs · 2024-04-25T14:58:37Z

/remove-lifecycle stale

mloiseleur · 2024-05-03T11:18:50Z

/retitle test: controller run() and successfully shutdown

Raffo · 2024-05-03T19:29:58Z

/approve

k8s-ci-robot · 2024-05-03T19:30:14Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Raffo

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~~ [Raffo]

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

test: controller run() should do somthing and successfully shutdown

5af4ae0

Signed-off-by: Sandor Szuecs <sandor.szuecs@zalando.de>

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 26, 2023

k8s-ci-robot requested review from njuettner and seanmalloy May 26, 2023 21:20

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 26, 2023

k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label May 26, 2023

johngmyers reviewed Jun 1, 2023

View reviewed changes

controller/controller_test.go Outdated Show resolved Hide resolved

johngmyers reviewed Jun 1, 2023

View reviewed changes

refactor: as suggested

2b50b14

fix: race as suggested Signed-off-by: Sandor Szücs <sandor.szuecs@zalando.de>

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 5, 2024

mloiseleur reviewed Jan 6, 2024

View reviewed changes

k8s-ci-robot assigned mloiseleur Jan 15, 2024

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

k8s-ci-robot assigned Raffo Jan 15, 2024

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 14, 2024

k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 25, 2024

k8s-ci-robot changed the title ~~test: controller run() should do somthing and successfully shutdown~~ test: controller run() and successfully shutdown May 3, 2024

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

k8s-ci-robot merged commit fa17f9d into master May 3, 2024
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: controller run() and successfully shutdown #3639

test: controller run() and successfully shutdown #3639

szuecs commented May 26, 2023

szuecs commented May 26, 2023

szuecs commented May 26, 2023

k8s-ci-robot commented May 26, 2023

johngmyers Jun 1, 2023

szuecs Jan 5, 2024

mloiseleur commented Oct 27, 2023

szuecs commented Jan 5, 2024

mloiseleur Jan 6, 2024

szuecs Jan 8, 2024

mloiseleur commented Jan 15, 2024

mloiseleur commented Jan 15, 2024

k8s-triage-robot commented Apr 14, 2024

szuecs commented Apr 25, 2024

mloiseleur commented May 3, 2024

Raffo commented May 3, 2024

k8s-ci-robot commented May 3, 2024

test: controller run() and successfully shutdown #3639

test: controller run() and successfully shutdown #3639

Conversation

szuecs commented May 26, 2023

szuecs commented May 26, 2023

szuecs commented May 26, 2023

k8s-ci-robot commented May 26, 2023

johngmyers Jun 1, 2023

Choose a reason for hiding this comment

szuecs Jan 5, 2024

Choose a reason for hiding this comment

mloiseleur commented Oct 27, 2023

szuecs commented Jan 5, 2024

mloiseleur Jan 6, 2024

Choose a reason for hiding this comment

szuecs Jan 8, 2024

Choose a reason for hiding this comment

mloiseleur commented Jan 15, 2024

mloiseleur commented Jan 15, 2024

k8s-triage-robot commented Apr 14, 2024

szuecs commented Apr 25, 2024

mloiseleur commented May 3, 2024

Raffo commented May 3, 2024

k8s-ci-robot commented May 3, 2024