Ksonnet: Configurable resource request and limit on all loki components

[c0ffeec0der]

Changes:

Loki.libsonnet (changed)

• Update import for consul from jsonnet-libs/consul to local consul.libsonnet

config.libsonnet (changed)

• Add consul_replicas
• ingester :
  • Update pvc size from 5 to 10Gi (same as ingester.libsonnet)
  • Add replicas, resource request and limit (same as ingester.libsonnet)
• querier : add replicas, resource request and limit (same as querier.libsonnet)
• query-frontend : add resource request and limit (same as query-frontend.libsonnet)
• table-manager : add replicas, resource request and limit (same as table-manager.libsonnet)
• gateway : add replicas, resource request and limit (same as table-manager.libsonnet)
• distributor : add replicas, resource request and limit (same as table-manager.libsonnet)
• storage_section: Reformat section

boltdb_shipper.libsonnet

• Rename stateful parameter

consul.libsonnet (new)

• Add explicit resource requirement (same as jsonnet-libs jsonnet-libs/consul)

ingester.libsonnet (changed)

• Refer replicas, resource request and limit, pvc size to config.libsonnet

querier.libsonnet (changed)

• Refer replicas, resource request and limit to config.libsonnet

query-frontend.libsonnet (changed)

• Refer resource request and limit to config.libsonnet

table-manager.libsonnet (changed)

• Refer replicas, resource request and limit to config.libsonnet

gateway.libsonnet (changed)

• Refer replicas, resource request and limit to config.libsonnet

distributor.libsonnet (changed)

• Refer replicas, resource request and limit to config.libsonnet

memcached.libsonnet (changed)

• Expose max_item_size and memory_limit_mb to config.libsonnet

What this PR does / why we need it:
Provides configurable resource request and limit in config. Beneficial for user who would want to use microservice but start with modest resource

Which issue(s) this PR fixes:
#2687

Special notes for your reviewer:
For memcached_chunks, it looks like tanka have its own calc by container arguments? I am putting the values the same as generated manifest. Suggest correction if the value is incorrect

Checklist

• Documentation added
• Tests updated

[CLAassistant]

All committers have signed the CLA.

[codecov-commenter]

Codecov Report

Merging #2695 into master will decrease coverage by 0.06%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2695      +/-   ##
==========================================
- Coverage   61.46%   61.39%   -0.07%     
==========================================
  Files         173      173              
  Lines       13421    13421              
==========================================
- Hits         8249     8240       -9     
- Misses       4421     4426       +5     
- Partials      751      755       +4     

Impacted Files	Coverage Δ
pkg/promtail/targets/file/tailer.go	68.53% <0.00%> (-4.50%)	⬇️
pkg/promtail/targets/file/filetarget.go	64.08% <0.00%> (-2.12%)	⬇️
pkg/logql/evaluator.go	92.38% <0.00%> (-0.43%)	⬇️

[owen-d]

Hey, thanks for the PR. You've got the right idea, but these meta parameters shouldn't be under the $._config.loki prefix because that actually gets marshalled into the configmap here. If you were to run this now, all the loki pods would crash because of these unexpected fields in their config.

That being said, I really like what this PR is trying to do. You can follow the example started by the querier & frontend here by using the $._config prefix instead ($._config.querier, etc). It may even be a good time to develop a specific prefix for these instead, such as $._config.components to centralize all these meta parameters such as $._config.components.{querier,queryFrontend,etc}

[owen-d]

This is better handled by the options exposed in the library, which also account for passing the args to memcached among other things:
https://github.com/grafana/jsonnet-libs/blob/master/memcached/memcached.libsonnet#L22-L31

[c0ffeec0der]

Ok, I am passing max_item_size and memory_limit_mb args to config.libsonnet

[owen-d]

I'm not sure if we should add this file if we don't make any changes.

[c0ffeec0der]

Sorry, I forgot to expose them to config.libsonnet

[c0ffeec0der]

There is unfortunate limitation in tanka that it could not refer to <$._config.components>. Failed on runtime.. It seems refering to nested variable is not supported yet. (possible different parser engine with https://github.com/google/jsonnet) so I have to put in $._config

[c0ffeec0der]

@owen-d Hello, any updates, comments?

[c0ffeec0der]

Hello @owen-d, I just saw there was a file conflict. It's corrected now. Please review and let me know your thoughts. Thank you

[owen-d]

Hey, I don't think I was clear enough earlier, but I was suggesting we create the $._config.components object to store this data. It doesn't as of yet exist and that's why you were seeing errors there.

After talking about this issue with @slim-bean, I'm questioning if adding these parameters is actually helpful. Jsonnet, unlike helm, can easily be arbitrarily overridden, so adding these parameter fields may just contribute to code bloat in the long term. I apologize that I've changed my tune on this PR; I'm sure it's frustrating.

I'm in favor of closing this PR now, but am open to other opinions. @slim-bean @cyriltovena thoughts?

[c0ffeec0der]

Hey, I don't think I was clear enough earlier, but I was suggesting we create the $._config.components object to store this data. It doesn't as of yet exist and that's why you were seeing errors there.

I had created $._config.components object but it could not be referenced to from config.libsonnet or other files. There is a limitation in tanka that during runtime it could only call parse reference to '$._config'. I know it's odd, but I ran to runtime error when using $._config.components and tried to reference it.

After talking about this issue with @slim-bean, I'm questioning if adding these parameters is actually helpful. Jsonnet, unlike helm, can easily be arbitrarily overridden

Yes, I agree jsonnet can be overriden. But when installing loki with tanka, I could not change the resource request from environments/loki/main.jsonnet, that is why I go through the troubles exposing them all in config.libsonnet. If it's not a trouble, could you please show me how to override the resource request from environments/loki/main.jsonnet?

[owen-d]

Absolutely! You can do it via

  querier_container+::
    $.util.resourcesRequests('2', '2Gi')

where $.util.resourcesRequests is a function already imported and defined here

[c0ffeec0der]

Hello @owen-d

Thank you! This is a relief! I was figuring out how to do it in main.jsonnet for quite a time before changing config.libsonnet

I am posting the solution here for other people who want to adjust the resource

// adjust resource
gateway_container+::
$.util.resourcesRequests('10m', '10Mi') +
$.util.resourcesLimits('20m','20Mi'),

// adjust replica
gateway_deployment+:
$.apps.v1.deployment.mixin.spec.withReplicas(1),

// adjust resource for memcached
memcached_chunks+: {
memory_limit_mb: 1024,
memcached_container+::
$.util.resourcesRequests('10m', $.util.bytesToK8sQuantity(self.memory_request_bytes)) +
$.util.resourcesLimits('20m',$.util.bytesToK8sQuantity(self.memory_limits_bytes)),
},

Closing the PR :)

[owen-d]

:)