Skip to content

[platform] delete extra dependencies for piraeus operator - #856

Merged
klinch0 merged 1 commit into
mainfrom
(fix)--delete-extra-depends-for-piraeus-operator
May 7, 2025
Merged

[platform] delete extra dependencies for piraeus operator#856
klinch0 merged 1 commit into
mainfrom
(fix)--delete-extra-depends-for-piraeus-operator

Conversation

@klinch0

@klinch0 klinch0 commented Apr 23, 2025

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Chores
    • Updated dependency configuration so that piraeus-operator no longer depends on victoria-metrics-operator.
  • Refactor
    • Improved compatibility by ensuring certain resources (VMPodScrape and alert definitions) are only rendered if the required API versions are available in the Kubernetes cluster.

@coderabbitai

coderabbitai Bot commented Apr 23, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

The changes modify Helm chart templates and bundle configuration files to add conditional rendering based on the presence of specific Kubernetes API versions. The piraeus-operator release in the bundle configuration no longer depends on victoria-metrics-operator. Additionally, the rendering of the VMPodScrape resource and alert YAML files is now gated by checks for the relevant API versions, ensuring these resources are only created if the required APIs are available in the cluster.

Changes

File(s) Change Summary
packages/core/platform/bundles/distro-full.yaml Removed victoria-metrics-operator from the dependsOn list for the piraeus-operator release.
packages/system/linstor/templates/podscrape.yaml Wrapped the VMPodScrape resource definition in a conditional block checking for the required API version.
packages/system/piraeus-operator/templates/alerts.yaml Added a conditional block to render alert files only if the required API version is present.

Sequence Diagram(s)

sequenceDiagram
    participant Helm Chart
    participant Kubernetes Cluster

    Helm Chart->>Kubernetes Cluster: Check for API version "operator.victoriametrics.com/v1beta1"
    alt API version present
        Helm Chart->>Kubernetes Cluster: Render VMPodScrape resource
    else API version absent
        Helm Chart-->>Kubernetes Cluster: Skip VMPodScrape resource
    end

    Helm Chart->>Kubernetes Cluster: Check for API version "monitoring.coreos.com/v1"
    alt API version present
        Helm Chart->>Kubernetes Cluster: Render alert YAML files
    else API version absent
        Helm Chart-->>Kubernetes Cluster: Skip alert YAML files
    end
Loading

Suggested reviewers

  • kvaps
  • lllamnyp

Poem

In the warren where Helm charts grow,
A rabbit checked which APIs show.
If Victoria’s metrics aren’t in sight,
Or CoreOS monitoring’s not quite right,
The templates skip, they do not fail—
Just hopping smartly down the YAML trail!
🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e79ee94 and 3c27192.

📒 Files selected for processing (3)
  • packages/core/platform/bundles/distro-full.yaml (1 hunks)
  • packages/system/linstor/templates/podscrape.yaml (2 hunks)
  • packages/system/piraeus-operator/templates/alerts.yaml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/core/platform/bundles/distro-full.yaml
🧰 Additional context used
🪛 YAMLlint (1.35.1)
packages/system/linstor/templates/podscrape.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Build and Test
🔇 Additional comments (6)
packages/system/linstor/templates/podscrape.yaml (3)

1-2: Conditional wrapper for Victoria Metrics PodScrape
Gating the VMPodScrape resources behind .Capabilities.APIVersions.Has "operator.victoriametrics.com/v1beta1" correctly prevents invalid CRs in clusters without the Victoria Metrics Operator API.

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)


3-23: Resource definitions of linstor-satellite and linstor-controller
The two VMPodScrape specs remain unchanged and are valid when the API is present. All fields (endpoints, relabel configs, selectors) align with expected usage.


24-46: Closing Helm conditional block
The {{- end }} properly scopes the conditional. Ignore the YAMLlint “syntax error” on these templating directives—it’s a false positive from plain YAML linting Helm templates.

packages/system/piraeus-operator/templates/alerts.yaml (3)

1-3: Guard alert rendering by Monitoring API
Adding if .Capabilities.APIVersions.Has "monitoring.coreos.com/v1" ensures that alerts in alerts/*.yaml are only emitted in clusters supporting the monitoring.coreos.com/v1 API.

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)


4-7: YAML embedding via range remains correct
Iterating over $files and inlining each alert with toString correctly embeds multiple alert definitions under the conditional.


8-8: Proper closing of the conditional
The {{- end -}} matches the opening if, with whitespace trimming for clean output.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@NickVolynkin Nick Volynkin (NickVolynkin) changed the title [fix]: delete extra depends for piraeus operator [fix]: delete extra dependencies for piraeus operator Apr 23, 2025
@klinch0 klinch0 changed the title [fix]: delete extra dependencies for piraeus operator [platform]: delete extra dependencies for piraeus operator Apr 23, 2025
Comment thread packages/core/platform/bundles/distro-full.yaml
@kvaps Andrei Kvapil (kvaps) changed the title [platform]: delete extra dependencies for piraeus operator [platform] delete extra dependencies for piraeus operator Apr 23, 2025
@klinch0
klinch0 marked this pull request as draft April 24, 2025 06:02
@klinch0
klinch0 force-pushed the (fix)--delete-extra-depends-for-piraeus-operator branch from 1475335 to d6fb13f Compare May 5, 2025 06:03
@klinch0
klinch0 marked this pull request as ready for review May 5, 2025 06:03

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/system/piraeus-operator/alerts/piraeus-datastore.yaml (1)

2-2: Move document separator inside Helm conditional to avoid empty YAML docs
Placing the --- before the if will emit an empty document when the condition is false.

- ---
- {{- if .Capabilities.APIVersions.Has "monitoring.coreos.com/v1" }}
+ {{- if .Capabilities.APIVersions.Has "monitoring.coreos.com/v1" }}
+ ---
🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 2-2: syntax error: expected the node content, but found '-'

(syntax)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1475335 and d6fb13f.

📒 Files selected for processing (3)
  • packages/core/platform/bundles/distro-full.yaml (1 hunks)
  • packages/system/linstor/templates/podscrape.yaml (2 hunks)
  • packages/system/piraeus-operator/alerts/piraeus-datastore.yaml (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/core/platform/bundles/distro-full.yaml
🧰 Additional context used
🪛 YAMLlint (1.35.1)
packages/system/linstor/templates/podscrape.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

packages/system/piraeus-operator/alerts/piraeus-datastore.yaml

[error] 2-2: syntax error: expected the node content, but found '-'

(syntax)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Build and Test
🔇 Additional comments (3)
packages/system/piraeus-operator/alerts/piraeus-datastore.yaml (1)

118-118: Closing Helm conditional
The {{- end }} correctly closes the block.

packages/system/linstor/templates/podscrape.yaml (2)

1-1: Gate VMPodScrape resources on API version availability
Using .Capabilities.APIVersions.Has "operator.victoriametrics.com/v1beta1" is the correct approach to conditionally render these resources.

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)


46-46: Closing Helm conditional
The {{- end }} correctly terminates the conditional block.

@klinch0
klinch0 force-pushed the (fix)--delete-extra-depends-for-piraeus-operator branch from d6fb13f to e9c5d14 Compare May 5, 2025 06:34

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
packages/system/piraeus-operator/alerts/piraeus-datastore.yaml (1)

2-118: Wrap PrometheusRule in Helm conditional and consider adjusting document separator
The new if .Capabilities.APIVersions.Has "monitoring.coreos.com/v1" guard ensures the PrometheusRule is only rendered when supported. To prevent emitting an empty YAML document when the condition is false, consider moving the leading --- inside the conditional block. Also, ignore the YAMLlint syntax error caused by the Helm whitespace control.

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 2-2: syntax error: expected the node content, but found '-'

(syntax)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d6fb13f and e9c5d14.

📒 Files selected for processing (4)
  • packages/core/platform/bundles/distro-full.yaml (1 hunks)
  • packages/system/linstor/templates/podscrape.yaml (2 hunks)
  • packages/system/piraeus-operator/alerts/piraeus-datastore.yaml (2 hunks)
  • packages/system/piraeus-operator/templates/alerts.yaml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/core/platform/bundles/distro-full.yaml
🧰 Additional context used
🪛 YAMLlint (1.35.1)
packages/system/piraeus-operator/templates/alerts.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

packages/system/linstor/templates/podscrape.yaml

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

packages/system/piraeus-operator/alerts/piraeus-datastore.yaml

[error] 2-2: syntax error: expected the node content, but found '-'

(syntax)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Build and Test
🔇 Additional comments (2)
packages/system/piraeus-operator/templates/alerts.yaml (1)

1-9: Approve conditional gating and ignore YAMLlint warning
The Helm whitespace control {{- ... }} is correctly used to strip whitespace, and this block appropriately gates the inclusion of alert YAML snippets to clusters with the monitoring.coreos.com/v1 API. The YAMLlint syntax error is a false positive in Helm templates.

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

packages/system/linstor/templates/podscrape.yaml (1)

1-46: Approve conditional gating and ignore YAMLlint warning
Wrapping both VMPodScrape resources inside {{- if .Capabilities.APIVersions.Has "operator.victoriametrics.com/v1beta1" }} correctly prevents rendering in clusters lacking that API. The Helm whitespace control is valid; the YAMLlint syntax error can be safely ignored.

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 1-1: syntax error: expected the node content, but found '-'

(syntax)

@klinch0
klinch0 force-pushed the (fix)--delete-extra-depends-for-piraeus-operator branch from e9c5d14 to e79ee94 Compare May 5, 2025 09:31
Signed-off-by: kklinch0 <kklinch0@gmail.com>
@klinch0
klinch0 force-pushed the (fix)--delete-extra-depends-for-piraeus-operator branch from e79ee94 to 3c27192 Compare May 5, 2025 13:56

@kvaps Andrei Kvapil (kvaps) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@klinch0
klinch0 merged commit 29b4949 into main May 7, 2025
@klinch0
klinch0 deleted the (fix)--delete-extra-depends-for-piraeus-operator branch May 7, 2025 09:30
Timofei Larkin (lllamnyp) pushed a commit that referenced this pull request May 20, 2025
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Chores**
- Updated dependency configuration so that piraeus-operator no longer
depends on victoria-metrics-operator.
- **Refactor**
- Improved compatibility by ensuring certain resources (VMPodScrape and
alert definitions) are only rendered if the required API versions are
available in the Kubernetes cluster.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

(cherry picked from commit 29b4949)
Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
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