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

The annotations-filter startup parameter is not used for configmap, rbac, service, serviceaccount resources. #3151

Closed
chinaran opened this issue Jul 22, 2024 · 2 comments · Fixed by #3163
Labels
area:collector Issues for deploying collector enhancement New feature or request question Further information is requested

Comments

@chinaran
Copy link
Contributor

Component(s)

collector

What happened?

Description

annotations-filter has a default configuration that filters kubectl.kubernetes.io/last-applied-configuration.

Steps to Reproduce

deploy a otel-collector instance.

Expected Result

None of the resources installed by opentelemetry-operator have kubectl.kubernetes.io/last-applied-configuration annotations.

Actual Result

The configmap, rbac, service, serviceaccount resources are annotated kubectl.kubernetes.io/last-applied-configuration.

Kubernetes Version

v1.29.2

Operator version

v0.104.0

Collector version

v0.104.0

Environment information

Environment

OS: MacOS Kind

Log output

No response

Additional context

I found that the hpa resource filters the kubectl.kubernetes.io/last-applied-configuration annotation.
Reference: https://github.com/open-telemetry/opentelemetry-operator/blob/v0.104.0/internal/manifests/collector/horizontalpodautoscaler.go# L31.

annotations, err := manifestutils.Annotations(params.OtelCol, params.Config.AnnotationsFilter())
if err != nil {
	return nil, err
}

When I try to fix this issue using similar code, I have the following two queries:

  1. whether Prometheus annotations should be added only in PodAnnotations.

  2. Should the opentelemetry-operator-config/sha256 annotation be added to all resources.

// Annotations return the annotations for OpenTelemetryCollector pod.
func Annotations(instance v1beta1.OpenTelemetryCollector, filterAnnotations []string) (map[string]string, error) {
	// new map every time, so that we don't touch the instance's annotations
	annotations := map[string]string{}

	// Enable Prometheus annotations by default if DisablePrometheusAnnotations is nil or true
	if !instance.Spec.Observability.Metrics.DisablePrometheusAnnotations {
		// Set default Prometheus annotations
		annotations["prometheus.io/scrape"] = "true"
		annotations["prometheus.io/port"] = "8888"
		annotations["prometheus.io/path"] = "/metrics"
	}

	// allow override of prometheus annotations
	if nil != instance.ObjectMeta.Annotations {
		for k, v := range instance.ObjectMeta.Annotations {
			if !IsFilteredSet(k, filterAnnotations) {
				annotations[k] = v
			}
		}
	}

	hash, err := GetConfigMapSHA(instance.Spec.Config)
	if err != nil {
		return nil, err
	}

	// make sure sha256 for configMap is always calculated
	annotations["opentelemetry-operator-config/sha256"] = hash

	return annotations, nil
}
@chinaran chinaran added bug Something isn't working needs triage labels Jul 22, 2024
@jaronoff97 jaronoff97 added enhancement New feature or request question Further information is requested area:collector Issues for deploying collector and removed bug Something isn't working needs triage labels Jul 24, 2024
@jaronoff97
Copy link
Contributor

@chinaran thanks for opening this! To answer your questions:

  1. I think only pod annotations needs the prometheus annotation filtering code you referenced
  2. I do think there's value in having the config hash for each generated resource, it allows us to restart the collector pod when a config change has been made (see the original issue here)

@chinaran
Copy link
Contributor Author

@jaronoff97 Thank you for your answer.
Based on these two conclusions, I submitted a PR: #3163.
Please check it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:collector Issues for deploying collector enhancement New feature or request question Further information is requested
Projects
None yet
2 participants