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

[CECO-884] Add sidecar injection basic config in operator #1190

Closed
wants to merge 5 commits into from

Conversation

kisungyi92
Copy link
Collaborator

@kisungyi92 kisungyi92 commented May 16, 2024

What does this PR do?

Add Agent sidecar injection feature with basic config in fargate environment.
Also refactored admissionController feature test codes.

Motivation

CECO-884

Additional Notes

Extending agent sidecar injection from helm to operator.

Minimum Agent Versions

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

  • Agent: v7.53.0
  • Cluster Agent: v7.53.0

Describe your test plan

Test label overrides with < 1.7.0. (e.g. 1.6.0)

  • Create DDA with below mandatory setup :
    • features.admissionController.agentSidecarInjection.enabled to true.
    • features.admissionController.agentSidecarInjection to “fargate” as minimal setup.
spec: 
  features:
    admissionController:
      agentSidecarInjection:
        enabled: true
        provider: fargate
  • Once you deploy it, Check the environment variable is set below.
  • below value is default variables with minimal setup.
  • image name and image tag will follow the default agent image name and tag.
  • In operator 1.6.0, DDA 7.53.0 is default version tag.
      DD_ADMISSION_CONTROLLER_AGENT_SIDECAR_ENABLED:                true
      DD_ADMISSION_CONTROLLER_AGENT_SIDECAR_CLUSTER_AGENT_ENABLED:  true
      DD_ADMISSION_CONTROLLER_AGENT_SIDECAR_PROVIDER:               fargate
      DD_ADMISSION_CONTROLLER_AGENT_SIDECAR_IMAGE_NAME:             agent
      DD_ADMISSION_CONTROLLER_AGENT_SIDECAR_IMAGE_TAG:              7.53.0
  • Check the sidecar agent has correctly attached with applications.
  datadog-agent-injected:
    Container ID:   containerd://b30f4e833aa7bfa51a7c183ffed54b3a14f75f2632bd1aad6353fd207c5c17ea
    Image:          gcr.io/datadoghq/agent:7.53.0
  • Test with extra features (e.g. registry, imageName, imageTag) and See the values are set properly.
  features:
    admissionController:
      agentSidecarInjection:
        enabled: true
        provider: fargate
        registry: public.ecr.aws/datadog
        imageName: agent
        imageTag: 7.52.0
  • Check the result
      DD_ADMISSION_CONTROLLER_AGENT_SIDECAR_ENABLED:                true
      DD_ADMISSION_CONTROLLER_AGENT_SIDECAR_CLUSTER_AGENT_ENABLED:  true
      DD_ADMISSION_CONTROLLER_AGENT_SIDECAR_PROVIDER:               fargate
      DD_ADMISSION_CONTROLLER_AGENT_SIDECAR_CONTAINER_REGISTRY:     public.ecr.aws/datadog
      DD_ADMISSION_CONTROLLER_AGENT_SIDECAR_IMAGE_NAME:             agent
      DD_ADMISSION_CONTROLLER_AGENT_SIDECAR_IMAGE_TAG:              7.52.0
  • Check the agent images is pulling the right path, name and version.
  datadog-agent-injected:
    Container ID:   
    Image:          public.ecr.aws/datadog/agent:7.52.0
  • Lastly, Test with nodeAgent override image name and tag and it will override default image name and tag.
  • and Also see If there is value of them in admissionController, It will take precedence.

When only set override

  override:
    nodeAgent:
      image:
        name: NameFromOverride
        tag: TagFromOverride

Result : See the configuration from override.

      DD_ADMISSION_CONTROLLER_AGENT_SIDECAR_ENABLED:                true
      DD_ADMISSION_CONTROLLER_AGENT_SIDECAR_CLUSTER_AGENT_ENABLED:  true
      DD_ADMISSION_CONTROLLER_AGENT_SIDECAR_PROVIDER:               fargate
      DD_ADMISSION_CONTROLLER_AGENT_SIDECAR_CONTAINER_REGISTRY:     public.ecr.aws/datadog
      DD_ADMISSION_CONTROLLER_AGENT_SIDECAR_IMAGE_NAME:             NameFromOverride
      DD_ADMISSION_CONTROLLER_AGENT_SIDECAR_IMAGE_TAG:              TagFromOverride

When both set override and feature

  override:
    nodeAgent:
      image:
        name: NameFromOverride
        tag: TagFromOverride
...
  features:
    admissionController:
      agentSidecarInjection:
        enabled: true
        provider: fargate
        registry: public.ecr.aws/datadog
        imageName: NameFromFeature
        imageTag: TagFromFeature

Result : See the configuration from feature will take precedence.

      DD_ADMISSION_CONTROLLER_AGENT_SIDECAR_ENABLED:                true
      DD_ADMISSION_CONTROLLER_AGENT_SIDECAR_CLUSTER_AGENT_ENABLED:  true
      DD_ADMISSION_CONTROLLER_AGENT_SIDECAR_PROVIDER:               fargate
      DD_ADMISSION_CONTROLLER_AGENT_SIDECAR_CONTAINER_REGISTRY:     public.ecr.aws/datadog
      DD_ADMISSION_CONTROLLER_AGENT_SIDECAR_IMAGE_NAME:             NameFromFeature
      DD_ADMISSION_CONTROLLER_AGENT_SIDECAR_IMAGE_TAG:              TagFromFeature

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

@kisungyi92 kisungyi92 added the enhancement New feature or request label May 16, 2024
@kisungyi92 kisungyi92 added this to the v1.7.0 milestone May 16, 2024
@kisungyi92 kisungyi92 requested review from a team as code owners May 16, 2024 20:40
@kisungyi92 kisungyi92 changed the title Kisung/add sidecar injection basic config in operator. [CECO-884] Add sidecar injection basic config in operator May 16, 2024
@codecov-commenter
Copy link

codecov-commenter commented May 17, 2024

Codecov Report

Attention: Patch coverage is 88.73239% with 16 lines in your changes are missing coverage. Please review.

Project coverage is 59.74%. Comparing base (0cf699a) to head (90127ea).
Report is 19 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1190      +/-   ##
==========================================
+ Coverage   59.55%   59.74%   +0.18%     
==========================================
  Files         174      174              
  Lines       21559    21697     +138     
==========================================
+ Hits        12839    12962     +123     
- Misses       7941     7953      +12     
- Partials      779      782       +3     
Flag Coverage Δ
unittests 59.74% <88.73%> (+0.18%) ⬆️

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

Files Coverage Δ
apis/datadoghq/v2alpha1/datadogagent_default.go 92.05% <100.00%> (+0.78%) ⬆️
apis/datadoghq/v2alpha1/datadogagent_types.go 100.00% <ø> (ø)
...atadogagent/feature/admissioncontroller/feature.go 83.77% <93.20%> (+7.99%) ⬆️
...s/datadogagent/feature/admissioncontroller/rbac.go 89.65% <30.76%> (-10.35%) ⬇️

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 0cf699a...90127ea. Read the comment docs.

Name: "v2alpha1 Admission Controller enabled with sidecar injection enabled",
DDAv2: v2alpha1test.NewDatadogAgentBuilder().
WithAdmissionControllerEnabled(true).
WithSidecarInjectionSetup(v2alpha1.AgentSidecarInjectionConfig{
Copy link
Contributor

Choose a reason for hiding this comment

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

Functionally this looks correct but it's a bit confusing both setup (this line) and verification (line 141) taking identical structure.

@kisungyi92 kisungyi92 requested review from brett0000FF and levan-m May 28, 2024 13:44
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.

Last points discussed offline were addressed, so overall looks good to me!

@kisungyi92 kisungyi92 force-pushed the kisung/add_sidecar_injection_basic_config branch from 4eb4ef1 to 69f0f81 Compare May 29, 2024 16:07
@kisungyi92 kisungyi92 force-pushed the kisung/add_sidecar_injection_basic_config branch from 69f0f81 to 6718e64 Compare May 29, 2024 16:27
Adding extra config for sidecar injection

adding selector and profile structs

set default config for agent sidecar injection

edited minor code for feature.go

added nodeAgent override variables for image and tags

deleting unnecessary variables

changing some variables for clarification

editing some dda types

adding feature test

editing test code for sidecar injection

refactoring AC test code

editing some typos

Editing some AC feature and test code

Adding CRDs for image name and tag

Editing AC test codes

editing minor changes in builder

Editing some test logics

Editing some AC test cases

Changing default value of AgentSidecarInjectionConfig.Enabled to false

Editing some description for sidecar injection provider

Changing agentsidecarConfig to agentSidecarConfig in AC feature

Editing some default value and rolling back AC v1alpha1 test code

Refactoring AC test code with struct args

Editing some name convention in AC feature test

Editing some builder name convention and deleting unused default var

Editing sidecar Injection statement for clarification

rearranging test case for admission controller and agent default

rolling back the sidecar image to the original

Editing some sidecar image and tag logics

changing some parameter for sidecar injection

Adding back v1alpha1 AC test

Editing some values in AC test

Adding some dropped features during merge

Editing agent version in AC test

Deleting duplicated options and changing test order in AC.

Editing builder.go

Editing sidecar injection logic

Changing some image and tag login in sidecar feature

Changing comments on cws descriptions

nit changes in feature test description

Adding minimal sidecar injection logic

Adding extra config for sidecar injection

adding selector and profile structs

set default config for agent sidecar injection

edited minor code for feature.go

added nodeAgent override variables for image and tags

deleting unnecessary variables

changing some variables for clarification

editing some dda types

adding feature test

editing test code for sidecar injection

refactoring AC test code

editing some typos

Editing some AC feature and test code

Adding CRDs for image name and tag

Editing AC test codes

editing minor changes in builder

Editing some test logics

Editing some AC test cases

Changing default value of AgentSidecarInjectionConfig.Enabled to false

Editing some description for sidecar injection provider

Changing agentsidecarConfig to agentSidecarConfig in AC feature

Editing some default value and rolling back AC v1alpha1 test code

Refactoring AC test code with struct args

Editing some name convention in AC feature test

Editing some builder name convention and deleting unused default var

Editing sidecar Injection statement for clarification

rearranging test case for admission controller and agent default

rolling back the sidecar image to the original

Editing some sidecar image and tag logics

changing some parameter for sidecar injection

Adding back v1alpha1 AC test

Editing some values in AC test

Adding some dropped features during merge

Editing agent version in AC test

Deleting duplicated options and changing test order in AC.

Editing builder.go

Editing sidecar injection logic

Changing some image and tag login in sidecar feature

Changing comments on cws descriptions

nit changes in feature test description

Squashing commits

Adding config CRD
@kisungyi92 kisungyi92 force-pushed the kisung/add_sidecar_injection_basic_config branch from 6718e64 to 7de4e91 Compare May 29, 2024 16:55
@celenechang
Copy link
Contributor

closing in favor of #1207

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants