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

[WIP] Make Loki Compute resources configurable #72

Closed

Conversation

4n4nd
Copy link
Contributor

@4n4nd 4n4nd commented May 21, 2021

Resolves #71

Depends on observatorium/observatorium#411

Will need to update the jsonnet dependency lock after the above PR is merged

@4n4nd 4n4nd force-pushed the loki-resources-configurable branch 2 times, most recently from 5b90971 to 494442b Compare May 21, 2021 15:57
@4n4nd
Copy link
Contributor Author

4n4nd commented May 21, 2021

@periklis I tried to implement what you suggested in your comment, but when testing this doesn't work for me.
when I add component resources in loki spec, example:

  loki:
    resources:
      compactor:
        limits:
          cpu: 200m
          memory: 200Mi
        requests:
          cpu: 100m
          memory: 100Mi

the operator just updates the CR and changes the resources to resources: {} and the default values are used for compute resources. Do you have any suggestions on how I could debug this?

@periklis
Copy link

@periklis I tried to implement what you suggested in your comment, but when testing this doesn't work for me.
when I add component resources in loki spec, example:

  loki:
    resources:
      compactor:
        limits:
          cpu: 200m
          memory: 200Mi
        requests:
          cpu: 100m
          memory: 100Mi

the operator just updates the CR and changes the resources to resources: {} and the default values are used for compute resources. Do you have any suggestions on how I could debug this?

The implemtation in this PR is incomplete to achieve what I suggested, i.e. per component resource requirement settings. A working solution looks like this:

  1. Amend the observatorium_types.go like this. I suggest to make the map to a struct because it is more visible for CRD users and the components are fully enumerable without making mistakes with the keys:
type LokiResourcesSpec {
    Compactor *v1.ResourceRequirements
    Distributor *v1.ResourceRequirements
    ...
}
type LokiSpec struct {
    ...
    Resources *LokiResourcesSpec
}
  1. Adapt obs-operator.jsonnet to read the new fields from the spec and pass them to the loki jsonnet configuration.

@4n4nd 4n4nd force-pushed the loki-resources-configurable branch 2 times, most recently from 0c0c2f6 to 8b69150 Compare May 27, 2021 17:32
@4n4nd 4n4nd force-pushed the loki-resources-configurable branch from 8b69150 to 3090ac0 Compare May 27, 2021 17:33
@4n4nd
Copy link
Contributor Author

4n4nd commented May 27, 2021

@periklis thanks for your help!
I was able to test this now 👍
This PR depends on observatorium/observatorium#411 for the example manifest generation and default values for loki compute resources. Once that is merged, I will update the jsonnet dependencies in this one and this should be good to go.

Comment on lines +383 to +391
Resources *LokiResourcesSpec `json:"resources,omitempty"`
}

type LokiResourcesSpec struct {
Compactor *v1.ResourceRequirements `json:"compactor,omitempty"`
Distributor *v1.ResourceRequirements `json:"distributor,omitempty"`
Ingester *v1.ResourceRequirements `json:"ingester,omitempty"`
Querier *v1.ResourceRequirements `json:"querier,omitempty"`
QueryFrontend *v1.ResourceRequirements `json:"query_frontend,omitempty"`

Choose a reason for hiding this comment

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

As mentioned on the observatorium/observatorium#411 PR, this needs to slightly change to:

type LokiSpec struct {
    Components *LokiComponentsSpec
}

type LokiComponentsSpec struct {
    Compactor *LokiComponentSpec
    ...
}
type LokiComponentSpec {
   Resources *v1.ResourceRequirements `json:"resources,omitempty"`
}

@4n4nd 4n4nd closed this Oct 6, 2021
saswatamcode pushed a commit to saswatamcode/observatorium-operator that referenced this pull request May 21, 2024
Signed-off-by: Subbarao Meduri <[email protected]>
This pull request was closed.
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.

Loki components compute resources are not configurable
2 participants