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.

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

Prometheus on Kubernetes Example #15

Closed
wants to merge 9 commits into from

Conversation

bill3tt
Copy link
Collaborator

@bill3tt bill3tt commented Jul 30, 2019

This PR implements an example which defines and generates all of the configuration needed to run a fully-featured Prometheus instance on Kubernetes.

@bill3tt bill3tt marked this pull request as ready for review July 31, 2019 17:02
@bill3tt bill3tt requested a review from domgreen July 31, 2019 17:02
@bill3tt bill3tt force-pushed the prometheus-kubernetes-example branch from 7a6aea7 to 07a2717 Compare August 1, 2019 09:50
@bill3tt bill3tt requested a review from bwplotka August 2, 2019 15:24
Copy link
Owner

@bwplotka bwplotka left a comment

Choose a reason for hiding this comment

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

Nice! Some suggestions.

namespace = "default"
alertManagerPort = 9093
// This constant is not seemingly available in any of the k8s libraries
imagePullPolicyIfNotPresent = "IfNotPresent"
Copy link
Owner

Choose a reason for hiding this comment

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

v1.PullIfNotPresent from v1 "k8s.io/api/core/v1"

)

var (
int32One = int32(1)
Copy link
Owner

Choose a reason for hiding this comment

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

kill this and let's use inlined swag.Int32 (:

},
Scheme: "https",
HTTPClientConfig: config.HTTPClientConfig{
TLSConfig: config.TLSConfig{
Copy link
Owner

Choose a reason for hiding this comment

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

I think this is default, plus it repeats everywhere. Can we have helper here? (:

},
}

// TODO: Is there a cleaner way of doing this?
Copy link
Owner

Choose a reason for hiding this comment

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

Nope, but it's actually neat (: Looks legit.

Copy link
Owner

Choose a reason for hiding this comment

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

We could have helper for that that saves 2 lines, but that's it (:

Copy link
Owner

Choose a reason for hiding this comment

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

we should add this to mimic:

func GenerateInPlace(r io.Reader) []byte {
	b, err := ioutil.ReadAll(r)
	if err != nil {
		mimic.PanicErr(err)
	}
	return b
}

},
}

generator.Add("server-configmap.yaml", encoding.YAML(serverConfigMap))
Copy link
Owner

Choose a reason for hiding this comment

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

Wonder - we should do it at the end or one by one?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmmm - trying them both out I like doing it all at the end better. That way we are grouping concerns together rather than having them spread out. Easier to reason about.

}

func generateKubeStateMetrics(generator *mimic.Generator) {
// Kube-state-metrics
Copy link
Owner

Choose a reason for hiding this comment

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

ditto


func generateNodeExporter(generator *mimic.Generator) {

// Node-exporter
Copy link
Owner

Choose a reason for hiding this comment

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

ditto

{
Name: "metrics",
Port: 9100,
Protocol: "TCP",
Copy link
Owner

Choose a reason for hiding this comment

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

ditto

@@ -2,17 +2,19 @@ module github.com/bwplotka/mimic

go 1.12

replace github.com/prometheus/alertmanager => github.com/bwplotka/alertmanager v0.1.0-mimic-am-config
Copy link
Owner

Choose a reason for hiding this comment

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

👍

github.com/beorn7/perks v0.0.0-20180321164747-3a771d992973/go.mod h1:Dwedo/Wpr24TaqPxmxbtue+5NUziq4I4S80YR8gNf3Q=
github.com/beorn7/perks v1.0.0 h1:HWo1m869IqiPhD389kmkxeTalrjNbbJTC8LXupb+sl0=
github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+CedLV8=
github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs=
github.com/bwplotka/alertmanager v0.1.0-mimic-am-config h1:ayiZ+4jriq3cFuI4QbecQUdbmCZXiuQE8jm+ZuMxSTI=
Copy link
Owner

Choose a reason for hiding this comment

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

I think it's must have at some point to maintain separate go.mod for all examples as well as abstractions. Otherwise using mimic.Generator would pull all those dependencies..... even though the user might want Kubernetes only for example.

Copy link
Owner

Choose a reason for hiding this comment

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

cc @ianbillett @domgreen

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah that makes sense - @domgreen had a similar idea recently suggesting pulling out the examples into a different repo.

I think its important, at least initially, that the examples live close to the implementation code. Different repos seem too far apart. Create #16 to discuss.

@bill3tt bill3tt force-pushed the prometheus-kubernetes-example branch from a51df2c to 54b008a Compare October 13, 2019 18:32
@bwplotka
Copy link
Owner

Great idea, thanks! But we need somebody who would finish this up. Closing for now (: I talked with you offline and I know you are busy with other topics, so closing.

@bwplotka bwplotka closed this Feb 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants