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 e2e tests for the remote config updater #1243

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

Conversation

mftoure
Copy link
Contributor

@mftoure mftoure commented Jun 21, 2024

What does this PR do?

This PR adds the e2e tests for the remote config updater

  • Deploy the operator
  • Deploy dda with features disabled
  • check via redapl that features are disabled
  • create config targeting the cluster name
  • check that the selected features are enabled
  • delete the config

A brief description of the change being made with this pull request.

Motivation

What inspired you to submit this pull request?

Additional Notes

Anything else we should know when reviewing?

Minimum Agent Versions

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

  • Agent: vX.Y.Z
  • Cluster Agent: vX.Y.Z

Describe your test plan

Write there any instructions and details you may have to test your PR.

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

@mftoure mftoure requested review from a team as code owners June 21, 2024 14:23
@mftoure mftoure marked this pull request as draft June 21, 2024 14:24
@codecov-commenter
Copy link

codecov-commenter commented Jun 21, 2024

Codecov Report

Attention: Patch coverage is 0% with 23 lines in your changes missing coverage. Please review.

Project coverage is 51.37%. Comparing base (0406868) to head (956a25b).
Report is 17 commits behind head on main.

Files with missing lines Patch % Lines
pkg/remoteconfig/updater.go 0.00% 23 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1243      +/-   ##
==========================================
+ Coverage   47.14%   51.37%   +4.22%     
==========================================
  Files         217      377     +160     
  Lines       18768    33608   +14840     
==========================================
+ Hits         8849    17265    +8416     
- Misses       9457    15447    +5990     
- Partials      462      896     +434     
Flag Coverage Δ
unittests 51.37% <0.00%> (+4.22%) ⬆️

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

Files with missing lines Coverage Δ
pkg/remoteconfig/updater.go 0.00% <0.00%> (ø)

... and 167 files with indirect coverage changes


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 0406868...956a25b. Read the comment docs.

Base automatically changed from fanny/CECO-934/e2e-kind to main June 26, 2024 18:19
func TestUpdaterSuite(t *testing.T) {

e2eParams := []e2e.SuiteOption{
e2e.WithStackName(fmt.Sprintf("operator-kind-2%s", k8sVersion)),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
e2e.WithStackName(fmt.Sprintf("operator-kind-2%s", k8sVersion)),
e2e.WithStackName(fmt.Sprintf("operator-kind-rc-%s", k8sVersion)),

Can we specify rc for remote config in the stack name?

e2e.WithDevMode(),
}

// testID := uuid.NewString()[:4]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this line still needed?

Suggested change
// testID := uuid.NewString()[:4]

.gitignore Outdated
@@ -100,3 +100,4 @@ tags
.idea/
# test errors
errors
go.work*
Copy link
Contributor

Choose a reason for hiding this comment

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

This is no longer needed in recent updates in main

@@ -29,6 +29,7 @@ spec:
args:
- --enable-leader-election
- --pprof
- --remoteConfigEnabled
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be better to have this rc-specific e2e-manager.yaml as a separate file in config/manager. I'll make an update in a separate PR to support this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Adding support here: #1294

@levan-m
Copy link
Contributor

levan-m commented Sep 7, 2024

I added a commit to resolve an issue with kustomize.

Problem was not a namespace deletion before running test, rather it was kustomize failing to create resources due to below error:

error: an unhandled error occurred: waiting for RPCs: kustomize invoke failed: rpc error: code = Unknown desc = invocation of kubernetes:kustomize:directory returned an error: kustomize failed for directory "./config/e2e": accumulating resources: accumulation err='merging resources from 'rc-e2e-manager.yaml': may not add resource with an already registered id: Namespace.v1.[noGrp]/system.[noNs]': must build at directory: './config/e2e/rc-e2e-manager.yaml': file is not directory

This commits changes how kustomize resources are overlayed. RC-specific configs are added as patch while e2e-manager.yaml acts as base config.

Tests now run fine but there is now a failure in the RC

Error Trace: /go/src/github.com/DataDog/datadog-operator/test/e2e/rc_updater_test.go:119
Error: Received unexpected error: failed to create config, status code: 403, response: {"errors":["Forbidden","Failed permission authorization checks"]}
Test: TestUpdaterSuite/TestEnableFeatures
Messages: Failed to apply config

@mftoure mftoure marked this pull request as ready for review September 16, 2024 13:37
Copy link
Contributor

@jhgilbert jhgilbert left a comment

Choose a reason for hiding this comment

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

Approved with minor suggestions, thank you!

docs/how-to-contribute.md Outdated Show resolved Hide resolved
docs/how-to-contribute.md Outdated Show resolved Hide resolved
docs/how-to-contribute.md Outdated Show resolved Hide resolved
docs/how-to-contribute.md Outdated Show resolved Hide resolved
docs/how-to-contribute.md Outdated Show resolved Hide resolved
mftoure and others added 2 commits September 18, 2024 13:53
Co-authored-by: Jen Gilbert <j.h.gilbert@gmail.com>
Co-authored-by: Jen Gilbert <j.h.gilbert@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants