From 5af4ae0edc13bae39ce9988fae76c8a760d0c16d Mon Sep 17 00:00:00 2001 From: Sandor Szuecs Date: Fri, 26 May 2023 23:18:29 +0200 Subject: [PATCH 1/2] test: controller run() should do somthing and successfully shutdown Signed-off-by: Sandor Szuecs --- controller/controller_test.go | 101 ++++++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/controller/controller_test.go b/controller/controller_test.go index f6b03d433..0660ef82f 100644 --- a/controller/controller_test.go +++ b/controller/controller_test.go @@ -224,6 +224,107 @@ func TestRunOnce(t *testing.T) { assert.Equal(t, math.Float64bits(1), valueFromMetric(verifiedAAAARecords)) } +// TestRun tests that Run correctly starts and stops +func TestRun(t *testing.T) { + // Fake some desired endpoints coming from our source. + source := new(testutils.MockSource) + cfg := externaldns.NewConfig() + cfg.ManagedDNSRecordTypes = []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME} + source.On("Endpoints").Return([]*endpoint.Endpoint{ + { + DNSName: "create-record", + RecordType: endpoint.RecordTypeA, + Targets: endpoint.Targets{"1.2.3.4"}, + }, + { + DNSName: "update-record", + RecordType: endpoint.RecordTypeA, + Targets: endpoint.Targets{"8.8.4.4"}, + }, + { + DNSName: "create-aaaa-record", + RecordType: endpoint.RecordTypeAAAA, + Targets: endpoint.Targets{"2001:DB8::1"}, + }, + { + DNSName: "update-aaaa-record", + RecordType: endpoint.RecordTypeAAAA, + Targets: endpoint.Targets{"2001:DB8::2"}, + }, + }, nil) + + // Fake some existing records in our DNS provider and validate some desired changes. + provider := newMockProvider( + []*endpoint.Endpoint{ + { + DNSName: "update-record", + RecordType: endpoint.RecordTypeA, + Targets: endpoint.Targets{"8.8.8.8"}, + }, + { + DNSName: "delete-record", + RecordType: endpoint.RecordTypeA, + Targets: endpoint.Targets{"4.3.2.1"}, + }, + { + DNSName: "update-aaaa-record", + RecordType: endpoint.RecordTypeAAAA, + Targets: endpoint.Targets{"2001:DB8::3"}, + }, + { + DNSName: "delete-aaaa-record", + RecordType: endpoint.RecordTypeAAAA, + Targets: endpoint.Targets{"2001:DB8::4"}, + }, + }, + &plan.Changes{ + Create: []*endpoint.Endpoint{ + {DNSName: "create-aaaa-record", RecordType: endpoint.RecordTypeAAAA, Targets: endpoint.Targets{"2001:DB8::1"}}, + {DNSName: "create-record", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"1.2.3.4"}}, + }, + UpdateNew: []*endpoint.Endpoint{ + {DNSName: "update-aaaa-record", RecordType: endpoint.RecordTypeAAAA, Targets: endpoint.Targets{"2001:DB8::2"}}, + {DNSName: "update-record", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"8.8.4.4"}}, + }, + UpdateOld: []*endpoint.Endpoint{ + {DNSName: "update-aaaa-record", RecordType: endpoint.RecordTypeAAAA, Targets: endpoint.Targets{"2001:DB8::3"}}, + {DNSName: "update-record", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"8.8.8.8"}}, + }, + Delete: []*endpoint.Endpoint{ + {DNSName: "delete-aaaa-record", RecordType: endpoint.RecordTypeAAAA, Targets: endpoint.Targets{"2001:DB8::4"}}, + {DNSName: "delete-record", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"4.3.2.1"}}, + }, + }, + ) + + r, err := registry.NewNoopRegistry(provider) + require.NoError(t, err) + + // Run our controller once to trigger the validation. + ctrl := &Controller{ + Source: source, + Registry: r, + Policy: &plan.SyncPolicy{}, + ManagedRecordTypes: cfg.ManagedDNSRecordTypes, + } + ctrl.nextRunAt = time.Now().Add(-time.Millisecond) + ctx, cancel := context.WithCancel(context.Background()) + stopped := make(chan struct{}) + go func() { + ctrl.Run(ctx) + close(stopped) + }() + time.Sleep(2 * time.Second) + cancel() // start shutdown + <-stopped + + // Validate that the mock source was called. + source.AssertExpectations(t) + // check the verified records + assert.Equal(t, math.Float64bits(1), valueFromMetric(verifiedARecords)) + assert.Equal(t, math.Float64bits(1), valueFromMetric(verifiedAAAARecords)) +} + func valueFromMetric(metric prometheus.Gauge) uint64 { ref := reflect.ValueOf(metric) return reflect.Indirect(ref).FieldByName("valBits").Uint() From 2b50b14726a652af5d3bf63cb262d8db48b5e551 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sandor=20Sz=C3=BCcs?= Date: Fri, 5 Jan 2024 20:11:19 +0100 Subject: [PATCH 2/2] refactor: as suggested fix: race as suggested MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Sandor Szücs --- controller/controller_test.go | 99 ++++++++--------------------------- 1 file changed, 23 insertions(+), 76 deletions(-) diff --git a/controller/controller_test.go b/controller/controller_test.go index 0660ef82f..9fa6f31bf 100644 --- a/controller/controller_test.go +++ b/controller/controller_test.go @@ -131,12 +131,9 @@ func newMockProvider(endpoints []*endpoint.Endpoint, changes *plan.Changes) prov return dnsProvider } -// TestRunOnce tests that RunOnce correctly orchestrates the different components. -func TestRunOnce(t *testing.T) { +func getTestSource() *testutils.MockSource { // Fake some desired endpoints coming from our source. source := new(testutils.MockSource) - cfg := externaldns.NewConfig() - cfg.ManagedDNSRecordTypes = []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME} source.On("Endpoints").Return([]*endpoint.Endpoint{ { DNSName: "create-record", @@ -160,8 +157,18 @@ func TestRunOnce(t *testing.T) { }, }, nil) + return source +} + +func getTestConfig() *externaldns.Config { + cfg := externaldns.NewConfig() + cfg.ManagedDNSRecordTypes = []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME} + return cfg +} + +func getTestProvider() provider.Provider { // Fake some existing records in our DNS provider and validate some desired changes. - provider := newMockProvider( + return newMockProvider( []*endpoint.Endpoint{ { DNSName: "update-record", @@ -203,6 +210,13 @@ func TestRunOnce(t *testing.T) { }, }, ) +} + +// TestRunOnce tests that RunOnce correctly orchestrates the different components. +func TestRunOnce(t *testing.T) { + source := getTestSource() + cfg := getTestConfig() + provider := getTestProvider() r, err := registry.NewNoopRegistry(provider) require.NoError(t, err) @@ -226,76 +240,9 @@ func TestRunOnce(t *testing.T) { // TestRun tests that Run correctly starts and stops func TestRun(t *testing.T) { - // Fake some desired endpoints coming from our source. - source := new(testutils.MockSource) - cfg := externaldns.NewConfig() - cfg.ManagedDNSRecordTypes = []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME} - source.On("Endpoints").Return([]*endpoint.Endpoint{ - { - DNSName: "create-record", - RecordType: endpoint.RecordTypeA, - Targets: endpoint.Targets{"1.2.3.4"}, - }, - { - DNSName: "update-record", - RecordType: endpoint.RecordTypeA, - Targets: endpoint.Targets{"8.8.4.4"}, - }, - { - DNSName: "create-aaaa-record", - RecordType: endpoint.RecordTypeAAAA, - Targets: endpoint.Targets{"2001:DB8::1"}, - }, - { - DNSName: "update-aaaa-record", - RecordType: endpoint.RecordTypeAAAA, - Targets: endpoint.Targets{"2001:DB8::2"}, - }, - }, nil) - - // Fake some existing records in our DNS provider and validate some desired changes. - provider := newMockProvider( - []*endpoint.Endpoint{ - { - DNSName: "update-record", - RecordType: endpoint.RecordTypeA, - Targets: endpoint.Targets{"8.8.8.8"}, - }, - { - DNSName: "delete-record", - RecordType: endpoint.RecordTypeA, - Targets: endpoint.Targets{"4.3.2.1"}, - }, - { - DNSName: "update-aaaa-record", - RecordType: endpoint.RecordTypeAAAA, - Targets: endpoint.Targets{"2001:DB8::3"}, - }, - { - DNSName: "delete-aaaa-record", - RecordType: endpoint.RecordTypeAAAA, - Targets: endpoint.Targets{"2001:DB8::4"}, - }, - }, - &plan.Changes{ - Create: []*endpoint.Endpoint{ - {DNSName: "create-aaaa-record", RecordType: endpoint.RecordTypeAAAA, Targets: endpoint.Targets{"2001:DB8::1"}}, - {DNSName: "create-record", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"1.2.3.4"}}, - }, - UpdateNew: []*endpoint.Endpoint{ - {DNSName: "update-aaaa-record", RecordType: endpoint.RecordTypeAAAA, Targets: endpoint.Targets{"2001:DB8::2"}}, - {DNSName: "update-record", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"8.8.4.4"}}, - }, - UpdateOld: []*endpoint.Endpoint{ - {DNSName: "update-aaaa-record", RecordType: endpoint.RecordTypeAAAA, Targets: endpoint.Targets{"2001:DB8::3"}}, - {DNSName: "update-record", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"8.8.8.8"}}, - }, - Delete: []*endpoint.Endpoint{ - {DNSName: "delete-aaaa-record", RecordType: endpoint.RecordTypeAAAA, Targets: endpoint.Targets{"2001:DB8::4"}}, - {DNSName: "delete-record", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"4.3.2.1"}}, - }, - }, - ) + source := getTestSource() + cfg := getTestConfig() + provider := getTestProvider() r, err := registry.NewNoopRegistry(provider) require.NoError(t, err) @@ -314,7 +261,7 @@ func TestRun(t *testing.T) { ctrl.Run(ctx) close(stopped) }() - time.Sleep(2 * time.Second) + time.Sleep(1500 * time.Millisecond) cancel() // start shutdown <-stopped