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

Support configuring resource requests and limits in TempoMonolithic CR #771

Conversation

andreasgerstmayr
Copy link
Collaborator

Support configuring resource requests and limits in TempoMonolithic CR

@andreasgerstmayr andreasgerstmayr force-pushed the tempo-monolithic-resource-limits branch 2 times, most recently from 6d087b6 to 24355ed Compare February 1, 2024 17:07
Signed-off-by: Andreas Gerstmayr <agerstmayr@redhat.com>
@andreasgerstmayr andreasgerstmayr force-pushed the tempo-monolithic-resource-limits branch from 24355ed to b7899d1 Compare February 1, 2024 17:09
// S3 defines the AWS S3 configuration
//
// +kubebuilder:validation:Optional
S3 *MonolithicTracesStorageS3Spec `json:"s3,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

these changes seem to be unrelated to the resources. Can it be done in a separate PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it was a change before I rebased on main. The change doesn't show up in the diff anymore.

@@ -135,6 +182,7 @@ type MonolithicIngestionOTLPProtocolsHTTPSpec struct {
// Enabled defines if OTLP over HTTP is enabled
//
// +kubebuilder:validation:Required
// +kubebuilder:default=true
Copy link
Collaborator

Choose a reason for hiding this comment

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

also this one seems to be unrelated

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it was a change before I rebased on main. The change doesn't show up in the diff anymore.

)

// ListFieldErrors converts field.ErrorList to a comma separated string of errors.
func ListFieldErrors(fieldErrs field.ErrorList) string {
Copy link
Collaborator

Choose a reason for hiding this comment

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

does this need to be exposed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's used in the controller package. I'll move it there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done: #772

@andreasgerstmayr andreasgerstmayr merged commit a75e929 into grafana:main Feb 1, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants