Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Support multi-tenancy in TempoMonolithic CR #816

Merged

Conversation

andreasgerstmayr
Copy link
Collaborator

@andreasgerstmayr andreasgerstmayr commented Feb 21, 2024

Support multi-tenancy in TempoMonolithic CR.

@andreasgerstmayr andreasgerstmayr changed the title Support multitenancy in TempoMonolithic CR (3/3) Support multi-tenancy in TempoMonolithic CR (3/3) Feb 22, 2024
@codecov-commenter
Copy link

codecov-commenter commented Feb 22, 2024

Codecov Report

Attention: Patch coverage is 84.19689% with 61 lines in your changes are missing coverage. Please review.

Project coverage is 75.25%. Comparing base (03d8e5f) to head (503200c).

Files Patch % Lines
internal/manifests/monolithic/build.go 0.00% 12 Missing and 2 partials ⚠️
internal/manifests/manifestutils/tls.go 0.00% 9 Missing ⚠️
internal/manifests/monolithic/configmap.go 25.00% 7 Missing and 2 partials ⚠️
internal/manifests/monolithic/statefulset.go 94.91% 6 Missing and 3 partials ⚠️
controllers/tempo/tempomonolithic_controller.go 53.33% 6 Missing and 1 partial ⚠️
internal/manifests/monolithic/gateway.go 86.95% 4 Missing and 2 partials ⚠️
internal/manifests/naming/naming.go 0.00% 4 Missing ⚠️
internal/manifests/monolithic/serviceaccount.go 62.50% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #816      +/-   ##
==========================================
+ Coverage   74.26%   75.25%   +0.99%     
==========================================
  Files          89       90       +1     
  Lines        6587     6932     +345     
==========================================
+ Hits         4892     5217     +325     
- Misses       1453     1463      +10     
- Partials      242      252      +10     
Flag Coverage Δ
unittests 75.25% <84.19%> (+0.99%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@andreasgerstmayr andreasgerstmayr force-pushed the monolithic-multitenancy branch 3 times, most recently from 788050c to cc26d4f Compare February 22, 2024 15:55
@andreasgerstmayr andreasgerstmayr marked this pull request as ready for review February 22, 2024 15:57
@andreasgerstmayr andreasgerstmayr marked this pull request as draft February 27, 2024 13:42
@andreasgerstmayr andreasgerstmayr force-pushed the monolithic-multitenancy branch 7 times, most recently from 289fb57 to da40700 Compare February 28, 2024 18:02
@andreasgerstmayr andreasgerstmayr changed the title Support multi-tenancy in TempoMonolithic CR (3/3) Support multi-tenancy in TempoMonolithic CR (depends on #814 and #815) Mar 11, 2024
@andreasgerstmayr andreasgerstmayr changed the title Support multi-tenancy in TempoMonolithic CR (depends on #814 and #815) Support multi-tenancy in TempoMonolithic CR Mar 12, 2024
@andreasgerstmayr andreasgerstmayr force-pushed the monolithic-multitenancy branch 3 times, most recently from 1fdddba to 1767cad Compare March 13, 2024 17:27
Signed-off-by: Andreas Gerstmayr <agerstmayr@redhat.com>
@andreasgerstmayr andreasgerstmayr force-pushed the monolithic-multitenancy branch from 1767cad to 7b67fa9 Compare March 13, 2024 17:33
@andreasgerstmayr andreasgerstmayr marked this pull request as ready for review March 13, 2024 17:34
@andreasgerstmayr andreasgerstmayr force-pushed the monolithic-multitenancy branch from d2fc200 to e0dcbe6 Compare March 14, 2024 12:25
Signed-off-by: Andreas Gerstmayr <agerstmayr@redhat.com>
@andreasgerstmayr andreasgerstmayr force-pushed the monolithic-multitenancy branch from e0dcbe6 to 590ca4e Compare March 14, 2024 14:04
Signed-off-by: Andreas Gerstmayr <agerstmayr@redhat.com>
Signed-off-by: Andreas Gerstmayr <agerstmayr@redhat.com>
// The internal server is required because if the gateway is enabled,
// the Tempo API will listen on localhost only,
// and then Kubernetes cannot reach the health check endpoint.
config.InternalServer.Enable = true
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wondering if we cannot have the internal server always enabled? :D

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I think we should always enable it in the TempoStack as well.
It is always enabled in TempoMonolithic.

Copy link
Collaborator

@rubenvp8510 rubenvp8510 left a comment

Choose a reason for hiding this comment

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

Small comment, no blocker LGTM!

@andreasgerstmayr andreasgerstmayr merged commit eed0197 into grafana:main Mar 15, 2024
11 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants