Skip to content

Commit

Permalink
Merge pull request #1937 from panslava/ipv6-netlb-metrics
Browse files Browse the repository at this point in the history
Add L4 NetLB Dual-Stack Metrics
  • Loading branch information
k8s-ci-robot authored Feb 16, 2023
2 parents c723cd1 + d6dc142 commit 839191b
Show file tree
Hide file tree
Showing 7 changed files with 233 additions and 116 deletions.
8 changes: 4 additions & 4 deletions pkg/l4lb/l4controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -497,21 +497,21 @@ func (l4c *L4Controller) publishMetrics(result *loadbalancers.L4ILBSyncResult, n
}
switch result.SyncType {
case loadbalancers.SyncTypeCreate, loadbalancers.SyncTypeUpdate:
klog.V(6).Infof("Internal L4 Loadbalancer for Service %s ensured, updating its state %v in metrics cache", namespacedName, result.MetricsState)
klog.V(2).Infof("Internal L4 Loadbalancer for Service %s ensured, updating its state %v in metrics cache", namespacedName, result.MetricsState)
l4c.ctx.ControllerMetrics.SetL4ILBService(namespacedName, result.MetricsState)
if l4c.enableDualStack {
klog.V(6).Infof("Internal L4 DualStack Loadbalancer for Service %s ensured, updating its state %v in metrics cache", namespacedName, result.MetricsState)
klog.V(2).Infof("Internal L4 DualStack Loadbalancer for Service %s ensured, updating its state %v in metrics cache", namespacedName, result.DualStackMetricsState)
l4c.ctx.ControllerMetrics.SetL4ILBDualStackService(namespacedName, result.DualStackMetricsState)
}
l4metrics.PublishILBSyncMetrics(result.Error == nil, result.SyncType, result.GCEResourceInError, utils.GetErrorType(result.Error), result.StartTime)

case loadbalancers.SyncTypeDelete:
// if service is successfully deleted, remove it from cache
if result.Error == nil {
klog.V(6).Infof("Internal L4 Loadbalancer for Service %s deleted, removing its state from metrics cache", namespacedName)
klog.V(2).Infof("Internal L4 Loadbalancer for Service %s deleted, removing its state from metrics cache", namespacedName)
l4c.ctx.ControllerMetrics.DeleteL4ILBService(namespacedName)
if l4c.enableDualStack {
klog.V(6).Infof("Internal L4 DualStack LoadBalancer for Service %s deleted, removing its state from metrics cache", namespacedName)
klog.V(2).Infof("Internal L4 DualStack LoadBalancer for Service %s deleted, removing its state from metrics cache", namespacedName)
l4c.ctx.ControllerMetrics.DeleteL4ILBDualStackService(namespacedName)
}
}
Expand Down
12 changes: 10 additions & 2 deletions pkg/l4lb/l4netlbcontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -633,14 +633,22 @@ func (lc *L4NetLBController) publishMetrics(result *loadbalancers.L4NetLBSyncRes
namespacedName := types.NamespacedName{Name: name, Namespace: namespace}.String()
switch result.SyncType {
case loadbalancers.SyncTypeCreate, loadbalancers.SyncTypeUpdate:
klog.V(4).Infof("External L4 Loadbalancer for Service %s ensured, updating its state %v in metrics cache", namespacedName, result.MetricsState)
klog.V(2).Infof("External L4 Loadbalancer for Service %s ensured, updating its state %v in metrics cache", namespacedName, result.MetricsState)
lc.ctx.ControllerMetrics.SetL4NetLBService(namespacedName, result.MetricsState)
if lc.enableDualStack {
klog.V(2).Infof("External L4 DualStack Loadbalancer for Service %s ensured, updating its state %v in metrics cache", namespacedName, result.DualStackMetricsState)
lc.ctx.ControllerMetrics.SetL4NetLBDualStackService(namespacedName, result.DualStackMetricsState)
}
lc.publishSyncMetrics(result)
case loadbalancers.SyncTypeDelete:
// if service is successfully deleted, remove it from cache
if result.Error == nil {
klog.V(4).Infof("External L4 Loadbalancer for Service %s deleted, removing its state from metrics cache", namespacedName)
klog.V(2).Infof("External L4 Loadbalancer for Service %s deleted, removing its state from metrics cache", namespacedName)
lc.ctx.ControllerMetrics.DeleteL4NetLBService(namespacedName)
if lc.enableDualStack {
klog.V(2).Infof("External L4 Loadbalancer for Service %s deleted, removing its state from metrics cache", namespacedName)
lc.ctx.ControllerMetrics.DeleteL4NetLBDualStackService(namespacedName)
}
}
lc.publishSyncMetrics(result)
default:
Expand Down
24 changes: 2 additions & 22 deletions pkg/loadbalancers/l4.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ type L4ILBSyncResult struct {
GCEResourceInError string
Status *corev1.LoadBalancerStatus
MetricsState metrics.L4ILBServiceState
DualStackMetricsState metrics.L4ILBDualStackServiceState
DualStackMetricsState metrics.L4DualStackServiceState
SyncType string
StartTime time.Time
}
Expand Down Expand Up @@ -287,7 +287,7 @@ func (l4 *L4) EnsureInternalLoadBalancer(nodeNames []string, svc *corev1.Service
Annotations: make(map[string]string),
StartTime: time.Now(),
SyncType: SyncTypeCreate,
DualStackMetricsState: l4.getInitialDualStackMetricsState(),
DualStackMetricsState: metrics.InitServiceDualStackMetricsState(svc),
}

// If service already has an IP assigned, treat it as an update instead of a new Loadbalancer.
Expand Down Expand Up @@ -534,26 +534,6 @@ func (l4 *L4) hasAnnotation(annotationKey string) bool {
return false
}

func (l4 *L4) getInitialDualStackMetricsState() metrics.L4ILBDualStackServiceState {
// Always init stats with error, and update with Success when service was provisioned
state := metrics.L4ILBDualStackServiceState{
Status: metrics.StatusError,
}

var ipFamiliesStrings []string
for _, ipFamily := range l4.Service.Spec.IPFamilies {
ipFamiliesStrings = append(ipFamiliesStrings, string(ipFamily))
}
state.IPFamilies = strings.Join(ipFamiliesStrings, ",")

state.IPFamilyPolicy = ""
if l4.Service.Spec.IPFamilyPolicy != nil {
state.IPFamilyPolicy = string(*l4.Service.Spec.IPFamilyPolicy)
}

return state
}

func (l4 *L4) getOldForwardingRule() (*composite.ForwardingRule, error) {
bsName := l4.namer.L4Backend(l4.Service.Namespace, l4.Service.Name)
// Check if protocol has changed for this service. In this case, forwarding rule has different protocol and name
Expand Down
32 changes: 18 additions & 14 deletions pkg/loadbalancers/l4netlb.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,29 +57,33 @@ type L4NetLB struct {
// L4NetLBSyncResult contains information about the outcome of an L4 NetLB sync. It stores the list of resource name annotations,
// sync error, the GCE resource that hit the error along with the error type, metrics and more fields.
type L4NetLBSyncResult struct {
Annotations map[string]string
Error error
GCEResourceInError string
Status *corev1.LoadBalancerStatus
MetricsState metrics.L4NetLBServiceState
SyncType string
StartTime time.Time
Annotations map[string]string
Error error
GCEResourceInError string
Status *corev1.LoadBalancerStatus
MetricsState metrics.L4NetLBServiceState
DualStackMetricsState metrics.L4DualStackServiceState
SyncType string
StartTime time.Time
}

func NewL4SyncResult(syncType string) *L4NetLBSyncResult {
func NewL4SyncResult(syncType string, svc *corev1.Service) *L4NetLBSyncResult {
startTime := time.Now()
result := &L4NetLBSyncResult{
Annotations: make(map[string]string),
StartTime: time.Now(),
SyncType: syncType,
Annotations: make(map[string]string),
StartTime: startTime,
SyncType: syncType,
MetricsState: metrics.InitL4NetLBServiceState(&startTime),
DualStackMetricsState: metrics.InitServiceDualStackMetricsState(svc),
}
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
r.DualStackMetricsState.Status = metrics.StatusSuccess
}

type L4NetLBParams struct {
Expand Down Expand Up @@ -117,7 +121,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 := NewL4SyncResult(SyncTypeCreate)
result := NewL4SyncResult(SyncTypeCreate, svc)
// If service already has an IP assigned, treat it as an update instead of a new Loadbalancer.
if len(svc.Status.LoadBalancer.Ingress) > 0 {
result.SyncType = SyncTypeUpdate
Expand Down Expand Up @@ -281,7 +285,7 @@ func (l4netlb *L4NetLB) ensureIPv4NodesFirewall(nodeNames []string, ipAddress st
// 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 := NewL4SyncResult(SyncTypeDelete)
result := NewL4SyncResult(SyncTypeDelete, svc)
l4netlb.Service = svc

l4netlb.deleteIPv4ResourcesOnDelete(result)
Expand Down
152 changes: 99 additions & 53 deletions pkg/metrics/l4metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,76 @@ func TestComputeL4NetLBMetrics(t *testing.T) {
}
}

func TestComputeL4NetLBDualStackMetrics(t *testing.T) {
t.Parallel()
for _, tc := range []struct {
desc string
serviceStates []L4DualStackServiceState
expectL4NetLBDualStackCount map[L4DualStackServiceState]int
}{
{
desc: "empty input",
serviceStates: []L4DualStackServiceState{},
expectL4NetLBDualStackCount: map[L4DualStackServiceState]int{},
},
{
desc: "one l4 NetLB dual-stack service",
serviceStates: []L4DualStackServiceState{
newL4DualStackServiceState("IPv4", "SingleStack", StatusSuccess),
},
expectL4NetLBDualStackCount: map[L4DualStackServiceState]int{
L4DualStackServiceState{"IPv4", "SingleStack", StatusSuccess}: 1,
},
},
{
desc: "l4 NetLB dual-stack service in error state",
serviceStates: []L4DualStackServiceState{
newL4DualStackServiceState("IPv4", "SingleStack", StatusError),
},
expectL4NetLBDualStackCount: map[L4DualStackServiceState]int{
L4DualStackServiceState{"IPv4", "SingleStack", StatusError}: 1,
},
},
{
desc: "L4 NetLB dual-stack service with IPv4,IPv6 Families",
serviceStates: []L4DualStackServiceState{
newL4DualStackServiceState("IPv4,IPv6", "RequireDualStack", StatusSuccess),
},
expectL4NetLBDualStackCount: map[L4DualStackServiceState]int{
L4DualStackServiceState{"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,
},
},
} {
tc := tc
t.Run(tc.desc, func(t *testing.T) {
t.Parallel()
newMetrics := FakeControllerMetrics()
for i, serviceState := range tc.serviceStates {
newMetrics.SetL4NetLBDualStackService(fmt.Sprint(i), serviceState)
}
got := newMetrics.computeL4NetLBDualStackMetrics()
if diff := cmp.Diff(tc.expectL4NetLBDualStackCount, got); diff != "" {
t.Fatalf("Got diff for L4 NetLB Dual-Stack service counts (-want +got):\n%s", diff)
}
})
}
}

func newL4NetLBServiceState(inSuccess, managed, premium, userError bool, errorTimestamp *time.Time) L4NetLBServiceState {
return L4NetLBServiceState{
IsPremiumTier: premium,
Expand Down Expand Up @@ -435,78 +505,54 @@ func TestComputeL4ILBDualStackMetrics(t *testing.T) {
t.Parallel()
for _, tc := range []struct {
desc string
serviceStates []L4ILBDualStackServiceState
expectL4ILBDualStackCount map[L4ILBDualStackServiceState]int
serviceStates []L4DualStackServiceState
expectL4ILBDualStackCount map[L4DualStackServiceState]int
}{
{
desc: "empty input",
serviceStates: []L4ILBDualStackServiceState{},
expectL4ILBDualStackCount: map[L4ILBDualStackServiceState]int{},
serviceStates: []L4DualStackServiceState{},
expectL4ILBDualStackCount: map[L4DualStackServiceState]int{},
},
{
desc: "one l4 ilb dual-stack service",
serviceStates: []L4ILBDualStackServiceState{
newL4ILBDualStackServiceState("IPv4", "SingleStack", StatusSuccess),
serviceStates: []L4DualStackServiceState{
newL4DualStackServiceState("IPv4", "SingleStack", StatusSuccess),
},
expectL4ILBDualStackCount: map[L4ILBDualStackServiceState]int{
L4ILBDualStackServiceState{
"IPv4",
"SingleStack",
StatusSuccess,
}: 1,
expectL4ILBDualStackCount: map[L4DualStackServiceState]int{
L4DualStackServiceState{"IPv4", "SingleStack", StatusSuccess}: 1,
},
},
{
desc: "l4 ilb dual-stack service in error state",
serviceStates: []L4ILBDualStackServiceState{
newL4ILBDualStackServiceState("IPv4", "SingleStack", StatusError),
serviceStates: []L4DualStackServiceState{
newL4DualStackServiceState("IPv4", "SingleStack", StatusError),
},
expectL4ILBDualStackCount: map[L4ILBDualStackServiceState]int{
L4ILBDualStackServiceState{
"IPv4",
"SingleStack",
StatusError,
}: 1,
expectL4ILBDualStackCount: map[L4DualStackServiceState]int{
L4DualStackServiceState{"IPv4", "SingleStack", StatusError}: 1,
},
},
{
desc: "L4 ILB dual-stack service with IPv4,IPv6 Families",
serviceStates: []L4ILBDualStackServiceState{
newL4ILBDualStackServiceState("IPv4,IPv6", "RequireDualStack", StatusSuccess),
serviceStates: []L4DualStackServiceState{
newL4DualStackServiceState("IPv4,IPv6", "RequireDualStack", StatusSuccess),
},
expectL4ILBDualStackCount: map[L4ILBDualStackServiceState]int{
L4ILBDualStackServiceState{
"IPv4,IPv6",
"RequireDualStack",
StatusSuccess,
}: 1,
expectL4ILBDualStackCount: map[L4DualStackServiceState]int{
L4DualStackServiceState{"IPv4,IPv6", "RequireDualStack", StatusSuccess}: 1,
},
},
{
desc: "many l4 ilb dual-stack services",
serviceStates: []L4ILBDualStackServiceState{
newL4ILBDualStackServiceState("IPv4,IPv6", "RequireDualStack", StatusSuccess),
newL4ILBDualStackServiceState("IPv4,IPv6", "RequireDualStack", StatusSuccess),
newL4ILBDualStackServiceState("IPv4", "SingleStack", StatusError),
newL4ILBDualStackServiceState("IPv6", "SingleStack", StatusSuccess),
newL4ILBDualStackServiceState("IPv6", "SingleStack", StatusSuccess),
},
expectL4ILBDualStackCount: map[L4ILBDualStackServiceState]int{
L4ILBDualStackServiceState{
"IPv4,IPv6",
"RequireDualStack",
StatusSuccess,
}: 2,
L4ILBDualStackServiceState{
"IPv4",
"SingleStack",
StatusError,
}: 1,
L4ILBDualStackServiceState{
"IPv6",
"SingleStack",
StatusSuccess,
}: 2,
serviceStates: []L4DualStackServiceState{
newL4DualStackServiceState("IPv4,IPv6", "RequireDualStack", StatusSuccess),
newL4DualStackServiceState("IPv4,IPv6", "RequireDualStack", StatusSuccess),
newL4DualStackServiceState("IPv4", "SingleStack", StatusError),
newL4DualStackServiceState("IPv6", "SingleStack", StatusSuccess),
newL4DualStackServiceState("IPv6", "SingleStack", StatusSuccess),
},
expectL4ILBDualStackCount: map[L4DualStackServiceState]int{
L4DualStackServiceState{"IPv4,IPv6", "RequireDualStack", StatusSuccess}: 2,
L4DualStackServiceState{"IPv4", "SingleStack", StatusError}: 1,
L4DualStackServiceState{"IPv6", "SingleStack", StatusSuccess}: 2,
},
},
} {
Expand All @@ -525,8 +571,8 @@ func TestComputeL4ILBDualStackMetrics(t *testing.T) {
}
}

func newL4ILBDualStackServiceState(ipFamilies string, ipFamilyPolicy string, status L4ILBDualStackServiceStateStatus) L4ILBDualStackServiceState {
return L4ILBDualStackServiceState{
func newL4DualStackServiceState(ipFamilies string, ipFamilyPolicy string, status L4DualStackServiceStatus) L4DualStackServiceState {
return L4DualStackServiceState{
IPFamilies: ipFamilies,
IPFamilyPolicy: ipFamilyPolicy,
Status: status,
Expand Down
Loading

0 comments on commit 839191b

Please sign in to comment.