-
Notifications
You must be signed in to change notification settings - Fork 487
Generate correctly the MinIO scrape configs for Multi Tenants scenarios #2456
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
any updates on this when this will be merged? |
@@ -304,6 +304,9 @@ func NewController( | |||
} | |||
controller.enqueueTenant(newObj) | |||
}, | |||
// Enqueue tenant to perform some delete handling actions | |||
// during reconciliation | |||
DeleteFunc: controller.enqueueTenant, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any actual deletes being added. It resources need to be removed when the Tenant
is removed, then they should use the Tenant
as the owner of the resource, so deletion is automatic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ramondeklein the prometheus scrape configs are stored by design in same secret for all tenants so it can not have a owner reference.
@JoelRuizRojas Is this still relevant? |
Yes @ramondeklein this is still needed, the same for AIStor (we have a PR there too). Sorry I had missed your comment above. |
Description
Next fixes introduced:
minio-prom-additional-scrape-config
to fix this issue.prometheusagents
resources (as with Helm) when deploying the Operator usingkustomize
.A few unit tests were added to cover points (1) and (2) above.
Note: I need to port this fix to AIStor
Related Issue
Fixing issues:
#2425
#2218
Type of Change
Screenshots (if applicable e.g before/after)
Checklist
Test Steps
kind load docker-image minio/operator:noop
(loads locally built image to kind cluster)helm install minio-operator helm/operator --namespace minio-operator --create-namespace
(install Operator)helm install prometheus-operator prometheus-community/kube-prometheus-stack
(installs prometheus controllers stack and grafana controller)helm install my-tenant helm/tenant --namespace tenant-ns --create-namespace
(installs a tenant in a given namespace)helm install my-tenant helm/tenant --namespace tenant-ns --create-namespace
(installs a tenant in another given namespace)prometheusOperator: true
minio-prom-additional-scrape-config
prometheusOperator: false
so that the tenant scrape configs are removed fromminio-prom-additional-scrape-config
secret. Or add more metric endpoint with tenant configPrometheusOperatorScrapeMetricsPaths
and verify the scrape configs are added to the secret.Additional Notes / Context