-
Notifications
You must be signed in to change notification settings - Fork 1.8k
OSDOCS-34: Updating with the latest examples #14309
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,22 +11,36 @@ newly created Service. | |
|
||
.Prerequisites | ||
|
||
- Go-based Operator generated using the Operator SDK | ||
- Kubernetes-based cluster with the Prometheus Operator deployed | ||
* Go-based Operator generated using the Operator SDK | ||
* Kubernetes-based cluster with the Prometheus Operator deployed | ||
|
||
.Procedure | ||
|
||
. Add the `metrics.CreateServiceMonitor()` helper function to your Operator code, | ||
creating only one ServiceMonitor per application and namespace: | ||
* Add the `metrics.CreateServiceMonitor()` helper function to your Operator code: | ||
+ | ||
[source,go] | ||
---- | ||
import "github.com/operator-framework/operator-sdk/pkg/metrics" | ||
import( | ||
"k8s.io/api/core/v1" | ||
"github.com/operator-framework/operator-sdk/pkg/metrics" | ||
"sigs.k8s.io/controller-runtime/pkg/client/config" | ||
) | ||
func main() { | ||
|
||
serviceMonitors, err := metrics.CreateServiceMonitors(config *rest.Config, ns string, services []*v1.Service) <1> | ||
... | ||
// Populate below with the Service(s) for which you want to create ServiceMonitors. | ||
services := []*v1.Service{} | ||
// Create one ServiceMonitor per application per namespace. | ||
// Change the below value to name of the Namespace you want the ServiceMonitor to be created in. | ||
ns := "default" | ||
// restConfig is used for talking to the Kubernetes apiserver | ||
restConfig := config.GetConfig() | ||
|
||
// Pass the Service(s) to the helper function, which in turn returns the array of ServiceMonitor objects. | ||
serviceMonitors, err := metrics.CreateServiceMonitors(restConfig, ns, services) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lilic Do we need to add an instruction for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, that makes sense! This is usually done in the main.go file, so the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lilic So should I add the following lines below the "ns := "default" line?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lilic, @jianzhangbjz I added the above line and updated the PR if you want to take a look. Let me know whether that's right or not, thanks! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @bergerhoffer Thanks! But, for me, I think we just need to add a comment for the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The only thing I would change is explicitly point out it's the Kubernetes apiserver config: // restConfig is used for talking to the Kubernetes apiserver
restConfig := config.GetConfig() There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jianzhangbjz @lilic Thank you both for the reviews, can you take a look at the updates and see if this looks good now? |
||
if err != nil { | ||
<2> | ||
// Handle errors here. | ||
} | ||
... | ||
} | ||
---- | ||
<1> Pass the Service(s) to the helper function, which in turn returns the ServiceMonitor object. | ||
<2> Handle errors here. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,6 @@ generate a ServiceMonitor Custom Resource (CR) based on it. | |
|
||
.Additional resources | ||
|
||
- See the | ||
* See the | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are no longer linking to GitHub-hosted content per our user guide There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sferich888 I've heard that the request to not link to GitHub content came from you. Just wanted to run this one by you (since you made an exception in the CLI content) to make sure this is indeed a blanket statement and if this instance should be removed. Just want to make sure I understand. Thanks! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why would we not link to downstream docs for that topic? https://github.com/openshift/openshift-docs/blob/enterprise-4.0/monitoring/monitoring.adoc looks like it's missing content if you don't have something to link too There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @sferich888, I'm not sure I understand what you're suggesting. It doesn't look like there is more information about ServiceMonitor in that file (which is here: https://docs.openshift.com/container-platform/4.0/monitoring/monitoring.html). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After discussing further with @sferich888, I opened https://jira.coreos.com/browse/OSDOCS-394 to cover adding the ServiceMonitor details into the docs, and removing this link altogether. Leaving this link as is for now, and will make that update in a separate PR. |
||
link:https://github.com/coreos/prometheus-operator/blob/7a25bf6b6bb2347dacb235659b73bc210117acc7/Documentation/design.md#servicemonitor[Prometheus Operator documentation] | ||
for more about the ServiceMonitor CRD. | ||
for more information about the ServiceMonitor CRD. |
Uh oh!
There was an error while loading. Please reload this page.