Send alert when MCO can't safely apply updated Kubelet CA on nodes in paused pool #2802
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,10 +27,42 @@ spec: | |
| cpu: 20m | ||
| memory: 50Mi | ||
| terminationMessagePolicy: FallbackToLogsOnError | ||
| - name: oauth-proxy | ||
|
||
| image: {{.Images.OauthProxy}} | ||
| ports: | ||
| - containerPort: 9001 | ||
| name: metrics | ||
| protocol: TCP | ||
| args: | ||
| - --https-address=:9001 | ||
| - --provider=openshift | ||
| - --openshift-service-account=machine-config-controller | ||
| - --upstream=http://127.0.0.1:8797 | ||
| - --tls-cert=/etc/tls/private/tls.crt | ||
| - --tls-key=/etc/tls/private/tls.key | ||
| - --cookie-secret-file=/etc/tls/cookie-secret/cookie-secret | ||
| - '--openshift-sar={"resource": "namespaces", "verb": "get"}' | ||
| - '--openshift-delegate-urls={"/": {"resource": "namespaces", "verb": "get"}}' | ||
| resources: | ||
| requests: | ||
| cpu: 20m | ||
| memory: 50Mi | ||
| volumeMounts: | ||
| - mountPath: /etc/tls/private | ||
| name: proxy-tls | ||
| - mountPath: /etc/tls/cookie-secret | ||
| name: cookie-secret | ||
| serviceAccountName: machine-config-controller | ||
| nodeSelector: | ||
| node-role.kubernetes.io/master: "" | ||
| priorityClassName: "system-cluster-critical" | ||
| volumes: | ||
| - name: proxy-tls | ||
| secret: | ||
| secretName: mcc-proxy-tls | ||
| - name: cookie-secret | ||
| secret: | ||
| secretName: cookie-secret | ||
| restartPolicy: Always | ||
| tolerations: | ||
| - key: node-role.kubernetes.io/master | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| package common | ||
|
|
||
| import ( | ||
| "context" | ||
| "net/http" | ||
|
|
||
| "github.com/golang/glog" | ||
| "github.com/prometheus/client_golang/prometheus" | ||
| "github.com/prometheus/client_golang/prometheus/promhttp" | ||
| ) | ||
|
|
||
| const ( | ||
| // DefaultBindAddress is the port for the metrics listener | ||
| DefaultBindAddress = ":8797" | ||
|
||
| ) | ||
|
|
||
| var ( | ||
| // MachineConfigControllerPausedPoolKubeletCA logs when a certificate rotation is being held up by pause | ||
| MachineConfigControllerPausedPoolKubeletCA = prometheus.NewGaugeVec( | ||
| prometheus.GaugeOpts{ | ||
| Name: "machine_config_controller_paused_pool_kubelet_ca", | ||
| Help: "Set to the unix timestamp in utc of the current certificate expiry date if a certificate rotation is pending in specified paused pool", | ||
| }, []string{"pool"}) | ||
|
|
||
| metricsList = []prometheus.Collector{ | ||
| MachineConfigControllerPausedPoolKubeletCA, | ||
| } | ||
| ) | ||
|
|
||
| func RegisterMCCMetrics() error { | ||
| for _, metric := range metricsList { | ||
| err := prometheus.Register(metric) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // StartMetricsListener is metrics listener via http on localhost | ||
| func StartMetricsListener(addr string, stopCh <-chan struct{}) { | ||
|
||
| if addr == "" { | ||
| addr = DefaultBindAddress | ||
| } | ||
|
|
||
| glog.Info("Registering Prometheus metrics") | ||
| if err := RegisterMCCMetrics(); err != nil { | ||
| glog.Errorf("unable to register metrics: %v", err) | ||
|
||
| // No sense in continuing starting the listener if this fails | ||
| return | ||
| } | ||
|
|
||
| glog.Infof("Starting metrics listener on %s", addr) | ||
| mux := http.NewServeMux() | ||
| mux.Handle("/metrics", promhttp.Handler()) | ||
| s := http.Server{Addr: addr, Handler: mux} | ||
|
|
||
| go func() { | ||
| if err := s.ListenAndServe(); err != nil && err != http.ErrServerClosed { | ||
| glog.Errorf("metrics listener exited with error: %v", err) | ||
| } | ||
| }() | ||
| <-stopCh | ||
| if err := s.Shutdown(context.Background()); err != http.ErrServerClosed { | ||
| glog.Errorf("error stopping metrics listener: %v", err) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Firing
warningalerts immediately seems like it's wound a bit too tightly. We can go a while before the lack of certificate rotation becomes a problem (significant fractions of a year?). So setting a large-ishforhere seems useful to avoid desensitizing folks by alerting every time they briefly pause. Of course, you could have someone leaving pools paused for the bulk of that time, and then very briefly unpause, have Prom happen to scrape, and then pause again, before the pool had time to rotate out many certs. Which is one reason "are we paused now, and have we been paused for a bit?" only a rough proxy for "are we at risk of certs expiring?". And that makes picking an appropriatefordifficult. But if we stick with the paused-pool angle on this, I'd expect at leastfor: 60mon this alert, based on our consistency guidelines.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I think if alerting config allows to change severity then it should change to
criticalafter certain number of warning alert has been fired. This is to emphasize that unpausing pool is necessary to keep cluster healthy.