Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new metric status PersistentError for L4 DualStack #1944

Merged
merged 1 commit into from
Feb 17, 2023
Merged

Add new metric status PersistentError for L4 DualStack #1944

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions pkg/l4lb/l4controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -638,6 +638,32 @@ func TestProcessDualStackServiceOnUserError(t *testing.T) {
}
}

func TestDualStackILBStatusForErrorSync(t *testing.T) {
l4c := newServiceController(t, newFakeGCEWithUserNoIPv6SubnetError())
l4c.enableDualStack = true
(l4c.ctx.Cloud.Compute().(*cloud.MockGCE)).MockForwardingRules.InsertHook = mock.InsertForwardingRulesInternalErrHook

newSvc := test.NewL4ILBDualStackService(8080, api_v1.ProtocolTCP, []api_v1.IPFamily{api_v1.IPv4Protocol, api_v1.IPv6Protocol}, api_v1.ServiceExternalTrafficPolicyTypeCluster)
addILBService(l4c, newSvc)
addNEG(l4c, newSvc)
syncResult := l4c.processServiceCreateOrUpdate(newSvc)
if syncResult.Error == nil {
t.Fatalf("Failed to generate error when syncing service %s", newSvc.Name)
}
if syncResult.MetricsState.IsUserError {
t.Errorf("syncResult.MetricsState.IsUserError should be false, got true")
}
if syncResult.MetricsState.InSuccess {
t.Errorf("syncResult.MetricsState.InSuccess should be false, got true")
}
if syncResult.DualStackMetricsState.Status != metrics.StatusError {
t.Errorf("syncResult.DualStackMetricsState.Status should be %s, got %s", metrics.StatusError, syncResult.DualStackMetricsState.Status)
}
if syncResult.DualStackMetricsState.FirstSyncErrorTime == nil {
t.Fatalf("Metric status FirstSyncErrorTime for service %s/%s mismatch, expected: not nil, received: nil", newSvc.Namespace, newSvc.Name)
}
}

func TestProcessUpdateILBIPFamilies(t *testing.T) {
testCases := []struct {
desc string
Expand Down
6 changes: 4 additions & 2 deletions pkg/loadbalancers/l4.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,11 +283,12 @@ func (l4 *L4) getFRNameWithProtocol(protocol string) string {
func (l4 *L4) EnsureInternalLoadBalancer(nodeNames []string, svc *corev1.Service) *L4ILBSyncResult {
l4.Service = svc

startTime := time.Now()
result := &L4ILBSyncResult{
Annotations: make(map[string]string),
StartTime: time.Now(),
StartTime: startTime,
SyncType: SyncTypeCreate,
DualStackMetricsState: metrics.InitServiceDualStackMetricsState(svc),
DualStackMetricsState: metrics.InitServiceDualStackMetricsState(svc, &startTime),
}

// If service already has an IP assigned, treat it as an update instead of a new Loadbalancer.
Expand Down Expand Up @@ -388,6 +389,7 @@ func (l4 *L4) EnsureInternalLoadBalancer(nodeNames []string, svc *corev1.Service
}
if l4.enableDualStack {
result.DualStackMetricsState.Status = metrics.StatusSuccess
result.DualStackMetricsState.FirstSyncErrorTime = nil
}
return result
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/loadbalancers/l4netlb.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func NewL4SyncResult(syncType string, svc *corev1.Service) *L4NetLBSyncResult {
StartTime: startTime,
SyncType: syncType,
MetricsState: metrics.InitL4NetLBServiceState(&startTime),
DualStackMetricsState: metrics.InitServiceDualStackMetricsState(svc),
DualStackMetricsState: metrics.InitServiceDualStackMetricsState(svc, &startTime),
}
return result
}
Expand Down
208 changes: 161 additions & 47 deletions pkg/metrics/l4metrics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,56 +381,82 @@ func TestComputeL4NetLBMetrics(t *testing.T) {

func TestComputeL4NetLBDualStackMetrics(t *testing.T) {
t.Parallel()

currTime := time.Now()
before10min := currTime.Add(-10 * time.Minute)
before20min := currTime.Add(-20 * time.Minute)

for _, tc := range []struct {
desc string
serviceStates []L4DualStackServiceState
expectL4NetLBDualStackCount map[L4DualStackServiceState]int
expectL4NetLBDualStackCount map[L4DualStackServiceLabels]int
}{
{
desc: "empty input",
serviceStates: []L4DualStackServiceState{},
expectL4NetLBDualStackCount: map[L4DualStackServiceState]int{},
expectL4NetLBDualStackCount: map[L4DualStackServiceLabels]int{},
},
{
desc: "one l4 NetLB dual-stack service",
serviceStates: []L4DualStackServiceState{
newL4DualStackServiceState("IPv4", "SingleStack", StatusSuccess),
newL4DualStackServiceState("IPv4", "SingleStack", StatusSuccess, nil),
},
expectL4NetLBDualStackCount: map[L4DualStackServiceState]int{
L4DualStackServiceState{"IPv4", "SingleStack", StatusSuccess}: 1,
expectL4NetLBDualStackCount: map[L4DualStackServiceLabels]int{
L4DualStackServiceLabels{"IPv4", "SingleStack", StatusSuccess}: 1,
},
},
{
desc: "l4 NetLB dual-stack service in error state",
serviceStates: []L4DualStackServiceState{
newL4DualStackServiceState("IPv4", "SingleStack", StatusError),
newL4DualStackServiceState("IPv4", "SingleStack", StatusError, nil),
},
expectL4NetLBDualStackCount: map[L4DualStackServiceLabels]int{
L4DualStackServiceLabels{"IPv4", "SingleStack", StatusError}: 1,
},
},
{
desc: "l4 NetLB dual-stack service in error state, for 10 minutes",
serviceStates: []L4DualStackServiceState{
newL4DualStackServiceState("IPv4", "SingleStack", StatusError, &before10min),
},
expectL4NetLBDualStackCount: map[L4DualStackServiceState]int{
L4DualStackServiceState{"IPv4", "SingleStack", StatusError}: 1,
expectL4NetLBDualStackCount: map[L4DualStackServiceLabels]int{
L4DualStackServiceLabels{"IPv4", "SingleStack", StatusError}: 1,
},
},
{
desc: "l4 NetLB dual-stack service in error state, for 20 minutes",
serviceStates: []L4DualStackServiceState{
newL4DualStackServiceState("IPv4", "SingleStack", StatusError, &before20min),
},
expectL4NetLBDualStackCount: map[L4DualStackServiceLabels]int{
L4DualStackServiceLabels{"IPv4", "SingleStack", StatusPersistentError}: 1,
},
},
{
desc: "L4 NetLB dual-stack service with IPv4,IPv6 Families",
serviceStates: []L4DualStackServiceState{
newL4DualStackServiceState("IPv4,IPv6", "RequireDualStack", StatusSuccess),
newL4DualStackServiceState("IPv4,IPv6", "RequireDualStack", StatusSuccess, nil),
},
expectL4NetLBDualStackCount: map[L4DualStackServiceState]int{
L4DualStackServiceState{"IPv4,IPv6", "RequireDualStack", StatusSuccess}: 1,
expectL4NetLBDualStackCount: map[L4DualStackServiceLabels]int{
L4DualStackServiceLabels{"IPv4,IPv6", "RequireDualStack", StatusSuccess}: 1,
},
},
{
desc: "many l4 NetLB dual-stack services",
serviceStates: []L4DualStackServiceState{
newL4DualStackServiceState("IPv4,IPv6", "RequireDualStack", StatusSuccess),
newL4DualStackServiceState("IPv4,IPv6", "RequireDualStack", StatusSuccess),
newL4DualStackServiceState("IPv4", "SingleStack", StatusError),
newL4DualStackServiceState("IPv6", "SingleStack", StatusSuccess),
newL4DualStackServiceState("IPv6", "SingleStack", StatusSuccess),
},
expectL4NetLBDualStackCount: map[L4DualStackServiceState]int{
L4DualStackServiceState{"IPv4,IPv6", "RequireDualStack", StatusSuccess}: 2,
L4DualStackServiceState{"IPv4", "SingleStack", StatusError}: 1,
L4DualStackServiceState{"IPv6", "SingleStack", StatusSuccess}: 2,
newL4DualStackServiceState("IPv4,IPv6", "RequireDualStack", StatusSuccess, nil),
newL4DualStackServiceState("IPv4,IPv6", "RequireDualStack", StatusSuccess, nil),
newL4DualStackServiceState("IPv4", "SingleStack", StatusError, nil),
newL4DualStackServiceState("IPv6", "SingleStack", StatusSuccess, nil),
newL4DualStackServiceState("IPv6", "SingleStack", StatusSuccess, nil),
newL4DualStackServiceState("IPv4", "SingleStack", StatusError, &before10min),
newL4DualStackServiceState("IPv4", "SingleStack", StatusError, &before20min),
},
expectL4NetLBDualStackCount: map[L4DualStackServiceLabels]int{
L4DualStackServiceLabels{"IPv4,IPv6", "RequireDualStack", StatusSuccess}: 2,
L4DualStackServiceLabels{"IPv4", "SingleStack", StatusError}: 2,
L4DualStackServiceLabels{"IPv6", "SingleStack", StatusSuccess}: 2,
L4DualStackServiceLabels{"IPv4", "SingleStack", StatusPersistentError}: 1,
},
},
} {
Expand Down Expand Up @@ -490,6 +516,58 @@ func TestRetryPeriodForL4NetLBServices(t *testing.T) {
}
}

func TestRetryPeriodForL4ILBDualStackServices(t *testing.T) {
t.Parallel()
currTime := time.Now()
before5min := currTime.Add(-5 * time.Minute)

svcName1 := "svc1"
metrics := FakeControllerMetrics()

errorState := newL4DualStackServiceState("IPv4", "SingleStack", StatusError, &currTime)
metrics.SetL4ILBDualStackService(svcName1, errorState)

// change FirstSyncErrorTime and verify it will not change metrics state
errorState.FirstSyncErrorTime = &before5min
metrics.SetL4ILBDualStackService(svcName1, errorState)
state, ok := metrics.l4ILBDualStackServiceMap[svcName1]
if !ok {
t.Fatalf("state should be set")
}
if *state.FirstSyncErrorTime != currTime {
t.Errorf("FirstSyncErrorTime should not change, expected %v, got %v", currTime, *state.FirstSyncErrorTime)
}
if state.Status != StatusError {
t.Errorf("Expected status %s, got %s", StatusError, state.Status)
}
}

func TestRetryPeriodForL4NetLBDualStackServices(t *testing.T) {
t.Parallel()
currTime := time.Now()
before5min := currTime.Add(-5 * time.Minute)

svcName1 := "svc1"
metrics := FakeControllerMetrics()

errorState := newL4DualStackServiceState("IPv4", "SingleStack", StatusError, &currTime)
metrics.SetL4NetLBDualStackService(svcName1, errorState)

// change FirstSyncErrorTime and verify it will not change metrics state
errorState.FirstSyncErrorTime = &before5min
metrics.SetL4NetLBDualStackService(svcName1, errorState)
state, ok := metrics.l4NetLBDualStackServiceMap[svcName1]
if !ok {
t.Fatalf("state should be set")
}
if *state.FirstSyncErrorTime != currTime {
t.Errorf("FirstSyncErrorTime should not change, expected %v, got %v", currTime, *state.FirstSyncErrorTime)
}
if state.Status != StatusError {
t.Errorf("Expected status %s, got %s", StatusError, state.Status)
}
}

func checkMetricsComputation(newMetrics *ControllerMetrics, expErrorCount, expSvcCount int) error {
got := newMetrics.computeL4NetLBMetrics()
if got.inError != expErrorCount {
Expand All @@ -503,56 +581,89 @@ func checkMetricsComputation(newMetrics *ControllerMetrics, expErrorCount, expSv

func TestComputeL4ILBDualStackMetrics(t *testing.T) {
t.Parallel()

currTime := time.Now()
before10min := currTime.Add(-10 * time.Minute)
before20min := currTime.Add(-20 * time.Minute)

for _, tc := range []struct {
desc string
serviceStates []L4DualStackServiceState
expectL4ILBDualStackCount map[L4DualStackServiceState]int
expectL4ILBDualStackCount map[L4DualStackServiceLabels]int
}{
{
desc: "empty input",
serviceStates: []L4DualStackServiceState{},
expectL4ILBDualStackCount: map[L4DualStackServiceState]int{},
expectL4ILBDualStackCount: map[L4DualStackServiceLabels]int{},
},
{
desc: "one l4 ilb dual-stack service",
serviceStates: []L4DualStackServiceState{
newL4DualStackServiceState("IPv4", "SingleStack", StatusSuccess),
newL4DualStackServiceState("IPv4", "SingleStack", StatusSuccess, nil),
},
expectL4ILBDualStackCount: map[L4DualStackServiceState]int{
L4DualStackServiceState{"IPv4", "SingleStack", StatusSuccess}: 1,
expectL4ILBDualStackCount: map[L4DualStackServiceLabels]int{
L4DualStackServiceLabels{"IPv4", "SingleStack", StatusSuccess}: 1,
},
},
{
desc: "l4 ilb dual-stack service in error state",
serviceStates: []L4DualStackServiceState{
newL4DualStackServiceState("IPv4", "SingleStack", StatusError),
newL4DualStackServiceState("IPv4", "SingleStack", StatusError, nil),
},
expectL4ILBDualStackCount: map[L4DualStackServiceState]int{
L4DualStackServiceState{"IPv4", "SingleStack", StatusError}: 1,
expectL4ILBDualStackCount: map[L4DualStackServiceLabels]int{
L4DualStackServiceLabels{"IPv4", "SingleStack", StatusError}: 1,
},
},
{
desc: "L4 ILB dual-stack service with IPv4,IPv6 Families",
desc: "l4 ilb dual-stack service in error state, for 10 minutes",
serviceStates: []L4DualStackServiceState{
newL4DualStackServiceState("IPv4,IPv6", "RequireDualStack", StatusSuccess),
newL4DualStackServiceState("IPv4", "SingleStack", StatusError, &before10min),
},
expectL4ILBDualStackCount: map[L4DualStackServiceState]int{
L4DualStackServiceState{"IPv4,IPv6", "RequireDualStack", StatusSuccess}: 1,
expectL4ILBDualStackCount: map[L4DualStackServiceLabels]int{
L4DualStackServiceLabels{
"IPv4",
"SingleStack",
StatusError,
}: 1,
},
},
{
desc: "many l4 ilb dual-stack services",
desc: "l4 ilb dual-stack service in error state, for 20 minutes",
serviceStates: []L4DualStackServiceState{
newL4DualStackServiceState("IPv4,IPv6", "RequireDualStack", StatusSuccess),
newL4DualStackServiceState("IPv4,IPv6", "RequireDualStack", StatusSuccess),
newL4DualStackServiceState("IPv4", "SingleStack", StatusError),
newL4DualStackServiceState("IPv6", "SingleStack", StatusSuccess),
newL4DualStackServiceState("IPv6", "SingleStack", StatusSuccess),
newL4DualStackServiceState("IPv4", "SingleStack", StatusError, &before20min),
},
expectL4ILBDualStackCount: map[L4DualStackServiceState]int{
L4DualStackServiceState{"IPv4,IPv6", "RequireDualStack", StatusSuccess}: 2,
L4DualStackServiceState{"IPv4", "SingleStack", StatusError}: 1,
L4DualStackServiceState{"IPv6", "SingleStack", StatusSuccess}: 2,
expectL4ILBDualStackCount: map[L4DualStackServiceLabels]int{
L4DualStackServiceLabels{
"IPv4",
"SingleStack",
StatusPersistentError,
}: 1,
},
},
{
desc: "L4 ILB dual-stack service with IPv4,IPv6 Families",
serviceStates: []L4DualStackServiceState{
newL4DualStackServiceState("IPv4,IPv6", "RequireDualStack", StatusSuccess, nil),
},
expectL4ILBDualStackCount: map[L4DualStackServiceLabels]int{
L4DualStackServiceLabels{"IPv4,IPv6", "RequireDualStack", StatusSuccess}: 1},
},
{
desc: "many l4 ilb dual-stack services",
serviceStates: []L4DualStackServiceState{
newL4DualStackServiceState("IPv4,IPv6", "RequireDualStack", StatusSuccess, nil),
newL4DualStackServiceState("IPv4,IPv6", "RequireDualStack", StatusSuccess, nil),
newL4DualStackServiceState("IPv4", "SingleStack", StatusError, nil),
newL4DualStackServiceState("IPv6", "SingleStack", StatusSuccess, nil),
newL4DualStackServiceState("IPv6", "SingleStack", StatusSuccess, nil),
newL4DualStackServiceState("IPv4", "SingleStack", StatusError, &before10min),
newL4DualStackServiceState("IPv4", "SingleStack", StatusError, &before20min),
},
expectL4ILBDualStackCount: map[L4DualStackServiceLabels]int{
L4DualStackServiceLabels{"IPv4,IPv6", "RequireDualStack", StatusSuccess}: 2,
L4DualStackServiceLabels{"IPv4", "SingleStack", StatusError}: 2,
L4DualStackServiceLabels{"IPv6", "SingleStack", StatusSuccess}: 2,
L4DualStackServiceLabels{"IPv4", "SingleStack", StatusPersistentError}: 1,
},
},
} {
Expand All @@ -571,10 +682,13 @@ func TestComputeL4ILBDualStackMetrics(t *testing.T) {
}
}

func newL4DualStackServiceState(ipFamilies string, ipFamilyPolicy string, status L4DualStackServiceStatus) L4DualStackServiceState {
func newL4DualStackServiceState(ipFamilies string, ipFamilyPolicy string, status L4DualStackServiceStatus, firstSyncErrorTime *time.Time) L4DualStackServiceState {
return L4DualStackServiceState{
IPFamilies: ipFamilies,
IPFamilyPolicy: ipFamilyPolicy,
Status: status,
L4DualStackServiceLabels: L4DualStackServiceLabels{
IPFamilies: ipFamilies,
IPFamilyPolicy: ipFamilyPolicy,
Status: status,
},
FirstSyncErrorTime: firstSyncErrorTime,
}
}
Loading