Add logmodule daemonset reconciler #3825

0sewa0 · 2024-09-20T07:05:12Z

Description

Adds the logmodule reconciler that will create a DaemonSet based on the spec.templates.logModule.

Has some minor adjustments for the configsecret reconciler:

How can this be tested?

What to configure in the `DynaKube`

spec.logModule.enabled: true
Because the final default image location is not known, we have an image in our private gcr, that GKE clusters can use without issues:

templates:
    logModule:
      imageRef:
        repository: us-central1-docker.pkg.dev/cloud-platform-207208/chmu/logmodule
        tag: latest

✨ <Whatever else you want to configure in the templates> ✨

What to configure in the tenant

Configure log-ingest rules:
- This will be done by the operator in the future. (upcoming ticket)
- My tenant has it at https://zib50933.dev.dynatracelabs.com/ui/settings/builtin:logmonitoring.log-storage-settings
- Press Add rule, this is a "catch-all" example:

What to check to see it "work"

If you see logs such as:


Process log content for /var/log/pods/kube-system_container-watcher-54sft_4a77a473-759c-4cf3-9db7-378637be0c30/container-watcher/#.log#


Successfully parsed 2 entries for /var/log/pods/kube-system_konnectivity-agent-8667cbffbd-sxwjk_da91698c-245c-4f49-9da5-725025937cc6/konnectivity-agent/#.log # and advanced file pointer by 306 after reading 612 bytes, current entry count: 2. Last file size: 515949, new position: 515643

It needs to be the communication endpoints of the oneagentm and not the api-url

The logmodule also relays on the oneagent connection-info

codecov-commenter · 2024-09-20T07:11:27Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

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

Project coverage is 65.52%. Comparing base (6c352e4) to head (266389a).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...rollers/dynakube/logmodule/daemonset/reconciler.go	82.45%	5 Missing and 5 partials ⚠️
pkg/util/kubeobjects/daemonset/builder.go	86.88%	6 Missing and 2 partials ⚠️
pkg/controllers/dynakube/controller.go	0.00%	2 Missing and 1 partial ⚠️
pkg/controllers/dynakube/logmodule/reconciler.go	50.00%	1 Missing and 1 partial ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3825      +/-   ##
==========================================
+ Coverage   65.06%   65.52%   +0.46%     
==========================================
  Files         397      405       +8     
  Lines       21414    21767     +353     
==========================================
+ Hits        13932    14262     +330     
- Misses       6143     6157      +14     
- Partials     1339     1348       +9

Flag	Coverage Δ
unittests	`65.52% <93.57%> (+0.46%)`	⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pkg/controllers/dynakube/logmodule/daemonset/container.go

waodim · 2024-09-20T07:47:25Z

pkg/api/v1beta3/dynakube/extensions.go

@@ -98,3 +98,15 @@ type ImageRefSpec struct {
 	// Indicates a tag of the image to use
 	Tag string `json:"tag,omitempty"`
 }
+
+func (ref ImageRefSpec) String(defaultRepo, defaultTag string) string {


do we really need to pass default repo and tag as parameters? Not that it matters a lot but it somehow looks weird because of the word "default"

ImageRefSpec is not only used by the LogModule, but I agree, there is a better solution

chrismuellner · 2024-09-20T07:49:22Z

pkg/api/v1beta3/dynakube/extensions.go

@@ -98,3 +98,15 @@ type ImageRefSpec struct {
 	// Indicates a tag of the image to use
 	Tag string `json:"tag,omitempty"`
 }
+
+func (ref ImageRefSpec) String(defaultRepo, defaultTag string) string {


why are you adding this to extensions?

I will move the ImageRefSpec elsewhere, it is not extensions specific

chrismuellner · 2024-09-20T07:50:27Z

pkg/controllers/dynakube/logmodule/configsecret/reconciler.go

@@ -18,7 +19,7 @@ import (

 const (
 	logModuleSecretSuffix    = "-logmodule-config"
-	deploymentConfigFilename = "deployment.conf"
+	DeploymentConfigFilename = "deployment.conf"


should this be moved to the config.go file of the pacakge?

chrismuellner · 2024-09-20T07:51:13Z

pkg/controllers/dynakube/logmodule/configsecret/reconciler.go

@@ -126,7 +127,7 @@ func (r *Reconciler) getSecretData(ctx context.Context) (map[string][]byte, erro
 	}

 	deploymentConfigContent := map[string]string{
-		serverKey:       r.dk.ApiUrl(),
+		serverKey:       fmt.Sprintf("{%s}", r.dk.Status.OneAgent.ConnectionInfoStatus.Endpoints),


can we move this to properties?

chrismuellner · 2024-09-20T07:53:55Z

pkg/controllers/dynakube/logmodule/daemonset/reconciler.go

+	return ds, nil
+}
+
+func GetName(dkName string) string {


isn't this the pod/daemonset name? just name makes is confusing when used in other places

we also have this in the properties for other components

we are in the daemonset package, that would be imported (probably with an alias) like lmdaemonset, then it would read lmdaemonset.GetName. So we avoid stuttering.

chrismuellner · 2024-09-20T07:56:56Z

pkg/controllers/dynakube/logmodule/daemonset/volumes.go

+	podLogsVolumePath       = "/var/log/pods"
+	dockerLogsVolumeName    = "var-lib-docker-containers"
+	dockerLogsVolumePath    = "/var/lib/docker/containers"
+	containerLogsVolumeName = "var-log-containers"


we could rename the volume name to logs-container or similar (same for others) as that's much more explicit than just reusing the path.

maybe more human readable version: container-logs ?

chrismuellner · 2024-09-20T08:00:57Z

pkg/controllers/dynakube/logmodule/daemonset/reconciler_test.go

+		require.NotNil(t, daemonset)
+
+		assert.Subset(t, daemonset.Spec.Template.Labels, customLabels)
+		// assert.Subset(t, daemonset.Labels, customLabels) TODO: Should this be the case?


should it? Checking the activegate, custom labels are only put on the pods

that is why I am asking :D

0sewa0 added 5 commits September 20, 2024 08:47

Fix Server propertie in deployment.conf

d4c057b

It needs to be the communication endpoints of the oneagentm and not the api-url

Add no-communication-hosts handling

d3a2cc5

The logmodule also relays on the oneagent connection-info

Add logmodule daemonset reconciler

bc994dc

Finalize volumes

9dbda0a

Add/update tests

266389a

0sewa0 added the logmodule label Sep 20, 2024

0sewa0 requested a review from a team as a code owner September 20, 2024 07:05

0sewa0 requested review from gkrenn, chrismuellner, albertogdd, waodim, StefanHauth, aorcholski and luhi-DT September 20, 2024 07:05

waodim reviewed Sep 20, 2024

View reviewed changes

chrismuellner requested changes Sep 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add logmodule daemonset reconciler #3825

Add logmodule daemonset reconciler #3825

0sewa0 commented Sep 20, 2024

codecov-commenter commented Sep 20, 2024

waodim Sep 20, 2024

0sewa0 Sep 20, 2024

chrismuellner Sep 20, 2024

0sewa0 Sep 20, 2024

chrismuellner Sep 20, 2024

chrismuellner Sep 20, 2024

chrismuellner Sep 20, 2024

0sewa0 Sep 20, 2024

chrismuellner Sep 20, 2024

0sewa0 Sep 20, 2024

chrismuellner Sep 20, 2024

0sewa0 Sep 20, 2024

Add logmodule daemonset reconciler #3825

Are you sure you want to change the base?

Add logmodule daemonset reconciler #3825

Conversation

0sewa0 commented Sep 20, 2024

Description

How can this be tested?

What to configure in the DynaKube

What to configure in the tenant

What to check to see it "work"

codecov-commenter commented Sep 20, 2024

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

What to configure in the `DynaKube`