Skip to content

Commit

Permalink
Reduce amount of firewall status updates through monitor controller.
Browse files Browse the repository at this point in the history
Currently updates happen as often as the firewall-controller updates the monitor. Most of the time this is completely irrelevant and just producing noise.
  • Loading branch information
Gerrit91 committed Apr 22, 2024
1 parent a33b405 commit 7fbb448
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 0 deletions.
24 changes: 24 additions & 0 deletions controllers/monitor/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"strconv"
"time"

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
v2 "github.com/metal-stack/firewall-controller-manager/api/v2"
"github.com/metal-stack/firewall-controller-manager/controllers"
"github.com/metal-stack/firewall-controller-manager/controllers/firewall"
Expand Down Expand Up @@ -43,8 +45,14 @@ func (c *controller) updateFirewallStatus(r *controllers.Ctx[*v2.FirewallMonitor
return nil, fmt.Errorf("associated firewall of monitor not found: %w", err)
}

old := fw.DeepCopy()

firewall.SetFirewallStatusFromMonitor(fw, r.Target)

if !significantFirewallStatusChange(old.Status, fw.Status) {
return fw, nil
}

err = c.c.GetSeedClient().Status().Update(r.Ctx, fw)
if err != nil {
return nil, fmt.Errorf("unable to update firewall status: %w", err)
Expand Down Expand Up @@ -128,3 +136,19 @@ func findCorrespondingSet(ctx context.Context, c client.Client, fw *v2.Firewall)

return set, nil
}

func significantFirewallStatusChange(o, n v2.FirewallStatus) bool {
// only consider relevant fields, we only care for controller status updates in this controller
// (to immediately see when the controller has connected)
// after that the firewall controller syncs the status every 2 minutes anyway

if o.ControllerStatus == nil && n.ControllerStatus != nil {
return true
}

if !cmp.Equal(o.Conditions, n.Conditions, cmpopts.IgnoreFields(v2.Condition{}, "Message", "LastUpdateTime", "LastTransitionTime")) {
return true
}

return false
}
72 changes: 72 additions & 0 deletions controllers/monitor/reconcile_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package monitor

import (
"fmt"
"testing"
"time"

"github.com/google/go-cmp/cmp"
v2 "github.com/metal-stack/firewall-controller-manager/api/v2"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func Test_significantFirewallStatusChange(t *testing.T) {
now := time.Now()

tests := []struct {
name string
o v2.FirewallStatus
n v2.FirewallStatus
want bool
}{
{
name: "controller registers",
o: v2.FirewallStatus{
ControllerStatus: nil,
},
n: v2.FirewallStatus{
ControllerStatus: &v2.ControllerConnection{
ActualVersion: "1.2.3",
Updated: v1.NewTime(now),
},
},
want: true,
},
{
name: "conditions not really changed",
o: v2.FirewallStatus{
Conditions: v2.Conditions{
v2.NewCondition(v2.FirewallControllerSeedConnected, v2.ConditionTrue, "Connected", fmt.Sprintf("Controller reconciled firewall at %s.", now.Add(-1*time.Minute))),
},
},
n: v2.FirewallStatus{
Conditions: v2.Conditions{
v2.NewCondition(v2.FirewallControllerSeedConnected, v2.ConditionTrue, "Connected", fmt.Sprintf("Controller reconciled firewall at %s.", now.Add(1*time.Minute))),
},
},
want: false,
},
{
name: "controller reconnects",
o: v2.FirewallStatus{
Conditions: v2.Conditions{
v2.NewCondition(v2.FirewallControllerConnected, v2.ConditionFalse, "StoppedReconciling", fmt.Sprintf("Controller has stopped reconciling since %s to shoot.", now)),
},
},
n: v2.FirewallStatus{
Conditions: v2.Conditions{
v2.NewCondition(v2.FirewallControllerConnected, v2.ConditionTrue, "Connected", fmt.Sprintf("Controller reconciled shoot at %s.", now)),
},
},
want: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := significantFirewallStatusChange(tt.o, tt.n)
if diff := cmp.Diff(tt.want, got); diff != "" {
t.Errorf("diff (+got -want):\n %s", diff)
}
})
}
}

0 comments on commit 7fbb448

Please sign in to comment.