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

Proposal: Helm - Allow loading all alloy files in Alloy configuration directory #1176

Open
fculpo opened this issue Jun 28, 2024 · 7 comments
Labels
proposal A proposal for new functionality.

Comments

@fculpo
Copy link

fculpo commented Jun 28, 2024

Background

Actual Helm chart is designed to load a single .alloy file (entrypoint) from a generated or provided configMap, that could in turn import other files via module paradigm (coming from a different configmap, or any other mount volumes).

This behaviour, even with modules, makes overrides or unloading components in some clusters difficult when using a common configuration.

Proposal

Helm chart should implement this Alloy cli feature: 0b0b4a5

By loading all .alloy files in a folder, files mounted from provided configMap or other mean can then be overridden or removed by k8s operators in some environments/clusters in order to achieve greater modularity (terraform like folder loading).

This proposal is backward compatible as it is opt-in and disabled by default.

Usage example

When using Pyroscope / Loki / Tempo / etc. and pushing data to a central observability cluster, one may (should) need we basic auth on the ingress in front of distributors.

However, on some cluster (maybe the observability cluster itself), it makes little sense to send data through https over the public ingress domain. It is probably better to use in-cluster routing, talking directly to distributor or gateway (not the ingress).

The actual issue

Today it is not possible to if-else blocks in components (such as basic-auth in endpoint of pyroscope.write)

So for almost all clusters pyroscope-push.alloy is:

pyroscope.write "grafana" {
  external_labels = {
    cluster = env("CLUSTER"),
  }
  endpoint {
    url = env("PYRO_ENDPOINT_URL")
    basic_auth {
	username = "pyroscope"
	password = env("PYRO_API_KEY")
    }
  }
}

But for the other clusters I want to push via internal service, not over ingress, hence without auth, we need to have something like

pyroscope.write "grafana" {
  external_labels = {
	cluster = env("CLUSTER"),
  }
  endpoint {
    url = env("PYRO_ENDPOINT_URL")
  }
}

So in these specific environments, we just have to override the pyroscope-push.alloy file, and everything will work nicely.

Detailed example of needed use case:
Common (all clusters) Helm chart values and configMap generation
Common Helm values (for most of clusters) - Jsonnet/Flux code
local fluxcd = import 'fluxcd.libsonnet';
local helmRelease = fluxcd.helmRelease;
local helmRepository = fluxcd.helmRepository;

local k = import 'k.libsonnet';
local configMap = k.core.v1.configMap;
local envVar = k.core.v1.envVar;

local name = 'grafana-alloy';
local namespace = 'grafana-alloy';

{
_config:: {
  version: '<0.4',
  cluster: error 'cluster required',
  pyro_url: 'https://sth/',
  values: {
    alloy: {
      stabilityLevel: 'public-preview',
      configMap+: {
        create: false, // we provide our own CM outside of chart (classic pattern)
        name: $.configMap.metadata.name, // reference the provided CM name
        key: 'config.river', // This key would be unused in this proposal as we load all files in directory
      },
      mounts: {
        varlog: true,
        dockercontainers: true,
      },
      securityContext: {
        privileged: true,
        runAsNonRoot: false,
      },
      extraEnv: [
        envVar.new('CLUSTER', $._config.cluster),
        envVar.new('PYRO_ENDPOINT_URL', $._config.pyro_url),
      ],
      envFrom: [
        {
          secretRef: {
            name: name, // Pyro / Loki / Tempo / etc. ingress authentication
          },
        },
      ],
    },
    controller: {
      hostPID: true,  // needed for java profiling
    },
    service: {
      internalTrafficPolicy: 'Local', // route to local DS pod
    },
  },
},

// PROPOSAL
// We provide our own configmap with multiples files
// Files will be automatically mounted in /etc/alloy by the chart
configMap:
  configMap.new(name) +
  configMap.metadata.withNamespace(namespace) +
  configMap.withData({
    'config.alloy': importstr './config/config.alloy',
    'pyroscope.alloy': importstr './config/pyroscope.alloy',
    // We separate the push/write elements in another file
    // so we can override this in another cluster (override configMap data key)
    'pyroscope-push.alloy': importstr './config/pyroscope-push.alloy',
  }),

helmRepository:
  helmRepository.new(name) +
  helmRepository.metadata.withNamespace(namespace) +
  helmRepository.spec.withInterval('1h') +
  helmRepository.spec.withUrl('https://grafana.github.io/helm-charts'),

helmRelease:
  helmRelease.new(name) +
  helmRelease.metadata.withNamespace(namespace) +
  helmRelease.spec.chart.spec.withChart('alloy') +
  helmRelease.spec.chart.spec.withVersion($._config.version) +
  helmRelease.spec.chart.spec.sourceRef.withKind('HelmRepository') +
  helmRelease.spec.chart.spec.sourceRef.withName($.helmRepository.metadata.name) +
  helmRelease.spec.chart.spec.sourceRef.withNamespace($.helmRepository.metadata.namespace) +
  helmRelease.spec.withValues($._config.values) +
  // WARNING: following is our actual fix implementation
  // This should be unneeded if proposal is Accepted
  helmRelease.spec.withPostRenderers([
    helmRelease.spec.postRenderers.kustomize.withPatches([
      // Override command line argument of alloy container to target a directory instead of a single file
      {
        target: {
          version: 'v1',
          kind: 'DaemonSet',
          name: 'grafana-alloy',
        },
        // This is the actual magic
        patch: |||
          - op: replace
            path: /spec/template/spec/containers/0/args/1
            value: /etc/alloy
        |||,
      },
    ]),
  ]),
}
Specific cluster
// import the common Helm chart
local agent = import 'grafana-agent/main.libsonnet';

local k = import 'k.libsonnet';
local configMap = k.core.v1.configMap;

{
  agent: agent {
    _config+:: {
      pyro_url: 'http://pyroscope-querier.pyroscope.svc:4040'
    },

    configMap+:
      configMap.withDataMixin({
        // we override this file to use in-cluster endpoint
        'pyroscope-push.river': importstr './config/pyroscope-push.river',
      }),
  },
}

Helm Chart code proposal

See. https://github.com/grafana/alloy/pull/1016/files

Basically, we could add a boolean flag changing the path passed to alloy run command

Container template

 args:
    - run
    {{- if .Values.alloy.loadFolder }}
    - /etc/alloy/
    {{- else }}
    - /etc/alloy/{{ include "alloy.config-map.key" . }}
    {{- end }}

Values.yaml

alloy:
  # -- If true, agent will load all files in /etc/alloy.
  # alloy.configMap.key will be ignored
  loadFolder: false

  configMap:
    # -- Create a new ConfigMap for the config file.
    create: true
    # -- Name of existing ConfigMap to use. Used when create is false.
    name: null
    # -- Key in ConfigMap to get config from.
    # Ignored if loadFolder is true.
    key: null
@fculpo fculpo added the proposal A proposal for new functionality. label Jun 28, 2024
@fculpo fculpo changed the title Helm - Allow loading all alloy files in Alloy configuration directory Proposal: Helm - Allow loading all alloy files in Alloy configuration directory Jun 28, 2024
@vlada-dudr
Copy link

this could be useful also for classic server management. One could simply have a directory to drop in files via Ansible, or package management. I was originally looking for classic include dir/*.alloy which would just literally include all the files verbatim.

@thampiotr
Copy link
Contributor

this could be useful also for classic server management. One could simply have a directory to drop in files via Ansible, or package management. I was originally looking for classic include dir/*.alloy which would just literally include all the files verbatim.

If you're not using a helm chart, you can already do this by providing a directory path instead of a file path: https://grafana.com/docs/alloy/next/reference/cli/run/

@vlada-dudr
Copy link

vlada-dudr commented Jul 12, 2024 via email

@thampiotr
Copy link
Contributor

I think this new option in the helm chart should ideally allow to provide a path to either a file or a folder, which will be passed to alloy run command. This way it'd be covering more potential use cases and would map nicely to alloy CLI.

An even more generic option would be to override all arguments 🤔 and disable the config map.

@Ronan-WeScale
Copy link

Hi !
Really interested about this PR #1016

@wildum
Copy link
Contributor

wildum commented Oct 7, 2024

@petewall what do you think about the proposal and the PR?

@petewall
Copy link
Contributor

I think the idea of Alloy loading configs from a directory (anything ending in ___.alloy?) makes some sense.
I'm adding some comments in the PR about the specific implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal A proposal for new functionality.
Projects
Status: Active
Development

No branches or pull requests

6 participants