Skip to content

Conversation

andrefrco
Copy link

@andrefrco andrefrco commented Aug 3, 2025

Description:

This PR makes sure the app.kubernetes.io/version label on workloads (Deployment, DaemonSet, and StatefulSet) matches the tag from the image passed via the --collector-image, --target-allocator-image, or --operator-opamp-bridge-image CLI flags in cases where .spec.image isn’t set directly in the OpenTelemetryCollector, OpAMPBridge, or TargetAllocator CR.

Before this, if .spec.image wasn’t defined, the operator would fall back to setting "latest" as the label even though the actual image (like 0.129.1) came from the CLI flag. This caused the service.version resource attribute to be incorrect in the workloads.

Link to tracking Issue(s):

Testing:

Added 3 unit tests per workload type:

  1. When .spec.image is defined: The label reflects the tag from .spec.image
  2. When only the global operator config is defined: The label reflects the tag from the configured image in config.Config
  3. When neither .spec.image nor the global config is set (config.Config{}): The label defaults to "latest"

Fixed a DefaultTests that incorrectly expected "latest" when using config.New(). Since config.New() returns a config with a fallback value (e.g.) used for tests, the test was updated to reflect that version "0.0.0" instead of "latest".

I've also tested the change locally using minikube and confirmed that the app.kubernetes.io/version label is now correctly set on the generated workloads.

Documentation:

This is a bug fix, no doc changes required.

Design consideration:

I thought about changing the labels.Labels() function to accept a config.Config parameter, which would let us handle the version resolution in a single place. Something like:

func Labels(instance metav1.ObjectMeta, name string, image string, component string, config.Config) map[string]string

That way, we could reuse this logic across all components (services, serviceMonitors, etc) using the resourceImage and filterLabels together.

But I was a bit worried it might make the labels package more complex than it needs to be, since it would have to know about operator components and global config.

So for now, I went with a simpler approach and kept the logic outside. I'm new here and happy to improve this if there's a better way 🙂

- Fix version label logic in Collector, OpAMPBridge, and TargetAllocator
  manifests
- Add fallback logic: spec image > config image > latest
- Add comprehensive tests for all components (Collector, OpAMPBridge, TargetAllocator)
- Fix expectations for default config values (0.0.0 instead of latest)
Fixes open-telemetry#4175

Signed-off-by: andrefrco <[email protected]>

fix(manifests): fix app.kubernetes.io/version label logic
@andrefrco andrefrco requested a review from a team as a code owner August 3, 2025 01:39
Copy link
Contributor

github-actions bot commented Aug 4, 2025

E2E Test Results

 33 files  ±0  221 suites  ±0   4h 0m 33s ⏱️ +57s
 85 tests ±0   85 ✅ +1  0 💤 ±0  0 ❌  - 1 
225 runs  ±0  225 ✅ +1  0 💤 ±0  0 ❌  - 1 

Results for commit 2f79f65. ± Comparison against base commit 9597117.

♻️ This comment has been updated with latest results.

@andrefrco
Copy link
Author

I'll check the e2e tests failure asap

Replace len(image) == 0 with image == "" for better string validation in Collector, OpAMPBridge, and TargetAllocator deployment manifests.
- Add config parameter to updateCollectorStatus for consistent image resolution

- Remove hardcoded version assertion from kubeletstats e2e test
@andrefrco
Copy link
Author

I'll check the e2e tests failure asap

Fixed the e2e test by removing the hardcoded version assertion from kubeletstats test (to avoid version-specific checks). Also added the same image resolution logic to UpdateCollectorStatus for consistency since it creates labelSelectors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Collector pod label app.kubernetes.io/version is wrong when using global --collector-image option
2 participants