From 28913217e96de10aedb0b807ca6a7f849b5e1c46 Mon Sep 17 00:00:00 2001 From: Reinier Schoof Date: Fri, 28 Mar 2025 16:59:57 +0100 Subject: [PATCH 1/3] refactor: cleanup unnecessary function --- birdwatcher/healthcheck.go | 6 +----- birdwatcher/healthcheck_test.go | 18 ------------------ birdwatcher/testdata/config/service_noprefixes | 3 --- 3 files changed, 1 insertion(+), 26 deletions(-) delete mode 100644 birdwatcher/testdata/config/service_noprefixes diff --git a/birdwatcher/healthcheck.go b/birdwatcher/healthcheck.go index d7c11ef..469c7a2 100644 --- a/birdwatcher/healthcheck.go +++ b/birdwatcher/healthcheck.go @@ -87,10 +87,6 @@ func (h *HealthCheck) Start(services []*ServiceCheck, ready chan<- bool, status } } -func (h *HealthCheck) didReloadBefore() bool { - return h.reloadedBefore -} - func (h *HealthCheck) handleAction(action *Action, status chan string) { for _, p := range action.Prefixes { switch action.State { @@ -160,7 +156,7 @@ func (h *HealthCheck) applyConfig(config Config, prefixes PrefixCollection) erro // if config did not change, we should still reload if we don't know the // state of BIRD if errors.Is(err, errConfigIdentical) { - if h.didReloadBefore() { + if h.reloadedBefore { cLog.Warning("config did not change, not reloading") return nil diff --git a/birdwatcher/healthcheck_test.go b/birdwatcher/healthcheck_test.go index b533c45..deeea1d 100644 --- a/birdwatcher/healthcheck_test.go +++ b/birdwatcher/healthcheck_test.go @@ -62,24 +62,6 @@ func TestHealthCheck_removePrefix(t *testing.T) { assert.Empty(t, testutil.ToFloat64(prefixStateMetric.WithLabelValues("svc1", "1.2.3.0/24"))) } -func TestHealthCheckDidReloadBefore(t *testing.T) { - t.Parallel() - - hc := NewHealthCheck(Config{}) - - // expect both to fail - assert.False(t, hc.didReloadBefore()) - - // should succeed now - hc.reloadedBefore = true - assert.True(t, hc.didReloadBefore()) - - hc.reloadedBefore = false - - // expect to fail again - assert.False(t, hc.didReloadBefore()) -} - func TestHealthCheck_handleAction(t *testing.T) { t.Parallel() diff --git a/birdwatcher/testdata/config/service_noprefixes b/birdwatcher/testdata/config/service_noprefixes deleted file mode 100644 index 5361317..0000000 --- a/birdwatcher/testdata/config/service_noprefixes +++ /dev/null @@ -1,3 +0,0 @@ -[services] - [services."foo"] - command = "/usr/bin/true" From 16bc4b1cc6d234b4aa59c91d1822af099465b27e Mon Sep 17 00:00:00 2001 From: Reinier Schoof Date: Fri, 28 Mar 2025 17:26:24 +0100 Subject: [PATCH 2/3] style: apply some linting --- birdwatcher/bird.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/birdwatcher/bird.go b/birdwatcher/bird.go index f1cf506..40ff81c 100644 --- a/birdwatcher/bird.go +++ b/birdwatcher/bird.go @@ -52,7 +52,9 @@ func writeBirdConfig(filename string, prefixes PrefixCollection, compatBird213 b return err } - tmpl := template.Must(template.New("func").Funcs(tplFuncs).Parse(functionsTemplate)) + tmpl := template.Must(template.New("func"). + Funcs(tplFuncs). + Parse(functionsTemplate)) tplBody := struct { Collections PrefixCollection @@ -63,7 +65,7 @@ func writeBirdConfig(filename string, prefixes PrefixCollection, compatBird213 b } var buf bytes.Buffer - if err := tmpl.Execute(&buf, tplBody); err != nil { + if err = tmpl.Execute(&buf, tplBody); err != nil { return err } From 563de315e0e256a95e4f57095c5b159b0f0a153b Mon Sep 17 00:00:00 2001 From: Reinier Schoof Date: Fri, 28 Mar 2025 17:31:25 +0100 Subject: [PATCH 3/3] feat: support services without prefixes When services without any prefixes are considered up, their corresponding filter will just match anything --- README.md | 2 +- birdwatcher/bird_test.go | 70 +++++++++++++++---- birdwatcher/config.go | 4 -- birdwatcher/config_test.go | 14 +--- birdwatcher/healthcheck.go | 15 ++-- birdwatcher/prefixset.go | 12 +++- birdwatcher/prefixset_test.go | 4 +- birdwatcher/templates/functions.tpl | 4 ++ birdwatcher/testdata/bird/config_compat | 9 --- .../bird/{config_empty => filter_down} | 0 birdwatcher/testdata/bird/filter_down_compat | 5 ++ .../bird/{config => filter_one_prefix} | 0 .../testdata/bird/filter_one_prefix_compat | 10 +++ birdwatcher/testdata/bird/filter_up | 5 ++ birdwatcher/testdata/bird/filter_up_compat | 5 ++ birdwatcher/testdata/config/minimal | 1 - 16 files changed, 108 insertions(+), 52 deletions(-) delete mode 100644 birdwatcher/testdata/bird/config_compat rename birdwatcher/testdata/bird/{config_empty => filter_down} (100%) create mode 100644 birdwatcher/testdata/bird/filter_down_compat rename birdwatcher/testdata/bird/{config => filter_one_prefix} (100%) create mode 100644 birdwatcher/testdata/bird/filter_one_prefix_compat create mode 100644 birdwatcher/testdata/bird/filter_up create mode 100644 birdwatcher/testdata/bird/filter_up_compat diff --git a/README.md b/README.md index 1504cb3..b1081ff 100644 --- a/README.md +++ b/README.md @@ -104,7 +104,7 @@ Each service under this section can have the following settings: | timeout | Time in which the check command should complete. Afterwards it will be handled as if the check command failed. Defaults to **10s**, format following that of [`time.ParseDuration`](https://pkg.go.dev/time#ParseDuration). | | fail | The amount of times the check command should fail before the service is considered to be down. Defaults to **1** | | rise | The amount of times the check command should succeed before the service is considered to be up. Defaults to **1** | -| prefixes | Array of prefixes, mixed IPv4 and IPv6. At least 1 prefix is **required** per service | +| prefixes | Array of prefixes, mixed IPv4 and IPv6. When no prefixes are given, the resulting filter won't match any addresses but just return **true** or **false**. | ## **[prometheus]** diff --git a/birdwatcher/bird_test.go b/birdwatcher/bird_test.go index 4e62aaf..785030d 100644 --- a/birdwatcher/bird_test.go +++ b/birdwatcher/bird_test.go @@ -13,7 +13,7 @@ import ( func TestWriteBirdConfig(t *testing.T) { t.Parallel() - t.Run("empty config", func(t *testing.T) { + t.Run("filter down", func(t *testing.T) { t.Parallel() // open tempfile @@ -21,10 +21,11 @@ func TestWriteBirdConfig(t *testing.T) { require.NoError(t, err) defer os.Remove(tmpFile.Name()) + // no prefixes, service unhealthy prefixes := make(PrefixCollection) - prefixes["match_route"] = NewPrefixSet("match_route") + prefixes["match_route"] = NewPrefixSet(&ServiceCheck{FunctionName: "match_route"}) - // write bird config with empty prefix list + // write bird config with empty prefix list and unhealthy err = writeBirdConfig(tmpFile.Name(), prefixes, false) require.NoError(t, err) @@ -32,7 +33,20 @@ func TestWriteBirdConfig(t *testing.T) { data, err := os.ReadFile(tmpFile.Name()) require.NoError(t, err) - fixture, err := os.ReadFile("testdata/bird/config_empty") + fixture, err := os.ReadFile("testdata/bird/filter_down") + require.NoError(t, err) + + assert.Equal(t, string(fixture), string(data)) + + // switch to compat mode and do it again + err = writeBirdConfig(tmpFile.Name(), prefixes, true) + require.NoError(t, err) + + // read data from temp file and compare it to file fixture + data, err = os.ReadFile(tmpFile.Name()) + require.NoError(t, err) + + fixture, err = os.ReadFile("testdata/bird/filter_down_compat") require.NoError(t, err) assert.Equal(t, string(fixture), string(data)) @@ -46,8 +60,9 @@ func TestWriteBirdConfig(t *testing.T) { require.NoError(t, err) defer os.Remove(tmpFile.Name()) + // one healthy prefix, service assumed healthy prefixes := make(PrefixCollection) - prefixes["match_route"] = NewPrefixSet("match_route") + prefixes["match_route"] = NewPrefixSet(&ServiceCheck{FunctionName: "match_route"}) for _, pref := range []string{"1.2.3.4/32", "2.3.4.5/26", "3.4.5.6/24", "4.5.6.7/21"} { _, prf, _ := net.ParseCIDR(pref) @@ -62,13 +77,27 @@ func TestWriteBirdConfig(t *testing.T) { data, err := os.ReadFile(tmpFile.Name()) require.NoError(t, err) - fixture, err := os.ReadFile("testdata/bird/config") + fixture, err := os.ReadFile("testdata/bird/filter_one_prefix") + require.NoError(t, err) + + assert.Equal(t, string(fixture), string(data)) + + // switch to compat mode and do it again + // write bird config to it + err = writeBirdConfig(tmpFile.Name(), prefixes, true) + require.NoError(t, err) + + // read data from temp file and compare it to file fixture + data, err = os.ReadFile(tmpFile.Name()) + require.NoError(t, err) + + fixture, err = os.ReadFile("testdata/bird/filter_one_prefix_compat") require.NoError(t, err) assert.Equal(t, string(fixture), string(data)) }) - t.Run("one prefix, compat", func(t *testing.T) { + t.Run("filter up", func(t *testing.T) { t.Parallel() // open tempfile @@ -76,23 +105,36 @@ func TestWriteBirdConfig(t *testing.T) { require.NoError(t, err) defer os.Remove(tmpFile.Name()) + // no healthy prefixes, service explicitly healthy prefixes := make(PrefixCollection) + prefixes["match_route"] = NewPrefixSet(&ServiceCheck{ + FunctionName: "match_route", + state: ServiceStateUp, + }) - prefixes["other_function"] = NewPrefixSet("other_function") - for _, pref := range []string{"5.6.7.8/32", "6.7.8.9/26", "7.8.9.10/24"} { - _, prf, _ := net.ParseCIDR(pref) - prefixes["other_function"].Add(*prf) - } + // write bird config to it + err = writeBirdConfig(tmpFile.Name(), prefixes, false) + require.NoError(t, err) + // read data from temp file and compare it to file fixture + data, err := os.ReadFile(tmpFile.Name()) + require.NoError(t, err) + + fixture, err := os.ReadFile("testdata/bird/filter_up") + require.NoError(t, err) + + assert.Equal(t, string(fixture), string(data)) + + // switch to compat mode and do it again // write bird config to it err = writeBirdConfig(tmpFile.Name(), prefixes, true) require.NoError(t, err) // read data from temp file and compare it to file fixture - data, err := os.ReadFile(tmpFile.Name()) + data, err = os.ReadFile(tmpFile.Name()) require.NoError(t, err) - fixture, err := os.ReadFile("testdata/bird/config_compat") + fixture, err = os.ReadFile("testdata/bird/filter_up_compat") require.NoError(t, err) assert.Equal(t, string(fixture), string(data)) diff --git a/birdwatcher/config.go b/birdwatcher/config.go index 3d62ac8..493c057 100644 --- a/birdwatcher/config.go +++ b/birdwatcher/config.go @@ -139,10 +139,6 @@ func validateService(s *ServiceCheck) error { s.Rise = defaultServiceRise } - if len(s.Prefixes) == 0 { - return fmt.Errorf("service %s has no prefixes set", s.name) - } - return nil } diff --git a/birdwatcher/config_test.go b/birdwatcher/config_test.go index 32857e5..645c1c8 100644 --- a/birdwatcher/config_test.go +++ b/birdwatcher/config_test.go @@ -52,16 +52,6 @@ func TestConfig(t *testing.T) { } }) - // check for error for service with no prefixes - t.Run("service no prefixes", func(t *testing.T) { - t.Parallel() - - err := ReadConfig(&Config{}, "testdata/config/service_noprefixes") - if assert.Error(t, err) { - assert.Regexp(t, regexp.MustCompile("^service .+ has no prefixes set"), err.Error()) - } - }) - // check for error for service with invalid prefix t.Run("invalid prefix", func(t *testing.T) { t.Parallel() @@ -106,9 +96,7 @@ func TestConfig(t *testing.T) { assert.Equal(t, defaultServiceRise, testConf.Services["foo"].Rise) assert.Equal(t, defaultServiceTimeout, testConf.Services["foo"].Timeout) - if assert.Len(t, testConf.Services["foo"].prefixes, 1) { - assert.Equal(t, "192.168.0.0/24", testConf.Services["foo"].prefixes[0].String()) - } + assert.Len(t, testConf.Services["foo"].prefixes, 0) // check GetServices result svcs := testConf.GetServices() diff --git a/birdwatcher/healthcheck.go b/birdwatcher/healthcheck.go index 469c7a2..2b6fb46 100644 --- a/birdwatcher/healthcheck.go +++ b/birdwatcher/healthcheck.go @@ -212,28 +212,31 @@ func (h *HealthCheck) applyConfig(config Config, prefixes PrefixCollection) erro } func (h *HealthCheck) addPrefix(svc *ServiceCheck, prefix net.IPNet) { - h.ensurePrefixSet(svc.FunctionName) + h.ensurePrefixSet(svc) h.prefixes[svc.FunctionName].Add(prefix) prefixStateMetric.WithLabelValues(svc.Name(), prefix.String()).Set(1.0) } func (h *HealthCheck) removePrefix(svc *ServiceCheck, prefix net.IPNet) { - h.ensurePrefixSet(svc.FunctionName) + h.ensurePrefixSet(svc) h.prefixes[svc.FunctionName].Remove(prefix) prefixStateMetric.WithLabelValues(svc.Name(), prefix.String()).Set(0.0) } -func (h *HealthCheck) ensurePrefixSet(functionName string) { +func (h *HealthCheck) ensurePrefixSet(svc *ServiceCheck) { // make sure the top level map is prepared if h.prefixes == nil { h.prefixes = make(PrefixCollection) } - // make sure a mapping for this function name exists - if _, found := h.prefixes[functionName]; !found { - h.prefixes[functionName] = NewPrefixSet(functionName) + // make sure a mapping for this function name exists and reflects the current + // health status of the service + if _, found := h.prefixes[svc.FunctionName]; found { + h.prefixes[svc.FunctionName].healthy = svc.IsUp() + } else { + h.prefixes[svc.FunctionName] = NewPrefixSet(svc) } } diff --git a/birdwatcher/prefixset.go b/birdwatcher/prefixset.go index 1c57a79..2c5db9a 100644 --- a/birdwatcher/prefixset.go +++ b/birdwatcher/prefixset.go @@ -16,11 +16,15 @@ type PrefixCollection map[string]*PrefixSet type PrefixSet struct { prefixes []net.IPNet functionName string + healthy bool } // NewPrefixSet returns a new prefixset with given function name -func NewPrefixSet(functionName string) *PrefixSet { - return &PrefixSet{functionName: functionName} +func NewPrefixSet(svc *ServiceCheck) *PrefixSet { + return &PrefixSet{ + functionName: svc.FunctionName, + healthy: svc.IsUp(), + } } // FunctionName returns the function name @@ -33,6 +37,10 @@ func (p PrefixSet) Prefixes() []net.IPNet { return p.prefixes } +func (p PrefixSet) Healthy() bool { + return p.healthy +} + // Add adds a prefix to the PrefixSet if it wasn't already in it func (p *PrefixSet) Add(prefix net.IPNet) { pLog := log.WithFields(log.Fields{ diff --git a/birdwatcher/prefixset_test.go b/birdwatcher/prefixset_test.go index 766ef91..28ae07c 100644 --- a/birdwatcher/prefixset_test.go +++ b/birdwatcher/prefixset_test.go @@ -10,7 +10,7 @@ import ( func TestPrefixSet_Add(t *testing.T) { t.Parallel() - p := NewPrefixSet("foobar") + p := NewPrefixSet(&ServiceCheck{FunctionName: "foobar"}) // should be empty assert.Empty(t, p.prefixes) @@ -44,7 +44,7 @@ func TestPrefixSet_Add(t *testing.T) { func TestPrefixSet_Remove(t *testing.T) { t.Parallel() - p := NewPrefixSet("foobar") + p := NewPrefixSet(&ServiceCheck{FunctionName: "foobar"}) // add some prefixes prefixes := make([]net.IPNet, 4) diff --git a/birdwatcher/templates/functions.tpl b/birdwatcher/templates/functions.tpl index cb1eed0..5595268 100644 --- a/birdwatcher/templates/functions.tpl +++ b/birdwatcher/templates/functions.tpl @@ -8,8 +8,12 @@ function {{.FunctionName}}(){{- if not $.CompatBird213 }} -> bool{{- end }} {{.}} {{- end }} ]; +{{- else }} +{{- if .Healthy }} + return true; {{- else }} return false; {{- end }} +{{- end }} } {{- end }} diff --git a/birdwatcher/testdata/bird/config_compat b/birdwatcher/testdata/bird/config_compat deleted file mode 100644 index a5a0c53..0000000 --- a/birdwatcher/testdata/bird/config_compat +++ /dev/null @@ -1,9 +0,0 @@ -# DO NOT EDIT MANUALLY -function other_function() -{ - return net ~ [ - 5.6.7.8/32, - 6.7.8.0/26, - 7.8.9.0/24 - ]; -} diff --git a/birdwatcher/testdata/bird/config_empty b/birdwatcher/testdata/bird/filter_down similarity index 100% rename from birdwatcher/testdata/bird/config_empty rename to birdwatcher/testdata/bird/filter_down diff --git a/birdwatcher/testdata/bird/filter_down_compat b/birdwatcher/testdata/bird/filter_down_compat new file mode 100644 index 0000000..07ca6f4 --- /dev/null +++ b/birdwatcher/testdata/bird/filter_down_compat @@ -0,0 +1,5 @@ +# DO NOT EDIT MANUALLY +function match_route() +{ + return false; +} diff --git a/birdwatcher/testdata/bird/config b/birdwatcher/testdata/bird/filter_one_prefix similarity index 100% rename from birdwatcher/testdata/bird/config rename to birdwatcher/testdata/bird/filter_one_prefix diff --git a/birdwatcher/testdata/bird/filter_one_prefix_compat b/birdwatcher/testdata/bird/filter_one_prefix_compat new file mode 100644 index 0000000..b0d2dd0 --- /dev/null +++ b/birdwatcher/testdata/bird/filter_one_prefix_compat @@ -0,0 +1,10 @@ +# DO NOT EDIT MANUALLY +function match_route() +{ + return net ~ [ + 1.2.3.4/32, + 2.3.4.0/26, + 3.4.5.0/24, + 4.5.0.0/21 + ]; +} diff --git a/birdwatcher/testdata/bird/filter_up b/birdwatcher/testdata/bird/filter_up new file mode 100644 index 0000000..b326196 --- /dev/null +++ b/birdwatcher/testdata/bird/filter_up @@ -0,0 +1,5 @@ +# DO NOT EDIT MANUALLY +function match_route() -> bool +{ + return true; +} diff --git a/birdwatcher/testdata/bird/filter_up_compat b/birdwatcher/testdata/bird/filter_up_compat new file mode 100644 index 0000000..20eaa2e --- /dev/null +++ b/birdwatcher/testdata/bird/filter_up_compat @@ -0,0 +1,5 @@ +# DO NOT EDIT MANUALLY +function match_route() +{ + return true; +} diff --git a/birdwatcher/testdata/config/minimal b/birdwatcher/testdata/config/minimal index fcf8d10..5361317 100644 --- a/birdwatcher/testdata/config/minimal +++ b/birdwatcher/testdata/config/minimal @@ -1,4 +1,3 @@ [services] [services."foo"] command = "/usr/bin/true" - prefixes = ["192.168.0.0/24"]