Prometheus should scrape the router by default #18254
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: smarterclayton The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
openshift-ci-robot
added
the
approved
openshift-ci-robot
requested review from
gabemontero,
juanvallejo,
0xmichalis and
mfojtik
openshift-ci-robot
added
the
size/L
smarterclayton
force-pushed
the
router_default
branch
from
83c0691 to
b6112f2
Compare
|
/test gcp |
|
/test all |
| @@ -730,13 +753,6 @@ func RunCmdRouter(f *clientcmd.Factory, cmd *cobra.Command, out, errout io.Write | |||
| }, | |||
| } | |||
|
|
|||
| if cfg.StatsPort > 0 && cfg.ExposeMetrics { | |||
| pc := generateMetricsExporterContainer(cfg, env) | |||
There was a problem hiding this comment.
Think the generateMetricsExporterContainer function isn't used anywhere else, if you are removing this code.
When router stats are on, use the service serving cert always
Add a new scrape target for the router. The router requires a token with permission to see routers/metrics in route.openshift.io, so create a new service account specifically for scraping and add the appropriate roles. Also create an out of the box prometheus-reader service account which has view access on the current project.
smarterclayton
force-pushed
the
router_default
branch
from
b6112f2 to
0786dad
Compare
|
my 0.02$ @smarterclayton: will be nice to be / keep consistent with all the other templates where this code is at the bottom of the template. |
|
I think the other templates are wrong ;)
…On Fri, Feb 23, 2018 at 4:12 AM, Dani Comnea ***@***.***> wrote:
my 0.02$ @smarterclayton <https://github.com/smarterclayton>: will be
nice to be / keep consistent with all the other templates where this
<https://github.com/smarterclayton/origin/blob/0786dad3509a8ac2a667b21d89a37343b9d3cec3/examples/prometheus/prometheus.yaml#L11-L30>
code is at the bottom of the template.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#18254 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABG_pw2-A9JHLSQ1p356ELFgDaA6lAp0ks5tXoEAgaJpZM4RqdFm>
.
|
|
@smarterclayton at least the other ones form the majority ;). Seriously don't mind which one it is as long as is all agreed so we can go and implement/ have consistency across all code base. |
|
/retest |
|
/test all [submit-queue is verifying that this PR is safe to merge] |
|
Automatic merge from submit-queue (batch tested with PRs 17420, 18254). |
Builds on top of #18245 and will scrape the installed router by default. We ensure that the router by default will be using a serving cert to serve metrics, then add new roles, bindings, and a prometheus-scraper service account that has permission to scrape it.
For 3.10