Custom queries #3720
Custom queries #3720
Conversation
testing/kuttl/e2e-other/exporter-append-custom-queries-enabled/03--check-queries.yaml
Outdated
Show resolved
Hide resolved
testing/kuttl/e2e-other/exporter-append-custom-queries-enabled/03--check-queries.yaml
Outdated
Show resolved
Hide resolved
testing/kuttl/e2e-other/exporter-append-custom-queries-enabled/10--tls-cluster.yaml
Outdated
Show resolved
Hide resolved
Makefile
Outdated
| @@ -187,7 +187,7 @@ build-postgres-operator-image: build/postgres-operator/Dockerfile | |||
| ##@ Test | |||
| .PHONY: check | |||
| check: ## Run basic go tests with coverage output | |||
| $(GO_TEST) -cover ./... | |||
| QUERIES_CONFIG_DIR="$(CURDIR)/${QUERIES_CONFIG_DIR}" $(GO_TEST) -cover ./... | |||
There was a problem hiding this comment.
🤔 is there a way you can write the go tests so that you don't need access to the queries?
There was a problem hiding this comment.
Hmm... So I added this because one of the tests was failing without it, but I'm realizing that this test was being run as a unit test, but it's really an integration test. So, I'm thinking I should just add the following to the top of the exporter_test.go file:
//go:build envtest
// +build envtest
There was a problem hiding this comment.
📝 we don't have to provide build tags in both of these syntaxes. Since go1.18, go:build is the new, preferred syntax and +build can be able to be removed.
I see that we have them in a bunch of places still - maybe it's worth a story to remove them
| }, | ||
| }, | ||
| } | ||
| configVolume.VolumeSource.Projected.Sources = append(configVolume.VolumeSource.Projected.Sources, |
There was a problem hiding this comment.
Let me just talk this out:
lines 301-310: we always have a configVolume that projects a volume from cluster.Spec.Monitoring.PGMonitor.Exporter.Configuration -- which might be nil, right? (And we've had this code for 2 years now.)
Now, lines 318-329:
- if you have the feature gate turned off & have provided no custom configuration, we add the default queries configmap to that projected volume -- the cluster now has both volumesources, but only default queries
- if you have the feature gate turned on & have provided no custom configuration, we add the default queries configmap to that projected volume -- the cluster now has both volumesources, but only default queries
- if you have the feature gate turned off & have provided custom configuration, we DON'T add the default queries configmap to that projected volume -- the cluster now has only custom queries (and one volumesource)
- if you have the feature gate turned on & have provided custom configuration, we add the default queries configmap to that projected volume -- the cluster now has both custom & default queries
OK, just wanted to type that out.
There was a problem hiding this comment.
🤔 makes me wonder if we should be adding the empty volumeSources?
There was a problem hiding this comment.
We've been doing it for ~2 years, so I guess it's ok, but I don't love it.
There was a problem hiding this comment.
Yeah, if it's not too difficult, I think we should stop doing that. Are there downsides? Would it be possible for someone to get into a situation where we added no volumeSources?
Maybe add a backlog issue and we can plan this
…exporter queries. This ability is feature gated with the AppendCustomQueries flag. The exporter kuttl tests have been adjusted. Tests that need the feature gate turned on have been added to e2e-other. This commit also moves some things from the postgrescluster package to the pgmonitor package.
dsessler7
force-pushed
the
custom_queries
branch
from
3e40f76 to
a0cf241
Compare
Checklist:
Type of Changes:
What is the current behavior (link to any open issues here)?
Currently, the only way that users can provide custom queries to the exporter is by providing a file that will be used instead of the default queries file. This means that if the user wants to keep the defaults, they have to copy the default queries into the custom queries file they provide.
What is the new behavior (if this is a feature change)?
This PR introduces the ability to simply append custom queries to the existing default queries. This ability is feature gated, so it is not a breaking change.
Other Information: