Specify --metrics-bind-addr for CAPO manager #725

jichenjc · 2021-01-22T04:02:27Z

What this PR does / why we need it:

add --metrics-bind-addr=127.0.0.1:8080 so that capo can report metrics info
this previously exist and removed somehow..

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

Release note:

k8s-ci-robot · 2021-01-22T04:02:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jichenjc

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:

~~OWNERS~~ [jichenjc]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

theopenlab-ci · 2021-01-22T04:56:55Z

Build failed.

cluster-api-provider-openstack-current-acceptance-test-v1.18 : FAILURE in 54m 05s

hidekazuna · 2021-01-22T05:33:16Z

Default is 127.0.0.1:10249. Do we have any reason --metrics-bind-addr should not to be default?

jichenjc · 2021-01-22T05:37:38Z

containerPort: 8080
name: metrics
protocol: TCP

is in same file ...previously it's also 8080
I am ok to change to 10249 but seems capi also default to 8080

jichenjc · 2021-01-22T05:38:08Z

recheck

hidekazuna · 2021-01-22T05:49:47Z

containerPort: 8080
name: metrics
protocol: TCP

is in same file ...previously it's also 8080
I am ok to change to 10249 but seems capi also default to 8080

I understand. I wondered after checking capa source which is not have --metrics-bind-addr. So we have to delete containerPort to be default.

jichenjc · 2021-01-22T06:04:26Z

containerPort: 8080
name: metrics
protocol: TCP

is in same file ...previously it's also 8080
I am ok to change to 10249 but seems capi also default to 8080

I understand. I wondered after checking capa source which is not have --metrics-bind-addr. So we have to delete containerPort to be default.

Sorry, I mean https://github.com/kubernetes-sigs/cluster-api-provider-openstack/blob/master/config/manager/manager_auth_proxy_patch.yaml#L24

hidekazuna · 2021-01-22T06:12:15Z

capi seems not to be 8080
https://github.com/kubernetes-sigs/cluster-api/blob/master/config/manager/manager.yaml

containerPort: 8080
name: metrics
protocol: TCP

is in same file ...previously it's also 8080
I am ok to change to 10249 but seems capi also default to 8080

I understand. I wondered after checking capa source which is not have --metrics-bind-addr. So we have to delete containerPort to be default.

Sorry, I mean https://github.com/kubernetes-sigs/cluster-api-provider-openstack/blob/master/config/manager/manager_auth_proxy_patch.yaml#L24

CAPI is also.
https://github.com/kubernetes-sigs/cluster-api/blob/master/config/manager/manager_auth_proxy_patch.yaml#L24

So,
Both CAPI and CAPO has --metrics-bind-addr in manager_auth_proxy_patch.yaml. That is fine.
Only CAPO has containerPort: 8080 in manager.yaml. This should to be fixed.

hidekazuna · 2021-01-22T06:14:05Z

Thanks for you, I found another issue and raised #728 😃

theopenlab-ci · 2021-01-22T06:33:53Z

Build failed.

cluster-api-provider-openstack-current-acceptance-test-v1.18 : FAILURE in 55m 22s

jichenjc · 2021-01-22T10:45:52Z

recheck

jichenjc · 2021-01-22T10:46:17Z

@hidekazuna So I think you are saying this good PR and only wait for CI? or any changes need to be made?

theopenlab-ci · 2021-01-22T11:26:58Z

Build failed.

cluster-api-provider-openstack-current-acceptance-test-v1.18 : FAILURE in 40m 40s

jichenjc · 2021-01-25T00:39:53Z

recheck

theopenlab-ci · 2021-01-25T01:23:18Z

Build failed.

cluster-api-provider-openstack-current-acceptance-test-v1.18 : FAILURE in 42m 58s

jichenjc · 2021-01-25T01:34:20Z

recheck

theopenlab-ci · 2021-01-25T02:16:56Z

Build failed.

cluster-api-provider-openstack-current-acceptance-test-v1.18 : FAILURE in 42m 11s

scruplelesswizard · 2021-01-27T19:21:47Z

recheck

scruplelesswizard · 2021-01-27T19:21:56Z

/lgtm

scruplelesswizard · 2021-01-27T19:24:14Z

Thanks for the patch @jichenjc

hidekazuna · 2021-01-28T04:18:37Z

@jichenjc --metrics-bind-addr=127.0.0.1:8080 is added by patches:

We do not need this PR. Actually we can get the same result using kustomize build under config directory.

jichenjc · 2021-01-28T04:28:05Z

@jichenjc --metrics-bind-addr=127.0.0.1:8080 is added by patches:

https://github.com/kubernetes-sigs/cluster-api-provider-openstack/blob/master/config/manager/manager_auth_proxy_patch.yaml#L24

https://github.com/kubernetes-sigs/cluster-api-provider-openstack/blob/master/config/webhook/manager_webhook_patch.yaml#L12

We do not need this PR. Actually we can get the same result using kustomize build under config directory.

are those for proxy and webhook? the PR is for the manager ..

hidekazuna · 2021-01-28T04:56:45Z

Patches are for manager.

- name: manager
        args:
        - "--metrics-bind-addr=127.0.0.1:8080"
        - "--leader-elect"

…ics_service" This reverts commit 8f51aa2, reversing changes made to 0163b5e.

🏃 Revert Merge pull request #725

…ics_service" This reverts commit 8f51aa2, reversing changes made to 0163b5e.

Add metrics serive ip and addr

33e40e0

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 22, 2021

k8s-ci-robot requested review from m1093782566 and scruplelesswizard January 22, 2021 04:02

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 22, 2021

This comment has been minimized.

Sign in to view

k8s-ci-robot assigned scruplelesswizard Jan 27, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 27, 2021

scruplelesswizard changed the title ~~Add metrics serive ip and addr~~ Specify --metrics-bind-addr for CAPO manager Jan 27, 2021

k8s-ci-robot merged commit 8f51aa2 into kubernetes-sigs:master Jan 27, 2021

hidekazuna added a commit to hidekazuna/cluster-api-provider-openstack that referenced this pull request Feb 3, 2021

Revert "Merge pull request kubernetes-sigs#725 from jichenjc/add_metr…

d3605d1

…ics_service" This reverts commit 8f51aa2, reversing changes made to 0163b5e.

k8s-ci-robot added a commit that referenced this pull request Feb 3, 2021

Merge pull request #733 from hidekazuna/revert_metrics-bind-addr

8a30dec

🏃 Revert Merge pull request #725

EmilienM mentioned this pull request Jan 31, 2023

Manual rebase on upstream openshift/cluster-api-provider-openstack#252

Closed

pierreprinetti pushed a commit to shiftstack/cluster-api-provider-openstack that referenced this pull request Apr 22, 2024

Revert "Merge pull request kubernetes-sigs#725 from jichenjc/add_metr…

a2a3c6d

…ics_service" This reverts commit 8f51aa2, reversing changes made to 0163b5e.

Specify --metrics-bind-addr for CAPO manager #725

Specify --metrics-bind-addr for CAPO manager #725

Uh oh!

Conversation

jichenjc commented Jan 22, 2021

Uh oh!

k8s-ci-robot commented Jan 22, 2021

Uh oh!

theopenlab-ci bot commented Jan 22, 2021

Uh oh!

hidekazuna commented Jan 22, 2021

Uh oh!

jichenjc commented Jan 22, 2021

Uh oh!

jichenjc commented Jan 22, 2021

Uh oh!

hidekazuna commented Jan 22, 2021

Uh oh!

jichenjc commented Jan 22, 2021

Uh oh!

hidekazuna commented Jan 22, 2021

Uh oh!

hidekazuna commented Jan 22, 2021

Uh oh!

theopenlab-ci bot commented Jan 22, 2021

Uh oh!

jichenjc commented Jan 22, 2021

Uh oh!

jichenjc commented Jan 22, 2021

Uh oh!

theopenlab-ci bot commented Jan 22, 2021

Uh oh!

This comment has been minimized.

jichenjc commented Jan 25, 2021

Uh oh!

theopenlab-ci bot commented Jan 25, 2021

Uh oh!

jichenjc commented Jan 25, 2021

Uh oh!

theopenlab-ci bot commented Jan 25, 2021

Uh oh!

scruplelesswizard commented Jan 27, 2021

Uh oh!

scruplelesswizard commented Jan 27, 2021

Uh oh!

scruplelesswizard commented Jan 27, 2021

Uh oh!

hidekazuna commented Jan 28, 2021

Uh oh!

jichenjc commented Jan 28, 2021

Uh oh!

hidekazuna commented Jan 28, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants