Skip to content

Conversation

@adellape
Copy link
Contributor

@adellape adellape commented Feb 19, 2019

@adellape adellape added this to the Future Release milestone Feb 19, 2019
@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 19, 2019
@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 19, 2019
@vikram-redhat
Copy link
Contributor

@rh-max can you review to make sure this is still correct?

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 20, 2019
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 20, 2019
@openshift-docs-preview-bot

The preview will be availble shortly at:

@adellape adellape changed the title Add monitoring to OSDK [WIP] Add monitoring to OSDK Feb 20, 2019
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lilic What should this be added to?

Copy link

Choose a reason for hiding this comment

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

Wherever in the users operator code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lilic Should "namespace" be a replaceable? services?

Copy link

Choose a reason for hiding this comment

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

services are meant in this case an array of kubernetesService objects the user creates for metrics exposing.

Yes "namespace" is a silly example I guess, should include the signature of the function as well. It's the name of the Namespace in which the ServiceMonitor will be created.

CreateServiceMonitors(config *rest.Config, ns string, services []*v1.Service)

@lilic
Copy link

lilic commented Feb 21, 2019

FYI I updated the original docs PR a bit.

@ahardin-rh ahardin-rh added the peer-review-done Signifies that the peer review team has reviewed this PR label Feb 22, 2019
@ahardin-rh
Copy link
Contributor

@adellape LGTM! 💯

@adellape
Copy link
Contributor Author

@lilic @ahardin-rh Thanks for the reviews! Updated with my latest understanding. Going to merge to get this in for QE review, and we can continue iterating from there.

Also cc @joelanford since you were reviewing the upstream operator-framework/operator-sdk#719 as well.

@adellape adellape merged commit 2d5a49b into openshift:enterprise-4.0 Feb 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.1 peer-review-done Signifies that the peer review team has reviewed this PR size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants