Healthcheck & Dashboard update #522

roffe · 2018-08-29T10:56:21Z

added configurable static timeout for the controllers, defaults to 30 sec
added metric controller_errors with label "controller"
changed metric controller_bgp_advertisements_received to controller_bgp_advertisements with label "type" sent or advertised

andrewsykim · 2018-08-29T13:47:00Z

docs/user-guide.md

@@ -46,7 +46,9 @@ Usage of kube-router:
  -h, --help                             Print usage information.
      --hostname-override string         Overrides the NodeName of the node. Set this if kube-router is unable to determine your NodeName automatically.
      --iptables-sync-period duration    The delay between iptables rule synchronizations (e.g. '5s', '1m'). Must be greater than 0. (default 5m0s)
+      --iptables-sync-timeout duration   The timeout for iptables rule synchronizations (e.g. '5s', '1m'). Must be greater than 0. (default 30s)


we should specifically mention that it will cause health endpoint to return 500

andrewsykim · 2018-08-29T13:47:14Z

pkg/cmd/kube-router.go

@@ -17,11 +17,12 @@ import (
 	"github.com/cloudnativelabs/kube-router/pkg/options"
 	"github.com/golang/glog"

+	"time"


This should go to the very top with the rest of stdlib

andrewsykim · 2018-08-29T13:48:20Z

pkg/controllers/netpol/network_policy_controller.go

@@ -19,14 +19,15 @@ import (
 	"github.com/golang/glog"
 	"github.com/prometheus/client_golang/prometheus"

+	"regexp"


same here, group with other stdlib

andrewsykim · 2018-08-29T15:02:27Z

pkg/healthcheck/health_controller.go

@@ -170,7 +153,7 @@ func (hc *HealthController) RunServer(stopCh <-chan struct{}, wg *sync.WaitGroup

 //RunCheck starts the HealthController's check
 func (hc *HealthController) RunCheck(healthChan <-chan *ControllerHeartbeat, stopCh <-chan struct{}, wg *sync.WaitGroup) error {
-	t := time.NewTicker(5000 * time.Millisecond)
+	t := time.NewTicker(1000 * time.Millisecond)


With the new timeout, does it still make sense to lower this value?

andrewsykim · 2018-08-29T15:03:05Z

pkg/controllers/netpol/network_policy_controller.go

-		}
+	err := npc.Sync()
+	if err != nil {
+		metrics.ControllerErrors.WithLabelValues("NPC").Inc()


Can we make "NPC" a constant?

andrewsykim · 2018-08-29T16:33:16Z

pkg/controllers/netpol/network_policy_controller.go

@@ -928,7 +941,7 @@ func cleanupStaleRules(activePolicyChains, activePodFwChains, activePolicyIPSets
 		for podFwChain := range activePodFwChains {
 			podFwChainRules, err := iptablesCmdHandler.List("filter", podFwChain)
 			if err != nil {
-
+				glog.Errorf("Failed to list iptable filters")


glog.Errorf("Failed to list iptable filters: %s", err)

should this have metrics.ControllerErrors.WithLabelValues(networkPolicyControllerLabel).Inc()?

pkg/controllers/proxy/network_services_controller.go

pkg/metrics/metrics_controller.go

andrewsykim · 2018-08-29T16:43:07Z

pkg/metrics/metrics_controller.go

-		Help:      "Time it took to sync internal bgp peers",
-	}, []string{})
+		Name:      "controller_bgp_advertisements",
+		Help:      "Number of BGP advertisements received and sent",


small nit: can we make this received and advertised to be consistent with code?

andrewsykim · 2018-08-29T16:43:55Z

pkg/controllers/routing/network_routes_controller.go

@@ -354,7 +377,7 @@ func (nrc *NetworkRoutingController) advertisePodRoute() error {
 		subnet), false, attrs, time.Now(), false)}); err != nil {
 		return fmt.Errorf(err.Error())
 	}
-
+	metrics.ControllerBGPadvertisements.WithLabelValues("sent").Inc()


Does this need a if nrc.MetricsEnabled check?

pkg/controllers/routing/ecmp_vip.go

murali-reddy · 2018-11-02T05:28:24Z

@roffe could you please rebase to latest master?

jay-rob · 2018-12-08T14:48:18Z

We have encountered some cases where our nodes have not properly reloaded their iptable rules and this went silently undetected for some time. We looked through the logs and found errors such as:

E1206 22:10:16.636454 1 network_routes_controller.go:335] Failed to inject routes due to: no such process

I am curious if this PR could help us detect this and self heal as it would cause that effected router pod to fail a health check and get restarted. If it does, I am happy to help get this PR merged in.

roffe · 2018-12-08T19:20:04Z

@jrthrawny no this pr would not adress that, it was mostly metrics and preventing a large rate of NSC.SYNC() to occur that was the motivation behind it but to much has changed in the codebase to salvage it now

roffe · 2018-12-08T19:21:54Z

also parts of this pr is adressed by #595 the debounce parts and such i'm planing to recreate as soon as other changes i want in has been merged

roffe requested review from murali-reddy and andrewsykim and removed request for murali-reddy August 29, 2018 10:56

andrewsykim reviewed Aug 29, 2018

View reviewed changes

pkg/controllers/proxy/network_services_controller.go Show resolved Hide resolved

andrewsykim reviewed Aug 29, 2018

View reviewed changes

pkg/metrics/metrics_controller.go Show resolved Hide resolved

andrewsykim reviewed Aug 29, 2018

View reviewed changes

pkg/controllers/routing/ecmp_vip.go Show resolved Hide resolved

andrewsykim reviewed Aug 29, 2018

View reviewed changes

pkg/controllers/routing/ecmp_vip.go Show resolved Hide resolved

roffe mentioned this pull request Sep 4, 2018

Use ipset to manage multiple CIDRs in a network policy rule #529

Merged

roffe added 12 commits November 14, 2018 09:03

update healthchecks and add configurable timeouts for healthchecks

8d5800f

move healthchan to controller config

7ce8f91

more metrics

1cd3d8a

mc health fix

28c5349

fix healthchan

03849ac

updated dashboard

74c5733

more npc error metrics

1a55d36

updated user-guide.md

6099f4c

cleanup

e1c06fe

more cleanup

9cb72bb

remove excessive if metricsEnabled

95ad4d4

advertised instead of sent

d4275db

roffe added 2 commits November 14, 2018 09:07

more metrics

1528f6b

typo

d87518a

roffe closed this Dec 8, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Healthcheck & Dashboard update #522

Healthcheck & Dashboard update #522

roffe commented Aug 29, 2018 •

edited

Loading

andrewsykim Aug 29, 2018

andrewsykim Aug 29, 2018

andrewsykim Aug 29, 2018

andrewsykim Aug 29, 2018

andrewsykim Aug 29, 2018

andrewsykim Aug 29, 2018

andrewsykim Aug 29, 2018

andrewsykim Aug 29, 2018

roffe Aug 29, 2018

andrewsykim Aug 29, 2018

murali-reddy commented Nov 2, 2018

jay-rob commented Dec 8, 2018

roffe commented Dec 8, 2018

roffe commented Dec 8, 2018 •

edited

Loading

Healthcheck & Dashboard update #522

Healthcheck & Dashboard update #522

Conversation

roffe commented Aug 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

murali-reddy commented Nov 2, 2018

jay-rob commented Dec 8, 2018

roffe commented Dec 8, 2018

roffe commented Dec 8, 2018 • edited Loading

roffe commented Aug 29, 2018 •

edited

Loading

roffe commented Dec 8, 2018 •

edited

Loading