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

Add DD_AGENT_IPC_* env vars #1661

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Add DD_AGENT_IPC_* env vars #1661

wants to merge 5 commits into from

Conversation

mackjmr
Copy link
Member

@mackjmr mackjmr commented Jan 7, 2025

What this PR does / why we need it:

This PR adds DD_AGENT_IPC_PORT and DD_AGENT_IPC_CONFIG_REFRESH_INTERVAL env vars in otel agent and core agent. It does not override them if they are added by the user. This is necessary for the otel-agent to pull the api key from core config in the case where backend secrets are used.

This PR also adds flag --sync-delay=30s to otel agent.

Which issue this PR fixes

OTEL-2323

Special notes for your reviewer:

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • Chart Version bumped
  • Documentation has been updated with helm-docs (run: .github/helm-docs.sh)
  • CHANGELOG.md has been updated
  • Variables are documented in the README.md
  • For Datadog Operator chart or value changes update the test baselines (run: make update-test-baselines)

This PR adds DD_AGENT_IPC_PORT and DD_AGENT_IPC_CONFIG_REFRESH_INTERVAL env vars in otel agent and core agent. It does not override them if they are added by the user. This is necessary for the otel-agent to pull the api key from core config in the case where backend secrets are used.
@mackjmr mackjmr requested a review from a team as a code owner January 7, 2025 16:24
@github-actions github-actions bot added the chart/datadog This issue or pull request is related to the datadog chart label Jan 7, 2025
Copy link
Contributor

@levan-m levan-m left a comment

Choose a reason for hiding this comment

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

Could you please consider adding test like this to make sure chart doesn't break and it works as intended.

@@ -59,6 +59,38 @@
{{- include "containers-common-env" . | nindent 4 }}
{{- include "fips-envvar" . | nindent 4 }}
{{- include "processes-common-envs" . | nindent 4 }}
{{- if eq (include "should-enable-otel-agent" .) "true" }}
{{- $found := false }}
{{- range .Values.agents.containers.agent.env }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you extract the $found computation to _helpers.tpl? like this e.g.:

{{/*
Return value of "DD_PROCESS_CONFIG_RUN_IN_CORE_AGENT_ENABLED" env var in core agent container.
*/}}
{{- define "get-process-checks-in-core-agent-envvar" -}}
{{- range .Values.agents.containers.agent.env -}}
{{- if eq .name "DD_PROCESS_CONFIG_RUN_IN_CORE_AGENT_ENABLED" -}}
{{- .value -}}
{{- end -}}
{{- end -}}
{{- end -}}

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved logic to helpers in: 267473a.

{{- end }}
{{- $found := false }}
{{- range .Values.agents.containers.agent.env }}
{{- if eq .name "DD_AGENT_IPC_CONFIG_REFRESH_INTERVAL" }}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see this or DD_AGENT_IPC_PORT used elsewhere in the chart, do we expect them to be added?

Copy link
Member Author

Choose a reason for hiding this comment

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

Youre right, we dont set this in helm charts. This is to make sure we don't override it if the user sets it in datadog.env, Values.agents.containers.agent.env or Values.agents.containers.otelAgent.env.

@mackjmr
Copy link
Member Author

mackjmr commented Jan 14, 2025

Thanks for the review @levan-m, I've addressed the feedback and added a test in: a20b97c. Unfortunately, it seems like I am unable to test the option to not override user provided env vars, as setting something like:

					"datadog.env": `
- name: DD_AGENT_IPC_PORT
  value: 1234
- name: DD_AGENT_IPC_CONFIG_REFRESH_INTERVAL
  value: 13`,

in the test cases does not reflect these env vars in the test results, so I am guessing this gets done at another stage ? I've tested this manually in a test k8s cluster, and the not override logic is taken into account, I'm just not able to test this programatically.

I was getting the same issue in not being able to test that we don't override in the operator PR: DataDog/datadog-operator#1604 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chart/datadog This issue or pull request is related to the datadog chart
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants