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

feat: fix serviceMonitor for additional daemonsets #3166

Merged
merged 4 commits into from
Jul 28, 2023

Conversation

sumo-drosiek
Copy link
Contributor

Checklist

Match correct label:

        app.kubernetes.io/name: RELEASE-NAME-sumologic-otelcol-logs-collector-linux
        app.kubernetes.io/app-name: RELEASE-NAME-sumologic-otelcol-logs-collector
  • Changelog updated or skip changelog label added
  • Documentation updated
  • Template tests added for new features
  • Integration tests added or modified for major features

@sumo-drosiek sumo-drosiek requested a review from a team as a code owner July 26, 2023 10:11
@@ -649,7 +649,7 @@ sumologic:
- port: metrics
selector:
matchLabels:
sumologic.com/app: otelcol-logs-collector
sumologic.com/app-name: otelcol-logs-collector

Choose a reason for hiding this comment

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

Where does this label come from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

app.kubernetes.io/app-name: {{ (include "sumologic.labels.app.logs.collector.pod" $ctx) | trunc 63 | trimSuffix "-" }}

Choose a reason for hiding this comment

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

But these selectors are for the Service, not the Daemonset.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, I was trying figure out why e2e are failing 🤦

@sumo-drosiek sumo-drosiek force-pushed the drosiek-fix-serviceMonitor branch from 9a42a55 to 90745dd Compare July 27, 2023 06:48
Dominik Rosiek added 2 commits July 27, 2023 09:06
Signed-off-by: Dominik Rosiek <[email protected]>
@sumo-drosiek sumo-drosiek force-pushed the drosiek-fix-serviceMonitor branch from 90745dd to 093ba0e Compare July 27, 2023 07:06
@@ -1,136 +1,4 @@
---
# Source: sumologic/templates/events/common/service.yaml

Choose a reason for hiding this comment

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

Why are these resources removed from this file? Were they unused, and regenerating the file caused them to disappear?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think something is wrong and only one object is put into the generated file

Copy link
Contributor

@perk-sumo perk-sumo left a comment

Choose a reason for hiding this comment

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

LGTM 🛠️

@sumo-drosiek sumo-drosiek merged commit b9d433d into main Jul 28, 2023
@sumo-drosiek sumo-drosiek deleted the drosiek-fix-serviceMonitor branch July 28, 2023 09:02
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.

3 participants