Skip to content

Enable metering proxy for local development #2622

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
openshift-merge-robot merged 1 commit into from
Sep 17, 2019
Merged

Enable metering proxy for local development #2622

openshift-merge-robot merged 1 commit into from
Sep 17, 2019

Conversation

@spadgett
Copy link
Member

@spadgett spadgett commented Sep 6, 2019

Like #2527, but for metering.

/cc @yaacov @dtaylor113

@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 6, 2019
dtaylor113
dtaylor113 approved these changes Sep 6, 2019
View reviewed changes
Copy link
Contributor

@dtaylor113 dtaylor113 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@spadgett
Copy link
Member Author

spadgett commented Sep 6, 2019

/retest

2 similar comments
@spadgett
Copy link
Member Author

spadgett commented Sep 6, 2019

/retest

@spadgett
Copy link
Member Author

spadgett commented Sep 6, 2019

/retest

yaacov
yaacov reviewed Sep 7, 2019
View reviewed changes
cmd/bridge/main.go Outdated
Copy link
Member

@yaacov yaacov Sep 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in #2527 we did && *fK8sMode == "off-cluster" for each off cluster arg:

if *fK8sModeOffClusterMetering != "" && *fK8sMode == "off-cluster" {

I think it's nicer to move from the code from L149 to L166 into the case "off-cluster": in L316 and create offClusterAlertManagerURL, offClusterPrometheusURL and offClusterMeteringURL inside the case, WDYT ?

@yaacov
Copy link
Member

yaacov commented Sep 7, 2019

other then the nitpick above /lgtm

@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Sep 9, 2019
@spadgett
Copy link
Member Author

spadgett commented Sep 9, 2019

@yaacov thanks, updated

@spadgett spadgett changed the base branch from master to master-4.3 September 17, 2019 13:48
@spadgett
Copy link
Member Author

spadgett commented Sep 17, 2019

@yaacov ptal

@yaacov
Copy link
Member

yaacov commented Sep 17, 2019

/lgtm 👍

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Sep 17, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dtaylor113, spadgett, yaacov

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@spadgett spadgett added the lgtm Indicates that a PR is ready to be merged. label Sep 17, 2019
@openshift-bot
Copy link
Contributor

openshift-bot commented Sep 17, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

2 similar comments
@openshift-bot
Copy link
Contributor

openshift-bot commented Sep 17, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

openshift-bot commented Sep 17, 2019

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 1e0ea20 into openshift:master-4.3 Sep 17, 2019
@spadgett spadgett deleted the dev-metering-proxy branch September 17, 2019 21:23
@spadgett spadgett added this to the v4.3 milestone Sep 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@yaacov yaacov yaacov approved these changes

@dtaylor113 dtaylor113 dtaylor113 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

v4.3

Development

Successfully merging this pull request may close these issues.

6 participants

@spadgett @yaacov @openshift-ci-robot @openshift-bot @dtaylor113 @openshift-merge-robot