Add L4 ILB usage metrics #1031
Add L4 ILB usage metrics #1031
Conversation
k8s-ci-robot
added
the
needs-ok-to-test
|
Hi @skmatti. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
cncf-cla: yes
| for key, state := range im.l4ILBServiceMap { | ||
| klog.V(6).Infof("ILB Service %s has EnabledGlobalAccess: %t, EnabledCustomSubnet: %t", key, state.EnabledGlobalAccess, state.EnabledCustomSubnet) | ||
| counts[l4ILBService] += 1 | ||
| if state.EnabledGlobalAccess { |
There was a problem hiding this comment.
A service can be updated to disable/enable global acess anytime. Will this handle the case where a service is created with global access on, then updated to disable it?
There was a problem hiding this comment.
Yes, the state of a service gets updated on each of it's sync. Metrics are computed based on the current state of all services. Since the metrics are computed at 10 mins interval, the updates may be delayed by 10 minutes at max.
| @@ -143,6 +146,10 @@ func (l *L4) EnsureInternalLoadBalancerDeleted(svc *corev1.Service) error { | |||
| klog.Errorf("Failed to delete healthcheck for internal loadbalancer service %s, err %v", l.NamespacedName.String(), err) | |||
| return err | |||
| } | |||
| if retErr == nil { | |||
There was a problem hiding this comment.
We can add this before line 139, since failing delete of a shared healthcheck is ok. Or we can do it in the caller of EnsureInternalLoadBalancerDeleted
There was a problem hiding this comment.
Modified this to take those cases into account. Keeping it in EnsureInternalLoadBalancerDeleted makes metrics collector symmetric, so prefer keeping it here.
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
pkg/metrics/metrics_test.go
Outdated
| }, | ||
| }, | ||
| { | ||
| "vm primary ip neg in local mode", |
There was a problem hiding this comment.
this is "vm primary ip neg in cluster mode" ?
k8s-ci-robot
added
the
lgtm
k8s-ci-robot
removed
the
lgtm
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: freehan, prameshj, skmatti The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
/assign @freehan @prameshj