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

The "query frontend" service does not support the grpc_client_config settings #3252

Closed
diranged opened this issue Jan 28, 2021 · 6 comments · Fixed by #4176
Closed

The "query frontend" service does not support the grpc_client_config settings #3252

diranged opened this issue Jan 28, 2021 · 6 comments · Fixed by #4176
Labels
keepalive An issue or PR that will be kept alive and never marked as stale.

Comments

@diranged
Copy link

Describe the bug
While trying to setup mTLS between the Loki nodes on the gRPC ports, we discovered that the query-frontend and ingester_client settings do not support the grpc_client_config keys, even though Cortex does (query_frontend_config, ingester_client_config).

This prevents actually using Loki (in a distributed mode) with gRPC secured by TLS or mTLS.

To Reproduce
Steps to reproduce the behavior:

  1. Implement a Loki "distributed" setup with the loki-distributed helm chart.
  2. Use these Values:
distributor:
  extraVolumeMounts:
    - name: tls
      mountPath: /tls
  extraVolumes:
    - name: tls
      secret:
        defaultMode: 420
        secretName: observability-loki-tls
gateway:
  enabled: true
  extraVolumeMounts:
    - name: tls
      mountPath: /tls
  extraVolumes:
    - name: tls
      secret:
        defaultMode: 420
        secretName: observability-loki-tls
  extraVolumeMounts:
    - name: tls
      mountPath: /tls
  extraVolumes:
    - name: tls
      secret:
        defaultMode: 420
        secretName: observability-loki-tls
  persistence:
    enabled: true
querier:
  extraVolumeMounts:
    - name: tls
      mountPath: /tls
  extraVolumes:
    - name: tls
      secret:
        defaultMode: 420
        secretName: observability-loki-tls
queryFrontend:
  extraVolumeMounts:
    - name: tls
      mountPath: /tls
  extraVolumes:
    - name: tls
      secret:
        defaultMode: 420
        secretName: observability-loki-tls
compactor:
  extraVolumeMounts:
    - name: tls
      mountPath: /tls
  extraVolumes:
    - name: tls
      secret:
        defaultMode: 420
        secretName: observability-loki-tls
  enabled: true
tableManager:
  enabled: false
memcachedExporter:
  enabled: true
memcachedFrontend:
  enabled: true
loki:
  config: |
    table_manager:
      retention_deletes_enabled: false
      retention_period: 0s
    storage_config:
      aws:
        #  Note - this template is relative to the internals of the
        #  loki-disstribution chart, so the .Values reference here doesn't
        #  need to be the fully qualified reference..
        s3: s3://{{ .Values.region }}/{{ .Values.bucket }}
        s3forcepathstyle: true
      boltdb_shipper:
        cache_ttl: 24h
        shared_store: s3
        # /var/loki is created by the helm chart and maps to
        # a persistent local volume for caching. This is
        # frustratingly not documented well.
        active_index_directory: /var/loki/index
        cache_location: /var/loki/boltdb-cache
        resync_interval: 5m
    schema_config:
      configs:
        - from: 2021-01-20
          store: boltdb-shipper
          # "aws": s3 for chunks, dynamo for index
          # "s3": s3 for chunks and index (new boltdb-shipper pattern)
          object_store: s3
          schema: v11
          index:
            prefix: index_
            period: 24h
    server:
      log_level: info
      http_listen_port: 3100  # cannot be customized, do not change.
      http_tls_config:
        cert_file: /tls/tls.crt
        key_file: /tls/tls.key
        client_ca_file: /tls/ca.crt
        #client_auth_type: RequireAndVerifyClientCert
        client_auth_type: VerifyClientCertIfGiven
      grpc_tls_config:
        cert_file: /tls/tls.crt
        key_file: /tls/tls.key
        client_ca_file: /tls/ca.crt
        #client_auth_type: RequireAndVerifyClientCert
        client_auth_type: VerifyClientCertIfGiven
    distributor:
      ring:
        kvstore:
          store: memberlist
    ingester:
      # Disable chunk transfer which is not possible with statefulsets
      # and unnecessary for boltdb-shipper
      max_transfer_retries: 0
      chunk_idle_period: 1h
      chunk_target_size: 1536000
      max_chunk_age: 1h
      lifecycler:
        join_after: 30s
        ring:
          kvstore:
            store: memberlist
          replication_factor: 1
    memberlist:
      join_members:
        - {{ include "loki.fullname" . }}-memberlist

    query_range:
      # make queries more cache-able by aligning them with their step intervals
      align_queries_with_step: true
      max_retries: 5
      # parallelize queries in 15min intervals
      split_queries_by_interval: 15m
      cache_results: true

      results_cache:
        cache:
          memcached_client:
            consistent_hash: true
            host: {{ include "loki.memcachedFrontendFullname" . }}
            max_idle_conns: 16
            service: http
            timeout: 500ms
            update_interval: 1m
    frontend_worker:
      frontend_address: {{ include "loki.queryFrontendFullname" . }}:9095
      grpc_client_config:
        tls_cert_path: /tls/tls.crt
        tls_key_path: /tls/tls.key
        tls_ca_path: /tls/ca.crt
        tls_insecure_skip_verify: true
    frontend:
      grpc_client_config:
        tls_cert_path: /tls/tls.crt
        tls_key_path: /tls/tls.key
        tls_ca_path: /tls/ca.crt
        tls_insecure_skip_verify: true
    ingester_client:
      grpc_client_config:
        tls_cert_path: /tls/tls.crt
        tls_key_path: /tls/tls.key
        tls_ca_path: /tls/ca.crt
        tls_insecure_skip_verify: true

You will receive startup errors in Loki that look like this:

      # 2021-01-25 23:47:25.527904 I | proto: duplicate proto type registered: ingester.Series
      # failed parsing config: /etc/loki/config/config.yaml: yaml: unmarshal errors:
      #   line 134: field grpc_client_config not found in type lokifrontend.Config
    #    2021-01-26 01:17:36.421860 I | proto: duplicate proto type registered: ingester.Series
    #    failed parsing config: /etc/loki/config/config.yaml: yaml: unmarshal errors:
    #      line 138: field tls_cert_path not found in type grpcclient.Config
    #      line 139: field tls_key_path not found in type grpcclient.Config
    #      line 140: field tls_ca_path not found in type grpcclient.Config
    #      line 141: field tls_insecure_skip_verify not found in type grpcclient.Config

Expected behavior
The Loki config file should allow us to use any setting that Cortex supports - rather than being limited to its own subset.

@stale
Copy link

stale bot commented Mar 19, 2021

This issue has been automatically marked as stale because it has not had any activity in the past 30 days. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale A stale issue or PR that will automatically be closed. label Mar 19, 2021
@diranged
Copy link
Author

Re-openinig... this is still an issue AFAIK?

@stale stale bot removed the stale A stale issue or PR that will automatically be closed. label Mar 21, 2021
@diranged
Copy link
Author

diranged commented Apr 6, 2021

This issue is still in place, I believe? Can anyone from the Loki team comment?

@randomchance
Copy link

Not 100% sure it's related in code, but as far as functionality goes this appears to go hand in hand with the fact that the grpc client documentation is wrong #3719 .

I'm trying to use grpc as the interface for internal tooling because I thought the proto and I have yet to get it working :/

@stale
Copy link

stale bot commented Jun 16, 2021

This issue has been automatically marked as stale because it has not had any activity in the past 30 days. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale A stale issue or PR that will automatically be closed. label Jun 16, 2021
@diranged
Copy link
Author

Come on Grafana team... can someone work on implementing this please?

@stale stale bot removed the stale A stale issue or PR that will automatically be closed. label Jun 16, 2021
@owen-d owen-d added the keepalive An issue or PR that will be kept alive and never marked as stale. label Jul 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keepalive An issue or PR that will be kept alive and never marked as stale.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants