Skip to content
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

catalog: create SSL cert for controller, add prometheus scrape #19286

Merged
merged 1 commit into from
Apr 13, 2018
Merged

catalog: create SSL cert for controller, add prometheus scrape #19286

merged 1 commit into from
Apr 13, 2018

Conversation

jboyd01
Copy link
Contributor

@jboyd01 jboyd01 commented Apr 9, 2018

  • Change secure port to 6443 to match how we install with ansible
  • Create controller manager service so we can utilize the service's SSL cert
  • Prometheus scrape config over for Catalog Controller over SSL

mirrors the configuration for ansible: openshift/openshift-ansible#7681

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 9, 2018
@jboyd01
Copy link
Contributor Author

jboyd01 commented Apr 9, 2018

/retest

2 similar comments
@jboyd01
Copy link
Contributor Author

jboyd01 commented Apr 10, 2018

/retest

@jboyd01
Copy link
Contributor Author

jboyd01 commented Apr 10, 2018

/retest

@pmorie
Copy link
Contributor

pmorie commented Apr 10, 2018

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 10, 2018
@pmorie
Copy link
Contributor

pmorie commented Apr 10, 2018

@jboyd01 we need a review from one of the prometheus owners

@jboyd01
Copy link
Contributor Author

jboyd01 commented Apr 10, 2018

@simonpasquier could you review and approve the prometheus config change for Service Catalog?

simonpasquier
simonpasquier approved these changes Apr 10, 2018
View reviewed changes
Copy link
Contributor

@simonpasquier simonpasquier left a comment

Choose a reason for hiding this comment

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

LGTM regarding the Prometheus config.

@jboyd01
Copy link
Contributor Author

jboyd01 commented Apr 10, 2018

/assign @bparees

bparees
bparees reviewed Apr 10, 2018
View reviewed changes
examples/prometheus/prometheus.yaml Outdated
relabel_configs:
- source_labels: [__meta_kubernetes_service_name]
action: keep
regex: controller-manager
Copy link
Contributor

Choose a reason for hiding this comment

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

i think @smarterclayton had some concerns about more stuff being added in here, so need him to approve.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you don't expose tenant metrics you just want to set the annotations on your service and be in the generic block up above.

@openshift-ci-robot openshift-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 11, 2018
@jboyd01
Copy link
Contributor Author

jboyd01 commented Apr 11, 2018

reworked per #19286 (comment)
Instead of a scrape config for catalog, we automatically are scraped by adding the prometheus annotations on the catalog controller service. Accomplishes the same thing more succinctly without the additional scrape config.

@bparees
Copy link
Contributor

bparees commented Apr 11, 2018

/approve

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 11, 2018
@jboyd01
Copy link
Contributor Author

jboyd01 commented Apr 11, 2018

/retest

1 similar comment
@jboyd01
Copy link
Contributor Author

jboyd01 commented Apr 11, 2018

/retest

@jboyd01 jboyd01 mentioned this pull request Apr 11, 2018
Merged
@jboyd01
Copy link
Contributor Author

jboyd01 commented Apr 11, 2018

Apr 11 16:27:41.092: INFO: WaitForABuild returning with error: The build "ruby-sample-build-td-1" status is "Failed"

ruby-sample-build-td-2 also failed

I can't locate any more details on this, nothing I can find in S3 artifacts. This is really painful.

/retest

@pmorie
Copy link
Contributor

pmorie commented Apr 11, 2018

/retest
/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 11, 2018
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees, jboyd01, pmorie

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jboyd01 jboyd01 mentioned this pull request Apr 12, 2018
Closed
@jboyd01
Copy link
Contributor Author

jboyd01 commented Apr 12, 2018

/retest

1 similar comment
@pmorie
Copy link
Contributor

pmorie commented Apr 12, 2018

/retest

@pmorie
Copy link
Contributor

pmorie commented Apr 12, 2018

Flake #18048

@openshift-merge-robot openshift-merge-robot merged commit 7c6b296 into master Apr 13, 2018
@jboyd01 jboyd01 deleted the sslrefinements branch April 13, 2018 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bparees bparees bparees left review comments

@smarterclayton smarterclayton smarterclayton left review comments

@simonpasquier simonpasquier simonpasquier approved these changes

@csrwng csrwng Awaiting requested review from csrwng

@dcbw dcbw Awaiting requested review from dcbw

Assignees

@pmorie pmorie

@bparees bparees

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
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@jboyd01 @pmorie @bparees @openshift-ci-robot @smarterclayton @simonpasquier @openshift-merge-robot