Skip to content

Commit

Permalink
review fixes
Browse files Browse the repository at this point in the history
address review comments, improve tests, added mutex guard preventing re-initialization of L4 healhtchecks, renamed a few places, added comments

It is important that all controllers in a single test case use the same fakeHC, I have fixed this in
TestHealthCheckFirewallDeletionWithILB
TestHealthCheckFirewallDeletionWithNetLB
  • Loading branch information
cezarygerard committed May 6, 2022
1 parent 9e17971 commit 570dc6e
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 4 deletions.
7 changes: 5 additions & 2 deletions pkg/loadbalancers/l4_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -626,8 +626,11 @@ func TestHealthCheckFirewallDeletionWithNetLB(t *testing.T) {
// When NetLB health check uses the same firewall rules we expect that hc firewall rule will not be deleted.
haName, hcFwName := l.namer.L4HealthCheck(l.Service.Namespace, l.Service.Name, true)
firewall, err := l.cloud.GetFirewall(hcFwName)
if err != nil || firewall == nil {
t.Errorf("Expected firewall exists err: %v, fwR: %v", err, firewall)
if err != nil {
t.Errorf("Expected error: firewall exists, got %v", err)
}
if firewall == nil {
t.Error("Healthcheck Firewall should still exist, got nil")
}

// The healthcheck itself should be deleted.
Expand Down
7 changes: 5 additions & 2 deletions pkg/loadbalancers/l4netlb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,11 @@ func TestHealthCheckFirewallDeletionWithILB(t *testing.T) {
// When ILB health check uses the same firewall rules we expect that hc firewall rule will not be deleted.
hcName, hcFwName := l4NetLB.namer.L4HealthCheck(l4NetLB.Service.Namespace, l4NetLB.Service.Name, true)
firewall, err := l4NetLB.cloud.GetFirewall(hcFwName)
if err != nil || firewall == nil {
t.Errorf("Expected firewall exists err: %v, fwR: %v", err, firewall)
if err != nil {
t.Errorf("Expected error: firewall exists, got %v", err)
}
if firewall == nil {
t.Error("Healthcheck Firewall should still exist, got nil")
}

// The healthcheck itself should be deleted.
Expand Down

0 comments on commit 570dc6e

Please sign in to comment.