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

fix: Reduce indentation for servicemonitor endpoints #5571

Merged

Conversation

pauloconnor
Copy link
Contributor

Fixes #N/A

Description

The indentation for serviceMonitor endpoints was set too high, causing bad yaml to be rendered

apiVersion: monitoring.coreos.com/v1
kind: ServiceMonitor
metadata:
  name: karpenter
  namespace: foo
spec:
  namespaceSelector:
    matchNames:
      - foo
  selector:
    matchLabels:
      bar
  endpoints:
    - port: http-metrics
      path: /metrics
      - path: /metrics
        port: webhook-metrics

How was this change tested?

Does this change impact docs?

  • Yes, PR includes docs updates
  • Yes, issue opened: #
  • No

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@pauloconnor pauloconnor requested a review from a team as a code owner January 31, 2024 15:40
@pauloconnor pauloconnor requested a review from engedaam January 31, 2024 15:40
Copy link

netlify bot commented Jan 31, 2024

Deploy Preview for karpenter-docs-prod canceled.

Name Link
🔨 Latest commit 1580010
🔍 Latest deploy log https://app.netlify.com/sites/karpenter-docs-prod/deploys/65ba69ebbce3cd000985c3b3

@njtran
Copy link
Contributor

njtran commented Jan 31, 2024

Seems odd that the first item in the list was indented properly, but the second wasn't.

Can you post the installation command you used to set these endpoint configs?

@njtran
Copy link
Contributor

njtran commented Jan 31, 2024

Ah no you're totally right, I missed that

Copy link
Contributor

@njtran njtran left a comment

Choose a reason for hiding this comment

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

LGTM!

@njtran njtran enabled auto-merge (squash) February 7, 2024 01:28
@coveralls
Copy link

Pull Request Test Coverage Report for Build 7728576834

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 82.676%

Totals Coverage Status
Change from base Build 7719422505: 0.0%
Covered Lines: 5011
Relevant Lines: 6061

💛 - Coveralls

@njtran njtran merged commit 431a813 into aws:main Feb 7, 2024
16 checks passed
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