Calico update and typha #7528
Calico update and typha #7528
Conversation
k8s-ci-robot
added
do-not-merge/hold
There was a problem hiding this comment.
Thanks for doing the hard work of picking this up and progressing it! The original got a bit forgotten about.
Just a couple of comments from me. W.r.t. the Typha enabling on a running cluster not automatically triggering a restart of the pod my inclination would be to leave the restarts to the user. I don't think we previously saw any issues with restarting pods where the only change was Typha config on our clusters, however this was ~Calico 3.2.
I'm away having some solid type 2 fun on a bike this week so I'm unfortunately not able to test my theories out until next weekend.
upup/models/cloudup/resources/addons/networking.projectcalico.org/k8s-1.12.yaml.template
Show resolved
Hide resolved
upup/models/cloudup/resources/addons/networking.projectcalico.org/k8s-1.12.yaml.template
Outdated
Show resolved
Hide resolved
mikesplain
force-pushed
the
calico_update_and_typha
branch
from
3c19662
to
8407565
Compare
Adds the ability to configure the number of Typha replicas when using Calico CNI in 1.12+ to limit the impact of Calico on the APIServer and increase the scalability of the cluster. Also adds the ability to configure Typha's Prometheus config. Add Passing TyphaReplicas Validation Test
mikesplain
force-pushed
the
calico_update_and_typha
branch
from
8407565
to
b76d9b1
Compare
|
/retest |
|
Testing complete, works as expected at the moment. |
|
/hold cancel |
k8s-ci-robot
removed
the
do-not-merge/hold
|
Once this is lgtmed, I'll cherry pick to the 1.15 branch and work on a backport to 1.14. |
|
Thanks @mikesplain - looks great /approve Not sure if we should cherry-pick this or #7517 to 1.14 ... I'd like to try to get both 1.14 and 1.15 out soonish, and I'm just not sure how broken we are if we just taken #7517 vs this (more complete) fix? |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: justinsb, mikesplain 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 |
…28-origin-release-1.15 Automated cherry pick of #7528: Add ability to configure number of Typha Replicas
| # Set to enable the experimental Prometheus metrics server | ||
| - name: FELIX_PROMETHEUSMETRICSENABLED | ||
| value: "{{- or .Networking.Calico.PrometheusMetricsEnabled "false" }}" | ||
| # TCP port that the Prometheus metrics server should bind to | ||
| - name: FELIX_PROMETHEUSMETRICSPORT | ||
| value: "{{- or .Networking.Calico.PrometheusMetricsPort "9091" }}" | ||
| # Enable Prometheus Go runtime metrics collection | ||
| - name: FELIX_PROMETHEUSGOMETRICSENABLED | ||
| value: "{{- or .Networking.Calico.PrometheusGoMetricsEnabled "true" }}" | ||
| # Enable Prometheus process metrics collection | ||
| - name: FELIX_PROMETHEUSPROCESSMETRICSENABLED | ||
| value: "{{- or .Networking.Calico.PrometheusProcessMetricsEnabled "true" }}" |
There was a problem hiding this comment.
Was removing these intentional? They still seem to be valid according to the calico docs.
After #7517 I noticed much of our calico is out of sync with upstream so I thought it may be worth reformatting to match upstream adding back our customizations. I know @gjtempleton has been working on adding Typha however he was facing rebasing issues, so I pulled them in here while upgrading us. I'm doing more testing on this but I think we should target 1.15 for this and possibly 1.14 depending on testing time.
Key points on this pr:
Todo:
/hold
Fixes #7181
Fixes #7158