diff --git a/pkg/l4lb/l4netlbcontroller.go b/pkg/l4lb/l4netlbcontroller.go index f7c9a27954..e9ffe94ef4 100644 --- a/pkg/l4lb/l4netlbcontroller.go +++ b/pkg/l4lb/l4netlbcontroller.go @@ -18,6 +18,8 @@ package l4lb import ( "fmt" + "reflect" + "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud" "github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta" v1 "k8s.io/api/core/v1" @@ -35,7 +37,6 @@ import ( "k8s.io/ingress-gce/pkg/utils/common" "k8s.io/ingress-gce/pkg/utils/namer" "k8s.io/klog" - "reflect" ) const l4NetLBControllerName = "l4netlb-controller" @@ -372,7 +373,7 @@ func (lc *L4NetLBController) syncInternal(service *v1.Service) *loadbalancers.L4 syncResult.Error = fmt.Errorf("failed to set resource annotations, err: %w", err) return syncResult } - syncResult.SetMetricsForSuccessfulService() + syncResult.SetMetricsForSuccessfulServiceSync() return syncResult } diff --git a/pkg/l4lb/l4netlbcontroller_test.go b/pkg/l4lb/l4netlbcontroller_test.go index 818f64948c..9eef15d1d9 100644 --- a/pkg/l4lb/l4netlbcontroller_test.go +++ b/pkg/l4lb/l4netlbcontroller_test.go @@ -671,6 +671,43 @@ func TestProcessServiceDeletionFailed(t *testing.T) { } } +func TestServiceStatusForErrorSync(t *testing.T) { + lc := newL4NetLBServiceController() + (lc.ctx.Cloud.Compute().(*cloud.MockGCE)).MockForwardingRules.InsertHook = mock.InsertForwardingRulesInternalErrHook + + svc := test.NewL4NetLBRBSService(8080) + addNetLBService(lc, svc) + + syncResult := lc.syncInternal(svc) + if syncResult.Error == nil { + t.Errorf("Expected error in sync controller") + } + if syncResult.MetricsState.InSuccess == true { + t.Fatalf("Metric status InSuccess for service %s/%s mismatch, expected: true, received: false", svc.Namespace, svc.Name) + } + if syncResult.MetricsState.FirstSyncErrorTime == nil { + t.Fatalf("Metric status FirstSyncErrorTime for service %s/%s mismatch, expected: not nil, received: nil", svc.Namespace, svc.Name) + } +} + +func TestServiceStatusForSuccessSync(t *testing.T) { + lc := newL4NetLBServiceController() + + svc := test.NewL4NetLBRBSService(8080) + addNetLBService(lc, svc) + + syncResult := lc.syncInternal(svc) + if syncResult.Error != nil { + t.Errorf("Unexpected error in sync controller") + } + if syncResult.MetricsState.InSuccess != true { + t.Fatalf("Metric status InSuccess for service %s/%s mismatch, expected: false, received: true", svc.Namespace, svc.Name) + } + if syncResult.MetricsState.FirstSyncErrorTime != nil { + t.Fatalf("Metric status FirstSyncErrorTime for service %s/%s mismatch, expected: nil, received: %v", svc.Namespace, svc.Name, syncResult.MetricsState.FirstSyncErrorTime) + } +} + func TestProcessServiceUpdate(t *testing.T) { flags.F.MaxIgSize = 1000 diff --git a/pkg/loadbalancers/l4netlb.go b/pkg/loadbalancers/l4netlb.go index 4dbe25c791..f6e2ed6e28 100644 --- a/pkg/loadbalancers/l4netlb.go +++ b/pkg/loadbalancers/l4netlb.go @@ -64,8 +64,18 @@ type L4NetLBSyncResult struct { StartTime time.Time } -// SetMetricsForSuccessfulService should be call after successful sync. -func (r *L4NetLBSyncResult) SetMetricsForSuccessfulService() { +func NewL4SynResult(syncType string) *L4NetLBSyncResult { + result := &L4NetLBSyncResult{ + Annotations: make(map[string]string), + StartTime: time.Now(), + SyncType: syncType, + } + result.MetricsState = metrics.InitL4NetLBServiceState(&result.StartTime) + return result +} + +// SetMetricsForSuccessfulServiceSync should be call after successful sync. +func (r *L4NetLBSyncResult) SetMetricsForSuccessfulServiceSync() { r.MetricsState.FirstSyncErrorTime = nil r.MetricsState.InSuccess = true } @@ -101,11 +111,7 @@ func (l4netlb *L4NetLB) createKey(name string) (*meta.Key, error) { // It returns a LoadBalancerStatus with the updated ForwardingRule IP address. // This function does not link instances to Backend Service. func (l4netlb *L4NetLB) EnsureFrontend(nodeNames []string, svc *corev1.Service) *L4NetLBSyncResult { - result := &L4NetLBSyncResult{ - Annotations: make(map[string]string), - StartTime: time.Now(), - SyncType: SyncTypeCreate, - } + result := NewL4SynResult(SyncTypeCreate) // If service already has an IP assigned, treat it as an update instead of a new Loadbalancer. if len(svc.Status.LoadBalancer.Ingress) > 0 { @@ -161,7 +167,7 @@ func (l4netlb *L4NetLB) EnsureFrontend(nodeNames []string, svc *corev1.Service) // EnsureLoadBalancerDeleted performs a cleanup of all GCE resources for the given loadbalancer service. // It is health check, firewall rules and backend service func (l4netlb *L4NetLB) EnsureLoadBalancerDeleted(svc *corev1.Service) *L4NetLBSyncResult { - result := &L4NetLBSyncResult{SyncType: SyncTypeDelete, StartTime: time.Now()} + result := NewL4SynResult(SyncTypeDelete) frName := l4netlb.GetFRName() key, err := l4netlb.createKey(frName) diff --git a/pkg/metrics/types.go b/pkg/metrics/types.go index a7314e12e1..f4dc5c8d77 100644 --- a/pkg/metrics/types.go +++ b/pkg/metrics/types.go @@ -87,6 +87,11 @@ type L4NetLBServiceState struct { FirstSyncErrorTime *time.Time } +// InitL4NetLBServiceState sets FirstSyncErrorTime +func InitL4NetLBServiceState(syncTime *time.Time) L4NetLBServiceState { + return L4NetLBServiceState{FirstSyncErrorTime: syncTime} +} + // IngressMetricsCollector is an interface to update/delete ingress states in the cache // that is used for computing ingress usage metrics. type IngressMetricsCollector interface {