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

WIP Call otelc reconciler if telemetryService is enabled #4421

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

aorcholski
Copy link
Contributor

@aorcholski aorcholski commented Feb 6, 2025

JIRA

Description

How can this be tested?

unittests

@aorcholski aorcholski added the core Changes to core functionality of the Operator label Feb 6, 2025
@aorcholski aorcholski force-pushed the feature/telemetry-service-otelc branch 2 times, most recently from 2ead5ed to 18613b7 Compare February 6, 2025 13:48
@aorcholski aorcholski force-pushed the feature/telemetry-service-otelc branch 3 times, most recently from 33592ce to 9af89d8 Compare February 6, 2025 15:33
@codecov-commenter
Copy link

codecov-commenter commented Feb 6, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 93.89313% with 8 lines in your changes missing coverage. Please review.

Project coverage is 65.00%. Comparing base (671a7e5) to head (26b6386).

Files with missing lines Patch % Lines
pkg/controllers/dynakube/otelc/statefulset/env.go 91.07% 4 Missing and 1 partial ⚠️
...ntrollers/dynakube/otelc/statefulset/reconciler.go 72.72% 2 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    #4421      +/-   ##
==========================================
+ Coverage   64.92%   65.00%   +0.08%     
==========================================
  Files         413      413              
  Lines       27199    27276      +77     
==========================================
+ Hits        17658    17730      +72     
- Misses       8216     8220       +4     
- Partials     1325     1326       +1     
Flag Coverage Δ
unittests 65.00% <93.89%> (+0.08%) ⬆️

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.

@aorcholski aorcholski force-pushed the feature/telemetry-service-otelc branch 5 times, most recently from 17ba726 to 2a94cc6 Compare February 10, 2025 14:53
@aorcholski aorcholski force-pushed the feature/telemetry-service-otelc branch from 2a94cc6 to 26b6386 Compare February 11, 2025 16:37
return corev1.Container{
Name: containerName,
Image: imageRepo + ":" + imageTag,
ImagePullPolicy: corev1.PullAlways,
SecurityContext: buildSecurityContext(),
Env: getEnvs(dk),
Resources: dk.Spec.Templates.OpenTelemetryCollector.Resources,
Args: []string{fmt.Sprintf("--config=eec://%s:%d/otcconfig/prometheusMetrics#refresh-interval=5s&auth-file=%s", dk.ExtensionsServiceNameFQDN(), consts.OtelCollectorComPort, otelcSecretTokenFilePath)},
Args: []string{arg},
Copy link
Contributor

Choose a reason for hiding this comment

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

if I'm not mistaken we need to append not replace, so it's 1 or another or both.

MountPath: customEecTLSCertificatePath,
ReadOnly: true,
})
if dk.IsExtensionsEnabled() {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we combine this if with negative one, and put to else block?

@@ -121,5 +156,13 @@ func buildContainerVolumeMounts(dk *dynakube.DynaKube) []corev1.VolumeMount {
})
}

if !dk.IsExtensionsEnabled() {
Copy link
Contributor

Choose a reason for hiding this comment

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

see comment on line 143

SecretName: "os",
Items: []corev1.KeyToPath{
{
Key: "config.yaml",
Copy link
Contributor

Choose a reason for hiding this comment

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

use constatnts

@@ -91,12 +101,35 @@ func setVolumes(dk *dynakube.DynaKube) func(o *appsv1.StatefulSet) {
},
})
}

if !dk.IsExtensionsEnabled() {
Copy link
Contributor

Choose a reason for hiding this comment

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

same idea as before combine with line 67

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Changes to core functionality of the Operator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants