Skip to content

Fix fluentbit service selector not using pod labels when defined #1575

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

solidDoWant
Copy link
Contributor

What this PR does / why we need it:

When spec.labels is defined, it is used to label created daemonset pods instead of metadata.labels. This causes the service, which always uses metadata.labels, to not select pods in some cases.

Unfortunately, because spec.labels is defined as map[string]string instead of a *map[string]string, it's not possible to tell the difference between a null spec.labels value (labels: ~, labels: null, or labels not set) and an empty spec.labels value (labels: {}). I think it's highly unlikely that anybody is doing this today, but if so, this could possibly be a breaking change.

Which issue(s) this PR fixes:

N/A

Does this PR introduced a user-facing change?

Fixed fluent bit service not always selecting pods when pod labels are set via `spec.labels`.

Additional documentation, usage docs, etc.:

N/A

@marcofranssen
Copy link
Collaborator

Any chance we can test this? e.g. unit or e2e test to prevent it breaking again in future?

@marcofranssen marcofranssen requested a review from Copilot May 2, 2025 15:03
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes an issue where the Fluent Bit service selector was not using pod labels defined in fb.Spec.Labels. The changes ensure that if fb.Spec.Labels is provided, it is used as the service selector instead of the default fb.Labels.

  • Introduced a new variable, podLabelSelector, to determine which label set to use.
  • Updated the service specification to utilize podLabelSelector for pod selection.

Comment on lines +32 to +34
podLabelSelector := fb.Labels
if len(fb.Spec.Labels) > 0 {
podLabelSelector = fb.Spec.Labels
Copy link
Preview

Copilot AI May 2, 2025

Choose a reason for hiding this comment

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

The current logic duplicates label assignment by using a separate variable podLabelSelector. Consider refactoring the label assignment to directly use fb.Spec.Labels when available, to improve clarity and maintainability.

Suggested change
podLabelSelector := fb.Labels
if len(fb.Spec.Labels) > 0 {
podLabelSelector = fb.Spec.Labels
podLabelSelector := fb.Spec.Labels
if len(podLabelSelector) == 0 {
podLabelSelector = fb.Labels

Copilot uses AI. Check for mistakes.

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.

2 participants