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

[CONTP-376] expose kubernetes_resources_labels/annotations_as_tags #1379

Conversation

adel121
Copy link
Contributor

@adel121 adel121 commented Aug 29, 2024

What does this PR do?

This PR exposes kubernetesResourcesLabelsAsTags and kubernetesResourcesAnnotationsAsTags in the DD operator.

Feature implemented in this PR.

Motivation

Exposing the feature.

Additional Notes

We also create DCA rbacs necessary for the feature to work as expected.

Minimum Agent Versions

Are there minimum versions of the Datadog Agent and/or Cluster Agent required?

Agent: v7.58.0
Cluster Agent: v7.58.0

Describe your test plan

Deploy the following DatadogAgent with the operator:

apiVersion: datadoghq.com/v2alpha1
kind: DatadogAgent
metadata:
  name: datadog
  namespace: system
spec:
  global:
    credentials:
      apiSecret:
        secretName: datadog-secret
        keyName: api-key
    kubelet:
      tlsVerify: false

    kubernetesResourcesLabelsAsTags:
      pods:
        foo: "bar"
        bar: "baz"
      deployments.apps:
        foo: "bar"
        bar: "baz"

    kubernetesResourcesAnnotationsAsTags:
      pods:
        foo: "bar1"
        bar: "baz"
      deployments.apps:
        foo: "bar"
        bar: "baz"
      daemonsets.apps:
        agent.datadoghq.com/agentspechash: hash

Create a deployment with these labels and annotations set on the deployment object and the children pods:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: python-process-deployment
  labels:
    app: python-process-app
    bar: label1
    foo: label2
  annotations:
    bar: label3
    foo: label4
spec:
  replicas: 3
  selector:
    matchLabels:
      app: python-process-app
  template:
    metadata:
      labels:
        app: python-process-app
        bar: label1
        foo: label2
      annotations:
        bar: label3
        foo: label4
    spec:
      containers:
        - name: python-process-container
          image: python:3.7 # Replace with your Python image
          command: ["python3", "-u", "your-python-script.py"]
          volumeMounts:
            - name: python-script-volume
              mountPath: /your-python-script.py
              subPath: your-python-script.py
      volumes:
        - name: python-script-volume
          configMap:
            name: python-script-configmap
---
apiVersion: v1
kind: ConfigMap
metadata:
  name: python-script-configmap
data:
  your-python-script.py: |
    while True:
      pass

Verify that the proper rbacs were created, in this case we should have get, list and watch permissions on the following resources:

  • deployments
  • pods
  • daemonsets

You can verify this by checking the clusterroles that were created, one of them should be related to metadata as tags:

kubectl get clusterrole datadog-cluster-agent-kubernetes-metadata-as-tags -o yaml | grep -A5000 rules:
      f:rules: {}
    manager: datadog-operator
    operation: Update
    time: "2024-08-29T13:10:17Z"
  name: datadog-cluster-agent-kubernetes-metadata-as-tags
  resourceVersion: "76104"
  uid: 459abb0d-0f55-4105-8056-b046e27bb316
rules:
- apiGroups:
  - apps
  resources:
  - deployments
  verbs:
  - get
  - list
  - watch
- apiGroups:
  - apps
  resources:
  - daemonsets
  verbs:
  - get
  - list
  - watch
- apiGroups:
  - ""
  resources:
  - pods
  verbs:
  - get
  - list
  - watch

Also verify that the feature is working by checking the content of the tagger.

You should be able to see bar1:label4 bar:label2 baz:label1 baz:label3 tags on the created deployments and its children pods. And you should see the hash label (e..g. hash:27a53207eedd862383f608e9a6aa1341) on the datadog-agent daemonset within a kubernetes_metadata entity.

kubectl exec <cluster-agent-pod-name> -c cluster-agent -- agent tagger-list

=== Entity kubernetes_pod_uid://c9befcc6-f60b-4b17-8fa7-7d5837380f19 ===
== Source workloadmeta-kubernetes_pod =
=Tags: [bar1:label4 bar:label2 baz:label1 baz:label3 kube_deployment:python-process-deployment kube_namespace:system kube_ownerref_kind:replicaset kube_ownerref_name:python-process-deployment-7c4cf7cdc7 kube_qos:BestEffort kube_replica_set:python-process-deployment-7c4cf7cdc7 pod_name:python-process-deployment-7c4cf7cdc7-7q5ht pod_phase:running]

=== Entity deployment://system/python-process-deployment ===
== Source workloadmeta-kubernetes_deployment =
=Tags: [bar:label2 bar:label4 baz:label1 baz:label3]
===

=== Entity kubernetes_metadata://apps/daemonsets/system/datadog-agent ===
== Source workloadmeta-kubernetes_metadata =
=Tags: [hash:27a53207eedd862383f608e9a6aa1341]
===

Checklist

  • PR has at least one valid label: bug, enhancement, refactoring, documentation, tooling, and/or dependencies
  • PR has a milestone or the qa/skip-qa label

@adel121 adel121 requested review from a team as code owners August 29, 2024 13:14
@adel121 adel121 added the enhancement New feature or request label Aug 29, 2024
@adel121 adel121 added this to the v1.10.0 milestone Aug 29, 2024
ok = false
}

return

Choose a reason for hiding this comment

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

🔵 Code Quality Violation

avoid bare return (...read more)

The "Avoid bare returns" rule in Go static analysis is designed to increase clarity and readability in your code. A bare return is when a function that has named return values returns those values implicitly, without explicitly stating what is being returned.

While Go's allowance for bare returns can make code more concise, it can also make it more difficult to understand and debug, especially in larger functions. Implicitly relying on the state of named return values can lead to unexpected behavior if those values are modified elsewhere in the function.

To adhere to this rule and promote better coding practices, always explicitly return values in your functions. This makes it clear what values are being returned and in what state, reducing the chance of bugs and making your code easier to understand. For example, instead of writing return in a function that returns an int and a bool, write return 0, false.

Learn More

View in Datadog  Leave us feedback  Documentation

@adel121
Copy link
Contributor Author

adel121 commented Aug 29, 2024

The new configuration options kubernetes_resources_labels_as_tags and kubernetes_resources_annotations_as_tags should ideally be replacing the following existing configs:

  • podLabelsAsTags
  • podAnnotationsAsTags
  • nodeAnnotationsAsTags
  • nodeLabelsAsTags
  • namespaceLabelsAsTags
  • namespaceAnnotationsAsTags

The intuition is that the new config is more generic, and it can be used to achieve the exact same functionality as the previous configs, but also extends to other types (e.g. daemonsets, statefulsets, etc.).

We can't drop support for the old config options, we should keep them in order not to break backward compatibility, but at the same time we should push users to use the new config.

What is the proper way to do this? Should we include comments in the code in the operator? or should we add a hint in the documentation?

@adel121 adel121 force-pushed the adelhajhassan/expose_kubernetes_labels_as_tags_and_annotations_as_tags branch from 66862d3 to d8c717b Compare August 29, 2024 13:26
@codecov-commenter
Copy link

codecov-commenter commented Aug 29, 2024

Codecov Report

Attention: Patch coverage is 55.68182% with 39 lines in your changes missing coverage. Please review.

Project coverage is 49.22%. Comparing base (fe0188f) to head (b602673).

Files with missing lines Patch % Lines
...nternal/controller/datadogagent/override/global.go 0.00% 26 Missing and 3 partials ⚠️
internal/controller/testutils/agent.go 0.00% 6 Missing ⚠️
...oller/datadogagent/component/clusteragent/utils.go 0.00% 2 Missing ⚠️
internal/controller/datadogagent/override/rbac.go 96.07% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1379      +/-   ##
==========================================
+ Coverage   49.19%   49.22%   +0.02%     
==========================================
  Files         223      224       +1     
  Lines       19612    19700      +88     
==========================================
+ Hits         9648     9697      +49     
- Misses       9461     9497      +36     
- Partials      503      506       +3     
Flag Coverage Δ
unittests 49.22% <55.68%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
api/datadoghq/v2alpha1/datadogagent_types.go 100.00% <ø> (ø)
...oller/datadogagent/component/clusteragent/utils.go 0.00% <0.00%> (ø)
internal/controller/datadogagent/override/rbac.go 96.07% <96.07%> (ø)
internal/controller/testutils/agent.go 0.00% <0.00%> (ø)
...nternal/controller/datadogagent/override/global.go 46.40% <0.00%> (-4.42%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe0188f...b602673. Read the comment docs.

Copy link

@aliciascott aliciascott left a comment

Choose a reason for hiding this comment

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

good for docs

@adel121 adel121 modified the milestones: v1.10.0, v1.9.0 Aug 29, 2024
@levan-m levan-m modified the milestones: v1.9.0, v1.10.0 Sep 3, 2024
…assan/expose_kubernetes_labels_as_tags_and_annotations_as_tags
@fanny-jiang
Copy link
Contributor

The new configuration options kubernetes_resources_labels_as_tags and kubernetes_resources_annotations_as_tags should ideally be replacing the following existing configs:

  • podLabelsAsTags
  • podAnnotationsAsTags
  • nodeAnnotationsAsTags
  • nodeLabelsAsTags
  • namespaceLabelsAsTags
  • namespaceAnnotationsAsTags

The intuition is that the new config is more generic, and it can be used to achieve the exact same functionality as the previous configs, but also extends to other types (e.g. daemonsets, statefulsets, etc.).

We can't drop support for the old config options, we should keep them in order not to break backward compatibility, but at the same time we should push users to use the new config.

What is the proper way to do this? Should we include comments in the code in the operator? or should we add a hint in the documentation?

We can update the type hints for those configs and add a note that encourages users to use the new config option. These notes will get autogenerated in the documentation. For example, the docs update would go in the datadogagent_types.go file here:

// Provide a mapping of Kubernetes Labels to Datadog Tags.
// <KUBERNETES_LABEL>: <DATADOG_TAG_KEY>

@@ -82,6 +82,11 @@ func GetExternalMetricsReaderClusterRoleName(dda metav1.Object, versionInfo *ver
return fmt.Sprintf("%s-metrics-reader", GetClusterAgentRbacResourcesName(dda))
}

// GetResourceMetadataAsTagsClusterRoleName returns the name for the cluster role name used for kubernetes resource labels and annotations as tags
func GetResourceMetadataAsTagsClusterRoleName(dda metav1.Object) string {
return fmt.Sprintf("%s-kubernetes-metadata-as-tags", GetClusterAgentRbacResourcesName(dda))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should use -annotations-and-labels-as-tags to stay consistent with what we're doing in the helm chart:

https://github.com/DataDog/helm-charts/blob/efa4d6629c240b545035bc80e05fbafaf83316c1/charts/datadog/templates/cluster-agent-rbac.yaml#L500

Suggested change
return fmt.Sprintf("%s-kubernetes-metadata-as-tags", GetClusterAgentRbacResourcesName(dda))
return fmt.Sprintf("%s-annotations-and-labels-as-tags", GetClusterAgentRbacResourcesName(dda))

@fanny-jiang fanny-jiang merged commit 26068ba into main Oct 10, 2024
19 checks passed
@fanny-jiang fanny-jiang deleted the adelhajhassan/expose_kubernetes_labels_as_tags_and_annotations_as_tags branch October 10, 2024 20:42
@adel121 adel121 changed the title expose kubernetes_resources_labels/annotations_as_tags [CONTP-376] expose kubernetes_resources_labels/annotations_as_tags Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants