Skip to content

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

Merged
merged 1 commit into from
Apr 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@
// * applications/operator_sdk/osdk-monitoring-prometheus.adoc

[id="osdk-monitoring-prometheus-metrics-helper-modifying-port-{context}"]
= Modifying metrics port
= Modifying the metrics port

Operator authors can modify the port that metrics are exposed on.

.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

. In the generated Operator's `cmd/manager/main.go` file, change the `var
metricsPort int32 = 8383` variable.
* In the generated Operator's `cmd/manager/main.go` file, change the value of `metricsPort` in the line `var
metricsPort int32 = 8383`.
52 changes: 28 additions & 24 deletions modules/osdk-monitoring-prometheus-metrics-helper.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,15 @@
[id="osdk-monitoring-prometheus-metrics-helper-{context}"]
= Metrics helper

In Go-based Operators generated using the Operator SDK, the following helper
In Go-based Operators generated using the Operator SDK, the following
function exposes general metrics about the running program:

[source,go]
----
func ExposeMetricsPort(ctx context.Context, port int32) (*v1.Service, error)
----

These metrics are inherited from the `controller-runtime` library API. The
metrics are by default served on `:8383/metrics`.
These metrics are inherited from the `controller-runtime` library API. By default, the metrics are served on `0.0.0.0:8383/metrics`.

A Service object is created with the metrics port exposed, which can be then
accessed by Prometheus. The Service object is garbage collected when the leader
Expand All @@ -25,29 +24,34 @@ Operators generated using the Operator SDK:

[source,go]
----
import(
"github.com/operator-framework/operator-sdk/pkg/metrics"
"sigs.k8s.io/controller-runtime/pkg/manager"
)

// Change the below variables to serve metrics on different host or port.
var metricsHost = "0.0.0.0" <1>
var metricsPort int32 = 8383 <2>

import(
"github.com/operator-framework/operator-sdk/pkg/metrics"
"sigs.k8s.io/controller-runtime/pkg/manager"
)

var (
// Change the below variables to serve metrics on a different host or port.
metricsHost = "0.0.0.0" <1>
metricsPort int32 = 8383 <2>
)
...
func main() {
...
// Pass metrics address to controller-runtime manager
mgr, err := manager.New(cfg, manager.Options{
Namespace: namespace,
MetricsBindAddress: fmt.Sprintf("%s:%d", metricsHost, metricsPort),
})
mgr, err := manager.New(cfg, manager.Options{
Namespace: namespace,
MetricsBindAddress: fmt.Sprintf("%s:%d", metricsHost, metricsPort),
})

...

// Create Service object to expose the metrics port.
_, err = metrics.ExposeMetricsPort(ctx, metricsPort)
if err != nil {
// Create Service object to expose the metrics port.
_, err = metrics.ExposeMetricsPort(ctx, metricsPort)
if err != nil {
// handle error
log.Info(err.Error())
}
log.Info(err.Error())
}
...
}
----
<1> Host the metrics are exposed on.
<2> Port the metrics are exposed on.
<1> The host that the metrics are exposed on.
<2> The port that the metrics are exposed on.
32 changes: 23 additions & 9 deletions modules/osdk-monitoring-prometheus-servicemonitor-creating.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Choose a reason for hiding this comment

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

@lilic Do we need to add an instruction for the restConfig argument?

Copy link

Choose a reason for hiding this comment

The 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 restConfig refers to https://github.com/operator-framework/operator-sdk-samples/blob/master/memcached-operator/cmd/manager/main.go#L69

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 So should I add the following lines below the "ns := "default" line?

// Get a config to talk to the apiserver
restConfig := config.GetConfig()

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, @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!

Choose a reason for hiding this comment

The 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 restConfig argument. @lilic What do you suggest?

Copy link

Choose a reason for hiding this comment

The 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()

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.
4 changes: 2 additions & 2 deletions modules/osdk-monitoring-prometheus-servicemonitor.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ generate a ServiceMonitor Custom Resource (CR) based on it.

.Additional resources

- See the
* See the
Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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!

Copy link
Contributor

@sferich888 sferich888 Apr 1, 2019

Choose a reason for hiding this comment

The 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

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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).

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.