[linstor] Add linstor-scheduler package #1824
Conversation
📝 WalkthroughWalkthroughAdds a new linstor-scheduler component: package source, bundle entries, Makefile/chart packaging, and a complete Helm chart (templates, values, docs, and licensing) including deployment, admission webhook, RBAC, autoscaling, and tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant KubeAPI as "Kubernetes API"
participant Scheduler as "kube-scheduler (wrapped)"
participant Extender as "linstor-scheduler-extender"
participant Admission as "linstor-scheduler-admission"
participant CertMgr as "cert-manager"
Note over CertMgr,Admission `#DDEEFF`: Chart installs cert-manager resources when admission.enabled
CertMgr->>Admission: provision TLS secret (CA + cert)
KubeAPI->>Scheduler: Pod create request
Scheduler->>Extender: extenders HTTP filter/prioritize (localhost:8099)
Extender->>KubeAPI: query Node/PV/PVC (if needed) & consult LINSTOR controller
KubeAPI->>Admission: send Pod to admission webhook (mutate)
Admission->>KubeAPI: response (mutated pod with schedulerName / annotations)
Note right of Scheduler: Final scheduling decision uses extender response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
Summary of ChangesHello @kvaps, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces the 'linstor-scheduler' package, a crucial enhancement for Kubernetes environments leveraging LINSTOR storage. It provides a specialized scheduling mechanism designed to improve performance and resource utilization by ensuring that pods requiring LINSTOR-backed persistent volumes are intelligently placed on nodes that already host local replicas of the necessary data. This is achieved through a combination of a custom scheduler extender and an admission webhook, which together automate the optimal placement of LINSTOR-dependent workloads. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces the linstor-scheduler package to enable optimal pod placement for nodes with LINSTOR storage. The changes include adding the package, its source definition, and integrating it into the distro-full and paas-full bundles.
While the implementation is mostly sound, there is a critical concern: the upstream linstor-scheduler Helm chart, which is being vendored, is marked as deprecated and no longer maintained. The chart's README.md explicitly recommends using volumeBindingMode: WaitForFirstConsumer instead. Adding a deprecated component introduces significant risk for future maintenance and compatibility. It is highly recommended to reconsider this approach and evaluate the alternative suggested by the upstream project. If there's a strong reason to proceed, it should be clearly documented in the pull request description.
Additionally, I've found a few issues in the vendored chart templates that should be addressed:
- An incorrect chart name in the package's
Chart.yaml. - An excessively long
httpTimeoutin the scheduler configuration. - A duplicated
imagePullSecretsblock in the main deployment template. - A typo in the
README.mdconfiguration table.
Please review the detailed comments for suggestions on how to fix these issues.
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/config.yaml
Outdated
Show resolved
Hide resolved
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/deployment.yaml
Outdated
Show resolved
Hide resolved
| | `serviceAccount.create` | Create the service account resource | `true` | | ||
| | `serviceAccount.name` | Sets the name of the service account. If left empty, will use the release name as default | `""` | | ||
| | `podDisruptionBudget.enabled` | Enable creation of a pod disruption budget to protect the availability of the scheduler | `true` | | ||
| | `autoscaling.enabled` | Enable creation of a horizontal pod autoscaler to ensure availability in case of high usage` | `"false` | |
There was a problem hiding this comment.
There's a typo in the configuration table for autoscaling.enabled. The description has a stray backtick, and the default value is malformed. This should be corrected for clarity.
| | `autoscaling.enabled` | Enable creation of a horizontal pod autoscaler to ensure availability in case of high usage` | `"false` | | |
| | `autoscaling.enabled` | Enable creation of a horizontal pod autoscaler to ensure availability in case of high usage | `false` | |
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In @packages/system/linstor-scheduler/Chart.yaml:
- Line 2: The Chart.yaml name is inconsistent: it is set to "cozy-linstor" but
bundle references expect "cozy-linstor-scheduler"; update the Chart.yaml name
field to "cozy-linstor-scheduler" (the chart's name property) so it matches
package/bundle references, then run the provided verification script (or ripgrep
for "cozy-linstor" and "linstor-scheduler" in packages/core/platform/bundles/)
to confirm no remaining mismatches and update any bundle references if needed.
In @packages/system/linstor-scheduler/charts/linstor-scheduler/README.md:
- Line 72: The table row for the Helm value `autoscaling.enabled` in the chart
README has a stray backtick in the description and a malformed default value;
update the description to remove the trailing backtick and fix the default
column to use a valid YAML/boolean string (e.g., "false" or false) so the row
reads correctly and the default is properly formatted; locate the table entry
containing `autoscaling.enabled` and correct both the description text and the
default value cell.
- Line 61: Fix the spelling mistake in the README table entry for
scheduler.securityContext by changing "capabilties" to "capabilities" in the
description string for scheduler.securityContext so the default securityContext
shows the correct key name (`capabilities: {drop: [ALL]}`) while keeping the
rest of the default value unchanged.
- Around line 23-26: Update the fenced code block in the README.md to include a
language identifier for syntax highlighting: change the opening triple backticks
to specify "bash" so the helm commands are highlighted correctly (locate the
fenced block containing the helm repo add / helm install commands and replace
``` with ```bash).
- Line 55: Fix the spelling in the README table for the extender.securityContext
entry: change "capabilties" to "capabilities" in the default value
`{capabilities: {drop: [ALL]}, readOnlyRootFilesystem: true, runAsNonRoot: true,
runAsUser: 1000}` so the configuration key reads `capabilities` for
`extender.securityContext`.
In
@packages/system/linstor-scheduler/charts/linstor-scheduler/templates/deployment.yaml:
- Line 109: The semver check on that line uses .Capabilities.KubeVersion.Version
directly which is inconsistent with the rest of the chart; change the condition
to use the existing helper by replacing .Capabilities.KubeVersion.Version with
the helper invocation (e.g. {{- if semverCompare ">= 1.22-0" (include
"linstor-scheduler.kubeVersion" .) }}), so the volume rendering uses the
configured linstor-scheduler.kubeVersion value just like the checks at lines
referencing the linstor-scheduler.kubeVersion helper.
- Around line 115-118: The deployment template contains a duplicated
imagePullSecrets block using .Values.imagePullSecrets which will cause
Kubernetes validation to fail; remove the second/duplicate imagePullSecrets
block (the one rendering {{- with .Values.imagePullSecrets }} ... {{- end }}) so
only the original imagePullSecrets declaration remains, or consolidate into a
single imagePullSecrets rendering that sits at the intended spec.template.spec
location and uses {{- toYaml . | nindent 8 }} with .Values.imagePullSecrets.
In
@packages/system/linstor-scheduler/charts/linstor-scheduler/templates/poddisruptionbudget.yaml:
- Around line 12-17: The podDisruptionBudget template renders both minAvailable
and maxUnavailable which are mutually exclusive; change the template to render
only one by using an if/else-if: if .Values.podDisruptionBudget.minAvailable is
set, emit minAvailable and skip maxUnavailable, else-if
.Values.podDisruptionBudget.maxUnavailable emit maxUnavailable; also update the
podDisruptionBudget section in values.yaml (and its comments) to document that
only one of minAvailable or maxUnavailable should be provided.
- Around line 9-11: The PDB selector is using the full labels helper; change the
selector to use the chart's selector-only labels helper so it matches pods.
Replace the use of the "linstor-scheduler.labels" include in
selector.matchLabels with the selector-only helper (e.g.,
"linstor-scheduler.selectorLabels" or the chart's selector helper) so only
app.kubernetes.io/name and app.kubernetes.io/instance are emitted for
selector.matchLabels.
In
@packages/system/linstor-scheduler/charts/linstor-scheduler/templates/tests/test-scheduler.yaml:
- Around line 12-15: The test container "wget" in
templates/tests/test-scheduler.yaml has an empty command (command: []), which
causes busybox to exit immediately; update the manifest to provide a simple,
successful command for the smoke test (e.g., use the wget container name) such
as running a no-op or short sleep that returns success (for example change
command to run ["sh","-c","true"] or ["sh","-c","sleep 1 && exit 0"]) so the pod
stays alive long enough for the scheduler test to succeed.
🧹 Nitpick comments (5)
packages/core/platform/bundles/distro-full.yaml (1)
360-365: Consider whetherlinstor-schedulershould be marked asoptional.The release is added without the
optional: trueflag. Since this scheduler is an optimization for pod placement (preferring nodes with local LINSTOR replicas) rather than a functional requirement, you may want to consider whether it should be optional in the distro-full bundle.This would allow deployments that don't need the scheduling optimization to skip it, similar to how other non-essential operators (e.g.,
etcd-operator,grafana-operator) are marked optional in this bundle.packages/system/linstor-scheduler/charts/linstor-scheduler/templates/hpa.yaml (1)
15-31: Potential issue: emptymetricslist if no utilization targets are configured.If
autoscaling.enabledistruebut neithertargetCPUUtilizationPercentagenortargetMemoryUtilizationPercentageis set, themetrics:field will be rendered with an empty list, which may cause validation errors or unexpected HPA behavior.Consider either:
- Adding a default CPU target in the values
- Adding a conditional check to render the entire
metricsblock only when at least one target is defined♻️ Option: Wrap metrics block conditionally
spec: scaleTargetRef: apiVersion: apps/v1 kind: Deployment name: {{ include "linstor-scheduler.fullname" . }} minReplicas: {{ .Values.autoscaling.minReplicas }} maxReplicas: {{ .Values.autoscaling.maxReplicas }} + {{- if or .Values.autoscaling.targetCPUUtilizationPercentage .Values.autoscaling.targetMemoryUtilizationPercentage }} metrics: {{- if .Values.autoscaling.targetCPUUtilizationPercentage }} - type: Resource resource: name: cpu target: type: Utilization averageUtilization: {{ .Values.autoscaling.targetCPUUtilizationPercentage }} {{- end }} {{- if .Values.autoscaling.targetMemoryUtilizationPercentage }} - type: Resource resource: name: memory target: type: Utilization averageUtilization: {{ .Values.autoscaling.targetMemoryUtilizationPercentage }} {{- end }} + {{- end }} {{- end }}Note: The static analysis YAML syntax error is a false positive caused by Helm templating directives.
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/certmanager.yaml (1)
46-49: Consider increasing the renewal buffer time.The
renewBefore: 24hsetting provides only 24 hours for cert-manager to renew and propagate the certificate before expiry. Industry best practice recommends a longer renewal window (e.g., 30 days or at least 7 days) to account for potential cert-manager downtime, API rate limits, or deployment issues.♻️ Proposed adjustment
secretName: {{ include "linstor-scheduler.fullname" . }}-admission-tls duration: 8760h # 1 year - renewBefore: 24h + renewBefore: 720h # 30 days issuerRef: name: {{ include "linstor-scheduler.fullname" . }}-admission-ca-issuerpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/admission-deployment.yaml (1)
36-54: Consider adding health probes for the admission webhook.The admission webhook container lacks liveness and readiness probes. Without probes, Kubernetes cannot detect if the webhook is unhealthy, potentially routing requests to unresponsive pods and causing pod creation failures or delays.
♻️ Suggested probes
ports: - containerPort: 8080 name: https protocol: TCP + livenessProbe: + httpGet: + path: /healthz + port: 8080 + scheme: HTTPS + initialDelaySeconds: 5 + periodSeconds: 10 + readinessProbe: + httpGet: + path: /healthz + port: 8080 + scheme: HTTPS + initialDelaySeconds: 5 + periodSeconds: 5 env:Note: Verify the actual health endpoint path supported by
linstor-scheduler-admission. Adjust the path if the application uses a different endpoint.packages/system/linstor-scheduler/charts/linstor-scheduler/templates/_helpers.tpl (1)
77-93: Silent failure when multipleLinstorControllerCRDs exist.When multiple
LinstorControllerresources exist in the namespace (len $crs != 1), the helper returns an empty string silently. This could lead to confusing deployment failures where TLS is unexpectedly disabled or the endpoint fails to resolve.💡 Consider adding a warning or fail for ambiguous CRD state
{{- if gt (len $crs) 1 }} {{- fail "Multiple LinstorController resources found. Please specify linstor.clientSecret explicitly." }} {{- else if eq (len $crs) 1 }} {{- $item := index $crs 0 }} {{- $item.spec.linstorHttpsClientSecret }} {{- end }}This would provide clearer feedback when the auto-discovery cannot determine which controller to use.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
packages/core/platform/bundles/distro-full.yamlpackages/core/platform/bundles/paas-full.yamlpackages/core/platform/sources/linstor-scheduler.yamlpackages/system/linstor-scheduler/.helmignorepackages/system/linstor-scheduler/Chart.yamlpackages/system/linstor-scheduler/Makefilepackages/system/linstor-scheduler/charts/linstor-scheduler/Chart.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/LICENSEpackages/system/linstor-scheduler/charts/linstor-scheduler/README.mdpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/NOTES.txtpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/_helpers.tplpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/admission-deployment.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/admission-service.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/certmanager.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/config.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/deployment.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/hpa.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/mutatingwebhookconfiguration.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/poddisruptionbudget.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/rbac.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/serviceaccount.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/tests/test-scheduler.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/values.yamlpackages/system/linstor-scheduler/values.yaml
🧰 Additional context used
📓 Path-based instructions (4)
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Git commits should follow
[component] Descriptionformat and include--signoffflag
Files:
packages/system/linstor-scheduler/charts/linstor-scheduler/README.md
packages/system/**
📄 CodeRabbit inference engine (AGENTS.md)
Use Helm Chart umbrella pattern with vendored upstream charts in
charts/directory
Files:
packages/system/linstor-scheduler/charts/linstor-scheduler/README.mdpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/NOTES.txtpackages/system/linstor-scheduler/values.yamlpackages/system/linstor-scheduler/Makefilepackages/system/linstor-scheduler/charts/linstor-scheduler/templates/certmanager.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/config.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/tests/test-scheduler.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/values.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/Chart.yamlpackages/system/linstor-scheduler/Chart.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/LICENSEpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/hpa.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/_helpers.tplpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/mutatingwebhookconfiguration.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/poddisruptionbudget.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/admission-deployment.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/rbac.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/deployment.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/admission-service.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/serviceaccount.yaml
packages/core/**
📄 CodeRabbit inference engine (AGENTS.md)
Use Helm Chart umbrella pattern with vendored upstream charts in
charts/directory
Files:
packages/core/platform/bundles/distro-full.yamlpackages/core/platform/bundles/paas-full.yamlpackages/core/platform/sources/linstor-scheduler.yaml
**/*.yaml
📄 CodeRabbit inference engine (AGENTS.md)
Do NOT directly edit upstream Helm charts in vendored charts directory; use proper chart vendoring mechanisms
Files:
packages/core/platform/bundles/distro-full.yamlpackages/system/linstor-scheduler/values.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/certmanager.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/config.yamlpackages/core/platform/bundles/paas-full.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/tests/test-scheduler.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/values.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/Chart.yamlpackages/system/linstor-scheduler/Chart.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/hpa.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/mutatingwebhookconfiguration.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/poddisruptionbudget.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/admission-deployment.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/rbac.yamlpackages/core/platform/sources/linstor-scheduler.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/deployment.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/admission-service.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/serviceaccount.yaml
🧠 Learnings (6)
📚 Learning: 2025-07-23T09:15:09.658Z
Learnt from: lllamnyp
Repo: cozystack/cozystack PR: 1233
File: packages/system/hetzner-robotlb/charts/robotlb/templates/deployment.yaml:33-35
Timestamp: 2025-07-23T09:15:09.658Z
Learning: The `./charts/robotlb/` directory in the hetzner-robotlb package contains vendored code, and the team generally avoids modifying vendored code to maintain clean separation from upstream dependencies.
Applied to files:
packages/system/linstor-scheduler/.helmignorepackages/system/linstor-scheduler/Makefilepackages/system/linstor-scheduler/charts/linstor-scheduler/Chart.yaml
📚 Learning: 2025-12-25T09:45:26.524Z
Learnt from: CR
Repo: cozystack/cozystack PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-25T09:45:26.524Z
Learning: Applies to packages/system/** : Use Helm Chart umbrella pattern with vendored upstream charts in `charts/` directory
Applied to files:
packages/system/linstor-scheduler/Makefilepackages/system/linstor-scheduler/Chart.yaml
📚 Learning: 2025-12-25T09:45:26.524Z
Learnt from: CR
Repo: cozystack/cozystack PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-25T09:45:26.524Z
Learning: Applies to packages/core/** : Use Helm Chart umbrella pattern with vendored upstream charts in `charts/` directory
Applied to files:
packages/system/linstor-scheduler/Makefilepackages/system/linstor-scheduler/Chart.yaml
📚 Learning: 2025-12-25T09:45:26.524Z
Learnt from: CR
Repo: cozystack/cozystack PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-25T09:45:26.524Z
Learning: Applies to **/*.yaml : Do NOT directly edit upstream Helm charts in vendored charts directory; use proper chart vendoring mechanisms
Applied to files:
packages/system/linstor-scheduler/charts/linstor-scheduler/Chart.yamlpackages/system/linstor-scheduler/Chart.yaml
📚 Learning: 2025-06-04T06:22:17.306Z
Learnt from: lllamnyp
Repo: cozystack/cozystack PR: 1025
File: packages/apps/kafka/charts/cozy-lib:1-1
Timestamp: 2025-06-04T06:22:17.306Z
Learning: Files in packages/apps/*/charts/cozy-lib that contain the path "../../../library/cozy-lib" are symbolic links, not regular files. Git diffs may display symlinks as regular files with their target path as content, which can be misleading in code review.
Applied to files:
packages/system/linstor-scheduler/Chart.yaml
📚 Learning: 2025-12-25T15:24:00.888Z
Learnt from: kvaps
Repo: cozystack/cozystack PR: 1760
File: packages/core/platform/sources/goldpinger.yaml:20-23
Timestamp: 2025-12-25T15:24:00.888Z
Learning: In Goldpinger YAML configurations (e.g., packages/core/platform/sources/goldpinger.yaml), when hostNetwork is enabled, set privileged: true to ensure required host-level capabilities for cross-node network monitoring. Apply this rule to similar Goldpinger YAML files under the same directory. Confirm privileges are limited to scenarios that truly require host access and are scoped to this component.
Applied to files:
packages/core/platform/sources/linstor-scheduler.yaml
🪛 checkmake (0.2.2)
packages/system/linstor-scheduler/Makefile
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
🪛 LanguageTool
packages/system/linstor-scheduler/charts/linstor-scheduler/README.md
[grammar] ~55-~55: Ensure spelling is correct
Context: ...urity context. Defaults to dropping all capabilties and running as user 1000. | `{capabil...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~61-~61: Ensure spelling is correct
Context: ...urity context. Defaults to dropping all capabilties and running as user 1000. | `{capabil...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
packages/system/linstor-scheduler/charts/linstor-scheduler/README.md
23-23: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 YAMLlint (1.37.1)
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/certmanager.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/config.yaml
[error] 7-7: syntax error: expected the node content, but found '-'
(syntax)
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/tests/test-scheduler.yaml
[error] 4-4: syntax error: expected , but found ''
(syntax)
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/hpa.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/mutatingwebhookconfiguration.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/poddisruptionbudget.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/admission-deployment.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/rbac.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/deployment.yaml
[error] 7-7: syntax error: expected the node content, but found '-'
(syntax)
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/admission-service.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/serviceaccount.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Prepare environment
🔇 Additional comments (17)
packages/system/linstor-scheduler/charts/linstor-scheduler/LICENSE (1)
1-201: ✓ Standard Apache License 2.0 boilerplate is appropriate.This is the correct and complete Apache License 2.0 text (January 2004 edition) and is suitable for a vendored Helm chart component. The license file properly documents the terms for the linstor-scheduler chart.
Please verify that this license matches the upstream piraeusdatastore/helm-charts project's official licensing to ensure consistency and compliance with any licensing requirements from the original project.
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/NOTES.txt (1)
1-12: LGTM!The NOTES.txt template provides clear deployment status, configuration information, and usage examples for users. The structure and template references are correct.
packages/system/linstor-scheduler/charts/linstor-scheduler/values.yaml (1)
1-106: LGTM!The values file follows Helm best practices with sensible defaults:
- Security contexts properly configured with dropped capabilities and non-root execution
- Resources left empty for user configuration with helpful commented examples
- Admission webhook disabled by default (line 91) for safer initial deployments
- Clear structure separating scheduler, extender, and operational concerns
packages/system/linstor-scheduler/.helmignore (1)
1-1: LGTM!The helmignore pattern correctly excludes the examples directory from the packaged chart.
packages/core/platform/bundles/paas-full.yaml (1)
433-438: LGTM!The release entry is correctly placed after
linstor, follows the naming conventions, and has appropriate dependencies onlinstorandcert-manager(required for TLS certificates used by the admission webhook).packages/system/linstor-scheduler/values.yaml (1)
1-7: LGTM!The values correctly configure the linstor-scheduler for the cozystack environment:
- HTTPS endpoint to the LINSTOR controller with correct namespace and port
- TLS client secret for mTLS authentication
- Admission webhook enabled to auto-route LINSTOR PVC pods to the custom scheduler
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/admission-service.yaml (1)
1-20: LGTM!The admission Service template is correctly structured:
- Conditionally rendered based on
admission.enabled- ClusterIP service with standard HTTPS port (443) mapping to container port (8080)
- Proper selector labels to match the admission Deployment pods
Note: The static analysis YAML syntax error is a false positive caused by Helm templating directives which are valid.
packages/core/platform/sources/linstor-scheduler.yaml (1)
1-22: LGTM!The PackageSource definition is correctly configured:
- Follows the
cozystack.*naming convention- Dependencies (
cozystack.linstor,cozystack.cert-manager) align with bundledependsOnentries- Component path matches the package directory structure
- Install configuration is consistent with the bundle release entries
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/serviceaccount.yaml (1)
1-12: LGTM!The ServiceAccount template follows Helm best practices with conditional creation, proper use of template helpers for naming and labels, and optional annotations support.
Note: The YAMLlint syntax error is a false positive—the tool doesn't recognize Helm template directives.
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/certmanager.yaml (1)
1-56: Certificate management structure looks solid.The cert-manager setup follows best practices with a self-signed root CA, intermediate CA issuer, and properly scoped TLS certificate with appropriate DNS names for the admission webhook service.
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/mutatingwebhookconfiguration.yaml (1)
22-27: The original code withscope: "*"is valid according to the official Kubernetes API specification. Thescopefield in MutatingWebhookConfiguration explicitly accepts three values:"Cluster","Namespaced", and"*"(no scope restriction). While"*"is the default when the field is omitted, it is also a valid explicit value and should not be changed.Likely an incorrect or invalid review comment.
packages/system/linstor-scheduler/Makefile (1)
1-10: LGTM!The Makefile correctly follows the Helm Chart umbrella pattern with vendored upstream charts. The
updatetarget properly vendors thelinstor-schedulerchart from the piraeus-charts repository. Based on learnings, this aligns with the project's pattern for vendored charts in thecharts/directory.The static analysis warnings about missing
all,clean, andtesttargets are false positives—these are inherited from../../../scripts/package.mk.packages/system/linstor-scheduler/charts/linstor-scheduler/templates/config.yaml (2)
1-45: Vendored chart template structure looks correct.The version-aware apiVersion selection (v1 for K8s ≥1.25, v1beta3 for ≥1.23, v1beta2 otherwise) and the legacy policy.cfg fallback for K8s <1.22 are appropriately implemented.
The YAMLlint syntax error on line 7 is a false positive caused by Helm's
nindentdirective in template syntax.
27-27: ExcessivehttpTimeoutvalue (~83 hours).The timeout of
300000sis extremely long and could cause pods to remain pending indefinitely if the extender is unresponsive. A value like10swould be more appropriate.However, per coding guidelines, this is a vendored upstream chart. Consider opening an upstream issue/PR to fix this, or apply an override mechanism if the chart supports it.
⛔ Skipped due to learnings
Learnt from: CR Repo: cozystack/cozystack PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-25T09:45:26.524Z Learning: Applies to **/*.yaml : Do NOT directly edit upstream Helm charts in vendored charts directory; use proper chart vendoring mechanismspackages/system/linstor-scheduler/charts/linstor-scheduler/templates/rbac.yaml (1)
1-108: LGTM!The RBAC configuration is well-structured:
- Role scoping is appropriate with namespace-bound resources (leases, configmaps) in Role vs cluster-wide resources in ClusterRole
- The admission ClusterRole follows least-privilege with only
getverbs- Name truncation (
trunc 57,trunc 58) correctly accounts for suffixes (-as-ks,-as-vs,-auth) to stay within the 63-character DNS limit- Version-aware configmaps permission for K8s <1.22 is correctly conditional
The YAMLlint error is a false positive due to Helm conditional syntax.
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/admission-deployment.yaml (1)
31-50: Good security posture on the admission webhook container.The security configuration follows best practices:
- Pod-level:
runAsNonRoot, explicit UID/GID (1000)- Container-level:
allowPrivilegeEscalation: false,drop: ALLcapabilities,readOnlyRootFilesystem: truepackages/system/linstor-scheduler/charts/linstor-scheduler/templates/_helpers.tpl (1)
1-141: Helper templates are well-structured.The helpers follow standard Helm conventions:
- Name/fullname truncation at 63 chars respects DNS naming limits
- Dual API group support (
piraeus.linbit.com/v1andlinstor.linbit.com/v1) provides good backward compatibility- The
linstorEndpointhelper properly fails with a clear message when no endpoint can be determined- The
kubeVersionhelper allows overriding the detected Kubernetes version for compatibility testing
| name: {{ include "linstor-scheduler.linstorClientSecretName" . }} | ||
| key: ca.crt | ||
| {{- end }} | ||
| {{- if semverCompare ">= 1.22-0" .Capabilities.KubeVersion.Version }} |
There was a problem hiding this comment.
Inconsistent Kubernetes version check.
Line 109 uses .Capabilities.KubeVersion.Version directly, while lines 38 and 73 use the linstor-scheduler.kubeVersion helper template. This inconsistency could cause issues if scheduler.image.compatibleKubernetesRelease is set, as the volume would be conditionally rendered based on the cluster version instead of the configured version.
♻️ Suggested fix for consistency
- {{- if semverCompare ">= 1.22-0" .Capabilities.KubeVersion.Version }}
+ {{- if semverCompare ">= 1.22-0" (include "linstor-scheduler.kubeVersion" .) }}
volumes:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {{- if semverCompare ">= 1.22-0" .Capabilities.KubeVersion.Version }} | |
| {{- if semverCompare ">= 1.22-0" (include "linstor-scheduler.kubeVersion" .) }} |
🤖 Prompt for AI Agents
In
@packages/system/linstor-scheduler/charts/linstor-scheduler/templates/deployment.yaml
at line 109, The semver check on that line uses
.Capabilities.KubeVersion.Version directly which is inconsistent with the rest
of the chart; change the condition to use the existing helper by replacing
.Capabilities.KubeVersion.Version with the helper invocation (e.g. {{- if
semverCompare ">= 1.22-0" (include "linstor-scheduler.kubeVersion" .) }}), so
the volume rendering uses the configured linstor-scheduler.kubeVersion value
just like the checks at lines referencing the linstor-scheduler.kubeVersion
helper.
| {{- with .Values.imagePullSecrets }} | ||
| imagePullSecrets: | ||
| {{- toYaml . | nindent 8 }} | ||
| {{- end }} |
There was a problem hiding this comment.
Duplicate imagePullSecrets block will cause API validation error.
This imagePullSecrets block duplicates the one already defined on lines 24-27. Kubernetes will reject this deployment spec with a validation error.
🐛 Proposed fix
volumes:
- configMap:
name: {{ include "linstor-scheduler.fullname" . }}
name: scheduler-config
{{- end }}
- {{- with .Values.imagePullSecrets }}
- imagePullSecrets:
- {{- toYaml . | nindent 8 }}
- {{- end }}
{{- with .Values.nodeSelector }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {{- with .Values.imagePullSecrets }} | |
| imagePullSecrets: | |
| {{- toYaml . | nindent 8 }} | |
| {{- end }} |
🤖 Prompt for AI Agents
In
@packages/system/linstor-scheduler/charts/linstor-scheduler/templates/deployment.yaml
around lines 115 - 118, The deployment template contains a duplicated
imagePullSecrets block using .Values.imagePullSecrets which will cause
Kubernetes validation to fail; remove the second/duplicate imagePullSecrets
block (the one rendering {{- with .Values.imagePullSecrets }} ... {{- end }}) so
only the original imagePullSecrets declaration remains, or consolidate into a
single imagePullSecrets rendering that sits at the intended spec.template.spec
location and uses {{- toYaml . | nindent 8 }} with .Values.imagePullSecrets.
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/poddisruptionbudget.yaml
Outdated
Show resolved
Hide resolved
| {{- if .Values.podDisruptionBudget.minAvailable }} | ||
| minAvailable: {{ .Values.podDisruptionBudget.minAvailable }} | ||
| {{- end }} | ||
| {{- if .Values.podDisruptionBudget.maxUnavailable }} | ||
| maxUnavailable: {{ .Values.podDisruptionBudget.maxUnavailable }} | ||
| {{- end }} |
There was a problem hiding this comment.
Clarify mutually exclusive PDB fields.
Both minAvailable and maxUnavailable are conditionally rendered, but these fields are mutually exclusive—only one should be set at a time. While Kubernetes accepts configurations with both defined, it can lead to confusion about which constraint is active.
🤖 Prompt for AI Agents
In
@packages/system/linstor-scheduler/charts/linstor-scheduler/templates/poddisruptionbudget.yaml
around lines 12 - 17, The podDisruptionBudget template renders both minAvailable
and maxUnavailable which are mutually exclusive; change the template to render
only one by using an if/else-if: if .Values.podDisruptionBudget.minAvailable is
set, emit minAvailable and skip maxUnavailable, else-if
.Values.podDisruptionBudget.maxUnavailable emit maxUnavailable; also update the
podDisruptionBudget section in values.yaml (and its comments) to document that
only one of minAvailable or maxUnavailable should be provided.
| containers: | ||
| - name: wget | ||
| image: busybox | ||
| command: [] |
There was a problem hiding this comment.
Fix empty command to make test functional.
The busybox container has an empty command: [], which will cause the container to fail immediately since busybox requires a command to execute. For a smoke test of the scheduler, provide a simple command that succeeds.
🔧 Proposed fix
containers:
- name: wget
image: busybox
- command: []
+ command: ['sh', '-c', 'echo "Scheduler test successful" && exit 0']📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| containers: | |
| - name: wget | |
| image: busybox | |
| command: [] | |
| containers: | |
| - name: wget | |
| image: busybox | |
| command: ['sh', '-c', 'echo "Scheduler test successful" && exit 0'] |
🤖 Prompt for AI Agents
In
@packages/system/linstor-scheduler/charts/linstor-scheduler/templates/tests/test-scheduler.yaml
around lines 12 - 15, The test container "wget" in
templates/tests/test-scheduler.yaml has an empty command (command: []), which
causes busybox to exit immediately; update the manifest to provide a simple,
successful command for the smoke test (e.g., use the wget container name) such
as running a no-op or short sleep that returns success (for example change
command to run ["sh","-c","true"] or ["sh","-c","sleep 1 && exit 0"]) so the pod
stays alive long enough for the scheduler test to succeed.
Add linstor-scheduler-extender for optimal pod placement on nodes with LINSTOR storage. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
706e305 to
47b5a57
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In @packages/system/linstor-scheduler/charts/linstor-scheduler/Chart.yaml:
- Line 3: Add documentation explaining why the vendored linstor-scheduler chart
has deprecated: true: either add a README.md to the parent chart directory or
update the parent Chart.yaml description to state that the upstream Piraeus Helm
charts were deprecated in v2.0.0 in favor of kustomize, explain why this
repository still vendors the Helm chart (e.g., compatibility, legacy
deployments, CI/testing), outline the recommended migration path or replacement
strategy (switch to kustomize manifests or provide links to conversion steps),
and explicitly state whether users should continue using the vendored Helm chart
or migrate to kustomize so expectations are clear.
In @packages/system/linstor-scheduler/charts/linstor-scheduler/README.md:
- Around line 23-26: The fenced code block in README.md lacks a language
identifier, so update the triple-backtick fence that surrounds the helm commands
to include "bash" (i.e., change ``` to ```bash) to enable proper syntax
highlighting; ensure the opening fence before the two helm commands is modified
and the closing fence remains unchanged so only the language tag is added.
- Line 55: The README entry for extender.securityContext has a typo: change
"capabilties" to "capabilities" in the description text ("Defaults to dropping
all capabilties and running as user 1000.") so the line reads "Defaults to
dropping all capabilities and running as user 1000."; update any other
occurrences in that table row (including the default value cell if present) to
use the correct spelling.
- Line 72: Fix the table row for autoscaling by removing the stray backtick from
the description and correcting the malformed default value; update the
`autoscaling.enabled` row so the description ends without a backtick and the
default cell shows a properly formatted value such as `false` (use backticks
around the value for consistent code-style) instead of the current broken
`"false`.
- Line 61: Spelling error in the README table: change the key label
"capabilties" to "capabilities" inside the description for
scheduler.securityContext so the example default becomes `{capabilities: {drop:
[ALL]}, readOnlyRootFilesystem: true, runAsNonRoot: true, runAsUser: 1000}`;
update only the displayed text in the table cell referencing
scheduler.securityContext to correct the typo.
In
@packages/system/linstor-scheduler/charts/linstor-scheduler/templates/deployment.yaml:
- Around line 109-114: The semverCompare call in the volumes block uses
.Capabilities.KubeVersion.Version directly which is inconsistent with the rest
of the chart; change the condition to use the same helper as other checks by
calling the linstor-scheduler.kubeVersion template (i.e. semverCompare ">=
1.22-0" (include "linstor-scheduler.kubeVersion" .)) so the volumes rendering
uses the same Kubernetes version source as the container configuration and keeps
behavior consistent for scheduler.image.compatibleKubernetesRelease and the
scheduler-config volume.
In
@packages/system/linstor-scheduler/charts/linstor-scheduler/templates/hpa.yaml:
- Around line 15-31: When autoscaling is enabled ensure the HPA metrics array is
never empty by adding validation around the metrics block: check
.Values.autoscaling.targetCPUUtilizationPercentage and
.Values.autoscaling.targetMemoryUtilizationPercentage and if both are unset emit
a Helm template failure (using fail or required) or supply a sane default
metric; update the conditional around the metrics array (the metrics block that
references .Values.autoscaling.targetCPUUtilizationPercentage and
.Values.autoscaling.targetMemoryUtilizationPercentage) so that when
.Values.autoscaling.enabled is true at least one metric is present or the
template fails with a clear message.
In @packages/system/linstor-scheduler/Makefile:
- Around line 6-10: The Makefile's update target should be made reproducible and
declared phony: modify the update target (symbol: update) to pass an explicit
chart version to the helm pull command by adding a --version <VERSION> argument
(replace <VERSION> with the pinned release like 1.2.3) so the pulled
piraeus-charts/linstor-scheduler chart is deterministic, and add a .PHONY:
update declaration (symbol: .PHONY and update) above the target so make won't
treat the target as a file.
🧹 Nitpick comments (1)
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/mutatingwebhookconfiguration.yaml (1)
22-27: Consider usingscope: "Namespaced"for pods.The webhook rule specifies
scope: "*"which matches both namespaced and cluster-scoped resources. Since pods are exclusively namespace-scoped, usingscope: "Namespaced"would be more precise and can improve webhook matching performance.♻️ Proposed refinement
rules: - operations: ["CREATE"] apiGroups: [""] apiVersions: ["v1"] resources: ["pods"] - scope: "*" + scope: "Namespaced"
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
packages/core/platform/bundles/distro-full.yamlpackages/core/platform/bundles/paas-full.yamlpackages/core/platform/sources/linstor-scheduler.yamlpackages/system/linstor-scheduler/.helmignorepackages/system/linstor-scheduler/Chart.yamlpackages/system/linstor-scheduler/Makefilepackages/system/linstor-scheduler/charts/linstor-scheduler/Chart.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/LICENSEpackages/system/linstor-scheduler/charts/linstor-scheduler/README.mdpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/NOTES.txtpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/_helpers.tplpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/admission-deployment.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/admission-service.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/certmanager.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/config.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/deployment.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/hpa.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/mutatingwebhookconfiguration.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/poddisruptionbudget.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/rbac.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/serviceaccount.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/tests/test-scheduler.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/values.yamlpackages/system/linstor-scheduler/values.yaml
✅ Files skipped from review due to trivial changes (2)
- packages/system/linstor-scheduler/charts/linstor-scheduler/values.yaml
- packages/system/linstor-scheduler/charts/linstor-scheduler/LICENSE
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/system/linstor-scheduler/.helmignore
- packages/core/platform/bundles/paas-full.yaml
- packages/system/linstor-scheduler/values.yaml
- packages/system/linstor-scheduler/charts/linstor-scheduler/templates/_helpers.tpl
🧰 Additional context used
📓 Path-based instructions (4)
packages/system/**
📄 CodeRabbit inference engine (AGENTS.md)
Use Helm Chart umbrella pattern with vendored upstream charts in
charts/directory
Files:
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/NOTES.txtpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/tests/test-scheduler.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/admission-deployment.yamlpackages/system/linstor-scheduler/Chart.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/Chart.yamlpackages/system/linstor-scheduler/Makefilepackages/system/linstor-scheduler/charts/linstor-scheduler/README.mdpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/admission-service.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/certmanager.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/config.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/deployment.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/hpa.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/mutatingwebhookconfiguration.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/poddisruptionbudget.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/rbac.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/serviceaccount.yaml
**/*.yaml
📄 CodeRabbit inference engine (AGENTS.md)
Do NOT directly edit upstream Helm charts in vendored charts directory; use proper chart vendoring mechanisms
Files:
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/tests/test-scheduler.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/admission-deployment.yamlpackages/core/platform/sources/linstor-scheduler.yamlpackages/core/platform/bundles/distro-full.yamlpackages/system/linstor-scheduler/Chart.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/Chart.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/admission-service.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/certmanager.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/config.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/deployment.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/hpa.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/mutatingwebhookconfiguration.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/poddisruptionbudget.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/rbac.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/templates/serviceaccount.yaml
packages/core/**
📄 CodeRabbit inference engine (AGENTS.md)
Use Helm Chart umbrella pattern with vendored upstream charts in
charts/directory
Files:
packages/core/platform/sources/linstor-scheduler.yamlpackages/core/platform/bundles/distro-full.yaml
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Git commits should follow
[component] Descriptionformat and include--signoffflag
Files:
packages/system/linstor-scheduler/charts/linstor-scheduler/README.md
🧠 Learnings (6)
📚 Learning: 2025-12-25T15:24:00.888Z
Learnt from: kvaps
Repo: cozystack/cozystack PR: 1760
File: packages/core/platform/sources/goldpinger.yaml:20-23
Timestamp: 2025-12-25T15:24:00.888Z
Learning: In Goldpinger YAML configurations (e.g., packages/core/platform/sources/goldpinger.yaml), when hostNetwork is enabled, set privileged: true to ensure required host-level capabilities for cross-node network monitoring. Apply this rule to similar Goldpinger YAML files under the same directory. Confirm privileges are limited to scenarios that truly require host access and are scoped to this component.
Applied to files:
packages/core/platform/sources/linstor-scheduler.yaml
📚 Learning: 2025-07-23T09:15:09.658Z
Learnt from: lllamnyp
Repo: cozystack/cozystack PR: 1233
File: packages/system/hetzner-robotlb/charts/robotlb/templates/deployment.yaml:33-35
Timestamp: 2025-07-23T09:15:09.658Z
Learning: The `./charts/robotlb/` directory in the hetzner-robotlb package contains vendored code, and the team generally avoids modifying vendored code to maintain clean separation from upstream dependencies.
Applied to files:
packages/system/linstor-scheduler/Chart.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/Chart.yamlpackages/system/linstor-scheduler/Makefile
📚 Learning: 2025-12-25T09:45:26.524Z
Learnt from: CR
Repo: cozystack/cozystack PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-25T09:45:26.524Z
Learning: Applies to packages/system/** : Use Helm Chart umbrella pattern with vendored upstream charts in `charts/` directory
Applied to files:
packages/system/linstor-scheduler/Chart.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/Chart.yaml
📚 Learning: 2025-12-25T09:45:26.524Z
Learnt from: CR
Repo: cozystack/cozystack PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-25T09:45:26.524Z
Learning: Applies to **/*.yaml : Do NOT directly edit upstream Helm charts in vendored charts directory; use proper chart vendoring mechanisms
Applied to files:
packages/system/linstor-scheduler/Chart.yamlpackages/system/linstor-scheduler/charts/linstor-scheduler/Chart.yaml
📚 Learning: 2025-06-04T06:22:17.306Z
Learnt from: lllamnyp
Repo: cozystack/cozystack PR: 1025
File: packages/apps/kafka/charts/cozy-lib:1-1
Timestamp: 2025-06-04T06:22:17.306Z
Learning: Files in packages/apps/*/charts/cozy-lib that contain the path "../../../library/cozy-lib" are symbolic links, not regular files. Git diffs may display symlinks as regular files with their target path as content, which can be misleading in code review.
Applied to files:
packages/system/linstor-scheduler/Chart.yaml
📚 Learning: 2025-12-25T09:45:26.524Z
Learnt from: CR
Repo: cozystack/cozystack PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-25T09:45:26.524Z
Learning: Applies to packages/core/** : Use Helm Chart umbrella pattern with vendored upstream charts in `charts/` directory
Applied to files:
packages/system/linstor-scheduler/charts/linstor-scheduler/Chart.yamlpackages/system/linstor-scheduler/Makefile
🪛 checkmake (0.2.2)
packages/system/linstor-scheduler/Makefile
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
[warning] 1-1: Missing required phony target "test"
(minphony)
🪛 LanguageTool
packages/system/linstor-scheduler/charts/linstor-scheduler/README.md
[grammar] ~55-~55: Ensure spelling is correct
Context: ...urity context. Defaults to dropping all capabilties and running as user 1000. | `{capabil...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~61-~61: Ensure spelling is correct
Context: ...urity context. Defaults to dropping all capabilties and running as user 1000. | `{capabil...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
packages/system/linstor-scheduler/charts/linstor-scheduler/README.md
23-23: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 YAMLlint (1.37.1)
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/tests/test-scheduler.yaml
[error] 4-4: syntax error: expected , but found ''
(syntax)
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/admission-deployment.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/admission-service.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/certmanager.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/config.yaml
[error] 7-7: syntax error: expected the node content, but found '-'
(syntax)
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/deployment.yaml
[error] 7-7: syntax error: expected the node content, but found '-'
(syntax)
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/hpa.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/mutatingwebhookconfiguration.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/poddisruptionbudget.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/rbac.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/serviceaccount.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🔇 Additional comments (12)
packages/system/linstor-scheduler/Makefile (2)
1-2: LGTM!The exported variables follow the expected naming conventions and align with the package structure.
4-4: LGTM!The inclusion of package.mk follows the standard pattern. Note that the static analysis warnings about missing phony targets (all, clean, test) are false positives, as these are likely provided by package.mk.
packages/system/linstor-scheduler/Chart.yaml (1)
1-3: LGTM!The chart metadata is correctly structured with an appropriate placeholder version that will be set during the build process. The chart name
cozy-linstor-scheduleris consistent with the bundle references in the platform bundles.packages/system/linstor-scheduler/charts/linstor-scheduler/templates/NOTES.txt (1)
1-12: LGTM!The NOTES template provides clear post-installation instructions, including verification steps and usage examples. The template correctly references the chart helpers and provides actionable guidance for users.
packages/core/platform/sources/linstor-scheduler.yaml (1)
1-22: LGTM!The PackageSource definition correctly references the OCIRepository, declares appropriate dependencies (linstor and cert-manager), and specifies the installation namespace and release name. The structure follows the standard pattern for cozystack package sources.
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/admission-service.yaml (1)
13-16: > Likely an incorrect or invalid review comment.packages/system/linstor-scheduler/charts/linstor-scheduler/templates/serviceaccount.yaml (1)
1-12: LGTM!Standard Helm ServiceAccount template with conditional creation, proper use of helper templates for naming and labels, and support for custom annotations.
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/certmanager.yaml (1)
1-56: LGTM!Standard cert-manager certificate chain for admission webhook TLS:
- Self-signed issuer creates a 5-year root CA
- CA issuer uses the root CA to issue a 1-year service certificate
- DNS names properly include both short service name and namespace-scoped FQDN
- Certificate renewal set to 24h before expiration
The certificate hierarchy and configuration follow best practices for webhook TLS with cert-manager.
packages/core/platform/bundles/distro-full.yaml (1)
360-364: LGTM!The linstor-scheduler release entry is correctly configured:
- Appropriate dependencies on
linstor(for controller API) andcert-manager(for webhook TLS)- Co-located in
cozy-linstornamespace with the linstor controller- Follows the bundle configuration pattern
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/config.yaml (1)
1-46: LGTM!The scheduler configuration correctly implements version-gated config formats:
- Uses KubeSchedulerConfiguration (v1/v1beta3/v1beta2) for Kubernetes >= 1.22
- Falls back to Policy.cfg JSON for older versions
- The
httpTimeout: 10s(line 27) is appropriate for scheduler extender operations- Extender configuration (localhost:8099, filter/prioritize verbs, weight 5) is consistent across both config formats
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/rbac.yaml (1)
1-108: LGTM!The RBAC configuration is well-structured and follows the principle of least privilege:
- Role: Grants minimal permissions for leader election (leases) and legacy config access (configmaps for Kubernetes < 1.22)
- Admission ClusterRole: Read-only access to pods, PVCs, PVs, and storageclasses—sufficient for the mutating webhook to determine LINSTOR storage usage
- Scheduler ClusterRoleBindings: Properly binds to built-in
system:kube-schedulerandsystem:volume-schedulerroles- Authentication RoleBinding: Grants access to
extension-apiserver-authentication-readerin kube-system for scheduler authentication- Name truncation: Lines 58, 71, and 98 appropriately truncate names to avoid exceeding Kubernetes' 63-character limit while preserving uniqueness
packages/system/linstor-scheduler/charts/linstor-scheduler/templates/admission-deployment.yaml (1)
1-97: LGTM with well-configured security settings.The admission webhook deployment follows Kubernetes security best practices with:
- Restrictive pod and container security contexts (runAsNonRoot, drop ALL capabilities, readOnlyRootFilesystem)
- Read-only volume mounts for TLS certificates
- Proper gating with
.Values.admission.enabled- Appropriate resource and placement configurations
| @@ -0,0 +1,19 @@ | |||
| apiVersion: v2 | |||
| appVersion: v0.3.2 | |||
| deprecated: true | |||
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
linstor-scheduler Helm chart piraeusdatastore deprecation status
💡 Result:
Short answer: The linstor-scheduler Helm chart is included in the piraeusdatastore helm-charts repo, but Piraeus declared its Helm chart deployment deprecated in favor of a kustomize-based deployment as of v2.0.0 (2023-02-23). See Piraeus changelog and the helm-charts repo. [1][2]
Sources:
- Piraeus Datastore CHANGELOG (v2.0.0, 2023-02-23) — "Helm chart deprecated in favor of new
kustomizedeployment." [1] - piraeusdatastore/helm-charts GitHub (lists linstor-scheduler chart present). [2]
🏁 Script executed:
# Check the repository structure and the actual Chart.yaml content
find . -path "*linstor-scheduler*" -name "Chart.yaml" 2>/dev/null | head -20Repository: cozystack/cozystack
Length of output: 181
🏁 Script executed:
# Look for any documentation or references to linstor-scheduler in the repo
rg -l "linstor-scheduler" --type yaml --type md -i | head -20Repository: cozystack/cozystack
Length of output: 1488
🏁 Script executed:
# Check if there's a parent chart or umbrella pattern in the system package
ls -la packages/system/ 2>/dev/null | head -20Repository: cozystack/cozystack
Length of output: 1327
🏁 Script executed:
cat -n packages/system/linstor-scheduler/Chart.yamlRepository: cozystack/cozystack
Length of output: 227
🏁 Script executed:
cat -n packages/system/linstor-scheduler/charts/linstor-scheduler/Chart.yamlRepository: cozystack/cozystack
Length of output: 914
🏁 Script executed:
cat -n packages/system/linstor-scheduler/values.yaml | head -30Repository: cozystack/cozystack
Length of output: 314
🏁 Script executed:
cat packages/system/linstor-scheduler/README.md 2>/dev/null || echo "No README found in parent chart"Repository: cozystack/cozystack
Length of output: 94
🏁 Script executed:
rg -A5 -B5 "deprecated|kustomize|migration" packages/system/linstor-scheduler/ --type yaml --type md 2>/dev/null | head -50Repository: cozystack/cozystack
Length of output: 1089
🏁 Script executed:
# Check if there's any documentation in the parent values about this
cat packages/system/linstor-scheduler/values.yamlRepository: cozystack/cozystack
Length of output: 265
Document why a deprecated upstream chart is being added to the repository.
The deprecated: true flag in the vendored linstor-scheduler chart is legitimate—piraeusdatastore deprecated its Helm charts in v2.0.0 (2023-02-23) in favor of kustomize-based deployment. However, this repository lacks documentation explaining why a deprecated upstream chart is being vendored.
Add a README.md to the parent chart directory (or update the parent Chart.yaml description) to clarify:
- Why the deprecated Piraeus Helm chart is included in this repository
- The migration path or replacement strategy (if any)
- Whether users should continue using this deployment method or switch to kustomize
This prevents confusion about the deprecated status and sets expectations for users.
🤖 Prompt for AI Agents
In @packages/system/linstor-scheduler/charts/linstor-scheduler/Chart.yaml at
line 3, Add documentation explaining why the vendored linstor-scheduler chart
has deprecated: true: either add a README.md to the parent chart directory or
update the parent Chart.yaml description to state that the upstream Piraeus Helm
charts were deprecated in v2.0.0 in favor of kustomize, explain why this
repository still vendors the Helm chart (e.g., compatibility, legacy
deployments, CI/testing), outline the recommended migration path or replacement
strategy (switch to kustomize manifests or provide links to conversion steps),
and explicitly state whether users should continue using the vendored Helm chart
or migrate to kustomize so expectations are clear.
| ``` | ||
| helm repo add piraeus-charts https://piraeus.io/helm-charts/ | ||
| helm install linstor-scheduler piraeus-charts/linstor-scheduler --set linstorEndpoint=https://piraeus-op-cs.piraeus.svc:3371 --set linstorClientSecret=piraeus-client-secret | ||
| ``` |
There was a problem hiding this comment.
Add language identifier to fenced code block.
The bash code block should specify a language identifier for proper syntax highlighting.
📝 Proposed fix
-```
+```bash
helm repo add piraeus-charts https://piraeus.io/helm-charts/
helm install linstor-scheduler piraeus-charts/linstor-scheduler --set linstorEndpoint=https://piraeus-op-cs.piraeus.svc:3371 --set linstorClientSecret=piraeus-client-secret</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
23-23: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In @packages/system/linstor-scheduler/charts/linstor-scheduler/README.md around
lines 23 - 26, The fenced code block in README.md lacks a language identifier,
so update the triple-backtick fence that surrounds the helm commands to include
"bash" (i.e., change ``` to ```bash) to enable proper syntax highlighting;
ensure the opening fence before the two helm commands is modified and the
closing fence remains unchanged so only the language tag is added.
| | `extender.image.pullPolicy` | Pull policy to use. Possible values: `IfNotPresent`, `Always`, `Never` | `IfNotPresent` | | ||
| | `extender.image.tag` | Override the tag to pull. If not given, defaults to charts `AppVersion`. | `""` | | ||
| | `extender.resources` | Resources to request and limit on the container. | `{}` | | ||
| | `extender.securityContext` | Configure container security context. Defaults to dropping all capabilties and running as user 1000. | `{capabilities: {drop: [ALL]}, readOnlyRootFilesystem: true, runAsNonRoot: true, runAsUser: 1000}` | |
There was a problem hiding this comment.
Fix spelling error in configuration table.
The word "capabilties" should be "capabilities".
📝 Proposed fix
-| `extender.securityContext` | Configure container security context. Defaults to dropping all capabilties and running as user 1000. | `{capabilities: {drop: [ALL]}, readOnlyRootFilesystem: true, runAsNonRoot: true, runAsUser: 1000}` |
+| `extender.securityContext` | Configure container security context. Defaults to dropping all capabilities and running as user 1000. | `{capabilities: {drop: [ALL]}, readOnlyRootFilesystem: true, runAsNonRoot: true, runAsUser: 1000}` |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | `extender.securityContext` | Configure container security context. Defaults to dropping all capabilties and running as user 1000. | `{capabilities: {drop: [ALL]}, readOnlyRootFilesystem: true, runAsNonRoot: true, runAsUser: 1000}` | | |
| | `extender.securityContext` | Configure container security context. Defaults to dropping all capabilities and running as user 1000. | `{capabilities: {drop: [ALL]}, readOnlyRootFilesystem: true, runAsNonRoot: true, runAsUser: 1000}` | |
🧰 Tools
🪛 LanguageTool
[grammar] ~55-~55: Ensure spelling is correct
Context: ...urity context. Defaults to dropping all capabilties and running as user 1000. | `{capabil...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
In @packages/system/linstor-scheduler/charts/linstor-scheduler/README.md at line
55, The README entry for extender.securityContext has a typo: change
"capabilties" to "capabilities" in the description text ("Defaults to dropping
all capabilties and running as user 1000.") so the line reads "Defaults to
dropping all capabilities and running as user 1000."; update any other
occurrences in that table row (including the default value cell if present) to
use the correct spelling.
| | `scheduler.image.tag` | Override the tag to pull. If not given, defaults to kubernetes version. | `""` | | ||
| | `scheduler.image.compatibleKubernetesRelease` | Compatible kubernetes version for this scheduler, used to generate configuration in the right version. | `""` | | ||
| | `scheduler.resources` | Resources to request and limit on the container. | `{}` | | ||
| | `scheduler.securityContext` | Configure container security context. Defaults to dropping all capabilties and running as user 1000. | `{capabilities: {drop: [ALL]}, readOnlyRootFilesystem: true, runAsNonRoot: true, runAsUser: 1000}` | |
There was a problem hiding this comment.
Fix spelling error in configuration table.
The word "capabilties" should be "capabilities".
📝 Proposed fix
-| `scheduler.securityContext` | Configure container security context. Defaults to dropping all capabilties and running as user 1000. | `{capabilities: {drop: [ALL]}, readOnlyRootFilesystem: true, runAsNonRoot: true, runAsUser: 1000}` |
+| `scheduler.securityContext` | Configure container security context. Defaults to dropping all capabilities and running as user 1000. | `{capabilities: {drop: [ALL]}, readOnlyRootFilesystem: true, runAsNonRoot: true, runAsUser: 1000}` |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | `scheduler.securityContext` | Configure container security context. Defaults to dropping all capabilties and running as user 1000. | `{capabilities: {drop: [ALL]}, readOnlyRootFilesystem: true, runAsNonRoot: true, runAsUser: 1000}` | | |
| | `scheduler.securityContext` | Configure container security context. Defaults to dropping all capabilities and running as user 1000. | `{capabilities: {drop: [ALL]}, readOnlyRootFilesystem: true, runAsNonRoot: true, runAsUser: 1000}` | |
🧰 Tools
🪛 LanguageTool
[grammar] ~61-~61: Ensure spelling is correct
Context: ...urity context. Defaults to dropping all capabilties and running as user 1000. | `{capabil...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
In @packages/system/linstor-scheduler/charts/linstor-scheduler/README.md at line
61, Spelling error in the README table: change the key label "capabilties" to
"capabilities" inside the description for scheduler.securityContext so the
example default becomes `{capabilities: {drop: [ALL]}, readOnlyRootFilesystem:
true, runAsNonRoot: true, runAsUser: 1000}`; update only the displayed text in
the table cell referencing scheduler.securityContext to correct the typo.
| | `serviceAccount.create` | Create the service account resource | `true` | | ||
| | `serviceAccount.name` | Sets the name of the service account. If left empty, will use the release name as default | `""` | | ||
| | `podDisruptionBudget.enabled` | Enable creation of a pod disruption budget to protect the availability of the scheduler | `true` | | ||
| | `autoscaling.enabled` | Enable creation of a horizontal pod autoscaler to ensure availability in case of high usage` | `"false` | |
There was a problem hiding this comment.
Fix malformed default value and stray backtick.
The configuration table entry has formatting issues: a stray backtick at the end of the description and a malformed default value.
📝 Proposed fix
-| `autoscaling.enabled` | Enable creation of a horizontal pod autoscaler to ensure availability in case of high usage` | `"false` |
+| `autoscaling.enabled` | Enable creation of a horizontal pod autoscaler to ensure availability in case of high usage | `false` |🤖 Prompt for AI Agents
In @packages/system/linstor-scheduler/charts/linstor-scheduler/README.md at line
72, Fix the table row for autoscaling by removing the stray backtick from the
description and correcting the malformed default value; update the
`autoscaling.enabled` row so the description ends without a backtick and the
default cell shows a properly formatted value such as `false` (use backticks
around the value for consistent code-style) instead of the current broken
`"false`.
| {{- if semverCompare ">= 1.22-0" .Capabilities.KubeVersion.Version }} | ||
| volumes: | ||
| - configMap: | ||
| name: {{ include "linstor-scheduler.fullname" . }} | ||
| name: scheduler-config | ||
| {{- end }} |
There was a problem hiding this comment.
Inconsistent Kubernetes version check (previously flagged).
Line 109 uses .Capabilities.KubeVersion.Version directly, while lines 38 and 73 correctly use the linstor-scheduler.kubeVersion helper template. This inconsistency means the volume rendering could use a different version than the container configuration, especially if scheduler.image.compatibleKubernetesRelease is set.
♻️ Suggested fix for consistency
- {{- if semverCompare ">= 1.22-0" .Capabilities.KubeVersion.Version }}
+ {{- if semverCompare ">= 1.22-0" (include "linstor-scheduler.kubeVersion" .) }}
volumes:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {{- if semverCompare ">= 1.22-0" .Capabilities.KubeVersion.Version }} | |
| volumes: | |
| - configMap: | |
| name: {{ include "linstor-scheduler.fullname" . }} | |
| name: scheduler-config | |
| {{- end }} | |
| {{- if semverCompare ">= 1.22-0" (include "linstor-scheduler.kubeVersion" .) }} | |
| volumes: | |
| - configMap: | |
| name: {{ include "linstor-scheduler.fullname" . }} | |
| name: scheduler-config | |
| {{- end }} |
🤖 Prompt for AI Agents
In
@packages/system/linstor-scheduler/charts/linstor-scheduler/templates/deployment.yaml
around lines 109 - 114, The semverCompare call in the volumes block uses
.Capabilities.KubeVersion.Version directly which is inconsistent with the rest
of the chart; change the condition to use the same helper as other checks by
calling the linstor-scheduler.kubeVersion template (i.e. semverCompare ">=
1.22-0" (include "linstor-scheduler.kubeVersion" .)) so the volumes rendering
uses the same Kubernetes version source as the container configuration and keeps
behavior consistent for scheduler.image.compatibleKubernetesRelease and the
scheduler-config volume.
| metrics: | ||
| {{- if .Values.autoscaling.targetCPUUtilizationPercentage }} | ||
| - type: Resource | ||
| resource: | ||
| name: cpu | ||
| target: | ||
| type: Utilization | ||
| averageUtilization: {{ .Values.autoscaling.targetCPUUtilizationPercentage }} | ||
| {{- end }} | ||
| {{- if .Values.autoscaling.targetMemoryUtilizationPercentage }} | ||
| - type: Resource | ||
| resource: | ||
| name: memory | ||
| target: | ||
| type: Utilization | ||
| averageUtilization: {{ .Values.autoscaling.targetMemoryUtilizationPercentage }} | ||
| {{- end }} |
There was a problem hiding this comment.
Ensure at least one metric is defined when autoscaling is enabled.
The metrics array is conditionally populated, but if both targetCPUUtilizationPercentage and targetMemoryUtilizationPercentage are unset, the HPA will have an empty metrics array, which is invalid and will cause deployment failures.
🔒 Recommended fix
Add validation to ensure at least one metric is configured when autoscaling is enabled:
{{- if .Values.autoscaling.enabled }}
+{{- if not (or .Values.autoscaling.targetCPUUtilizationPercentage .Values.autoscaling.targetMemoryUtilizationPercentage) }}
+{{- fail "autoscaling.enabled is true but no metrics are configured. Set either targetCPUUtilizationPercentage or targetMemoryUtilizationPercentage" }}
+{{- end }}
apiVersion: autoscaling/v2
kind: HorizontalPodAutoscaler📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| metrics: | |
| {{- if .Values.autoscaling.targetCPUUtilizationPercentage }} | |
| - type: Resource | |
| resource: | |
| name: cpu | |
| target: | |
| type: Utilization | |
| averageUtilization: {{ .Values.autoscaling.targetCPUUtilizationPercentage }} | |
| {{- end }} | |
| {{- if .Values.autoscaling.targetMemoryUtilizationPercentage }} | |
| - type: Resource | |
| resource: | |
| name: memory | |
| target: | |
| type: Utilization | |
| averageUtilization: {{ .Values.autoscaling.targetMemoryUtilizationPercentage }} | |
| {{- end }} | |
| {{- if .Values.autoscaling.enabled }} | |
| {{- if not (or .Values.autoscaling.targetCPUUtilizationPercentage .Values.autoscaling.targetMemoryUtilizationPercentage) }} | |
| {{- fail "autoscaling.enabled is true but no metrics are configured. Set either targetCPUUtilizationPercentage or targetMemoryUtilizationPercentage" }} | |
| {{- end }} | |
| apiVersion: autoscaling/v2 | |
| kind: HorizontalPodAutoscaler | |
| metadata: | |
| name: {{ include "linstor-scheduler.fullname" . }} | |
| labels: | |
| {{- include "linstor-scheduler.labels" . | nindent 4 }} | |
| spec: | |
| scaleTargetRef: | |
| apiVersion: apps/v1 | |
| kind: Deployment | |
| name: {{ include "linstor-scheduler.fullname" . }} | |
| minReplicas: {{ .Values.autoscaling.minReplicas }} | |
| maxReplicas: {{ .Values.autoscaling.maxReplicas }} | |
| metrics: | |
| {{- if .Values.autoscaling.targetCPUUtilizationPercentage }} | |
| - type: Resource | |
| resource: | |
| name: cpu | |
| target: | |
| type: Utilization | |
| averageUtilization: {{ .Values.autoscaling.targetCPUUtilizationPercentage }} | |
| {{- end }} | |
| {{- if .Values.autoscaling.targetMemoryUtilizationPercentage }} | |
| - type: Resource | |
| resource: | |
| name: memory | |
| target: | |
| type: Utilization | |
| averageUtilization: {{ .Values.autoscaling.targetMemoryUtilizationPercentage }} | |
| {{- end }} | |
| {{- end }} |
🤖 Prompt for AI Agents
In
@packages/system/linstor-scheduler/charts/linstor-scheduler/templates/hpa.yaml
around lines 15 - 31, When autoscaling is enabled ensure the HPA metrics array
is never empty by adding validation around the metrics block: check
.Values.autoscaling.targetCPUUtilizationPercentage and
.Values.autoscaling.targetMemoryUtilizationPercentage and if both are unset emit
a Helm template failure (using fail or required) or supply a sane default
metric; update the conditional around the metrics array (the metrics block that
references .Values.autoscaling.targetCPUUtilizationPercentage and
.Values.autoscaling.targetMemoryUtilizationPercentage) so that when
.Values.autoscaling.enabled is true at least one metric is present or the
template fails with a clear message.
| update: | ||
| rm -rf charts | ||
| helm repo add piraeus-charts https://piraeus.io/helm-charts/ | ||
| helm repo update piraeus-charts | ||
| helm pull piraeus-charts/linstor-scheduler --untar --untardir charts |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd packages/system/linstor-scheduler && cat -n MakefileRepository: cozystack/cozystack
Length of output: 418
🌐 Web query:
Piraeus Datastore linstor-scheduler Helm chart repository URL official
💡 Result:
The official Piraeus Charts Helm repo URL is:
(Chart sources are also hosted in the piraeusdatastore/helm-charts GitHub repo.) [2]
Sources:
[1] Piraeus Datastore docs — Helm usage.
[2] piraeusdatastore/helm-charts GitHub repository.
🌐 Web query:
Helm chart version pinning best practices reproducible builds
💡 Result:
Best-practice checklist for pinning Helm charts to achieve reproducible builds:
- Commit Chart.yaml and Chart.lock (or helmfile.lock) to version control; use helm dependency update to generate the lock and helm dependency build to reproduce charts from it. [1][3][4]
- Prefer exact versions or constrained semver in Chart.yaml for dependencies (avoid wildcard/latest). Use Chart.lock (or helmfile.lock) as the authoritative pinned snapshot. [1][3][4]
- Use OCI registries and pin to digests (sha256) when possible (oci://...@sha256:...). Store provenance files if available. This prevents tag re-use or mutable-tag drift. [2][5]
- Mirror or host critical charts in a private/internal registry or artifact repository (or vendor chart tgz into your repo) so CI/CD doesn’t depend on external availability. Commit vendored tgz only when you need absolute reproducibility. [3][4]
- Make lockfile regeneration an explicit, tested CI step: run helm repo update; helm dependency update locally; run tests; then commit updated lockfiles. Don’t auto-update locks in production pipelines. [1][3]
- Pin associated container images (values.yaml) to digests as well, and verify chart contents (checksums/provenance) during pipeline. [2][5]
- Audit and rotate: periodically review/upstream updates in a controlled cadence (PRs, testing), record the date/version of the roll-forward in changelogs. [3]
Sources: Helm docs (dependency build, Chart.lock, OCI registries); Helmfile docs (lockfile best practices); community guidance (StackOverflow, Chainguard examples). [1][2][3][4][5]
References:
[1] Helm — helm dependency build / Chart.lock.
[2] Helm — Use OCI-based registries (digests & push).
[3] Helmfile docs — lock files & reproducible deployments.
[4] StackOverflow — commit Chart.lock recommendation.
[5] Chainguard — example advising digest pinning.
Pin the Helm chart version for reproducible builds.
The helm pull command on line 10 lacks a --version flag, allowing different chart versions to be pulled across executions. This breaks build reproducibility and introduces supply chain risk when vendoring charts.
Recommend adding --version to pin to a specific chart release:
📦 Fix version pinning
update:
rm -rf charts
helm repo add piraeus-charts https://piraeus.io/helm-charts/
helm repo update piraeus-charts
- helm pull piraeus-charts/linstor-scheduler --untar --untardir charts
+ helm pull piraeus-charts/linstor-scheduler --version <VERSION> --untar --untardir chartsReplace <VERSION> with the specific chart version (e.g., 1.2.3).
Add .PHONY declaration for the update target.
The Makefile lacks a .PHONY: update declaration for a target that does not produce a file. This is a best-practice improvement.
🔧 Add .PHONY declaration
+.PHONY: update
update:
rm -rf charts
helm repo add piraeus-charts https://piraeus.io/helm-charts/
helm repo update piraeus-charts
helm pull piraeus-charts/linstor-scheduler --untar --untardir charts📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| update: | |
| rm -rf charts | |
| helm repo add piraeus-charts https://piraeus.io/helm-charts/ | |
| helm repo update piraeus-charts | |
| helm pull piraeus-charts/linstor-scheduler --untar --untardir charts | |
| .PHONY: update | |
| update: | |
| rm -rf charts | |
| helm repo add piraeus-charts https://piraeus.io/helm-charts/ | |
| helm repo update piraeus-charts | |
| helm pull piraeus-charts/linstor-scheduler --untar --untardir charts |
🤖 Prompt for AI Agents
In @packages/system/linstor-scheduler/Makefile around lines 6 - 10, The
Makefile's update target should be made reproducible and declared phony: modify
the update target (symbol: update) to pass an explicit chart version to the helm
pull command by adding a --version <VERSION> argument (replace <VERSION> with
the pinned release like 1.2.3) so the pulled piraeus-charts/linstor-scheduler
chart is deterministic, and add a .PHONY: update declaration (symbol: .PHONY and
update) above the target so make won't treat the target as a file.
|
/retest |
## What this PR does Add linstor-scheduler-extender for optimal pod placement on nodes with LINSTOR storage. ### How it works The linstor-scheduler consists of two components: #### 1. Scheduler Extender A custom Kubernetes scheduler that works alongside the default kube-scheduler. When a pod requests LINSTOR-backed storage, the scheduler extender communicates with the LINSTOR controller to find nodes that have local replicas of the requested volumes, prioritizing placement on nodes with existing data to minimize network traffic. #### 2. Admission Webhook A MutatingWebhookConfiguration that automatically sets `schedulerName: linstor-scheduler` on pods that use LINSTOR CSI driver volumes. This ensures that only pods with LINSTOR PVCs are routed through the custom scheduler, while all other pods continue using the default scheduler. The webhook inspects pod creation requests and checks if any of the pod's PVCs use a StorageClass with the LINSTOR CSI provisioner (`linstor.csi.linbit.com`). If so, it mutates the pod spec to use the linstor-scheduler. ### Components - `linstor-scheduler` package in `packages/system/linstor-scheduler/` - PackageSource definition in `packages/core/platform/sources/linstor-scheduler.yaml` - Integration into `paas-full` and `distro-full` bundles ### Upstream PRs - piraeusdatastore/helm-charts#67 — fix: KubeSchedulerConfiguration v1 support for Kubernetes 1.25+ - piraeusdatastore/helm-charts#68 — feat: admission webhook support with cert-manager - piraeusdatastore/helm-charts#69 — fix chart template issues ### Release note ```release-note [linstor] Add linstor-scheduler for optimal pod placement on nodes with local LINSTOR replicas. Includes admission webhook that automatically routes pods using LINSTOR CSI volumes to the custom scheduler. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added LINSTOR scheduler component to the platform, packaged as a Helm chart and installable package. * Includes admission webhook, scheduler extender, autoscaling support, pod disruption budget, RBAC and service account integration, and Helm test hooks. * **Documentation** * Added chart README and default configuration values for easy deployment and customization. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## What this PR does Add linstor-scheduler-extender for optimal pod placement on nodes with LINSTOR storage. ### How it works The linstor-scheduler consists of two components: #### 1. Scheduler Extender A custom Kubernetes scheduler that works alongside the default kube-scheduler. When a pod requests LINSTOR-backed storage, the scheduler extender communicates with the LINSTOR controller to find nodes that have local replicas of the requested volumes, prioritizing placement on nodes with existing data to minimize network traffic. #### 2. Admission Webhook A MutatingWebhookConfiguration that automatically sets `schedulerName: linstor-scheduler` on pods that use LINSTOR CSI driver volumes. This ensures that only pods with LINSTOR PVCs are routed through the custom scheduler, while all other pods continue using the default scheduler. The webhook inspects pod creation requests and checks if any of the pod's PVCs use a StorageClass with the LINSTOR CSI provisioner (`linstor.csi.linbit.com`). If so, it mutates the pod spec to use the linstor-scheduler. ### Components - `linstor-scheduler` package in `packages/system/linstor-scheduler/` - PackageSource definition in `packages/core/platform/sources/linstor-scheduler.yaml` - Integration into `paas-full` and `distro-full` bundles ### Upstream PRs - piraeusdatastore/helm-charts#67 — fix: KubeSchedulerConfiguration v1 support for Kubernetes 1.25+ - piraeusdatastore/helm-charts#68 — feat: admission webhook support with cert-manager - piraeusdatastore/helm-charts#69 — fix chart template issues ### Release note ```release-note [linstor] Add linstor-scheduler for optimal pod placement on nodes with local LINSTOR replicas. Includes admission webhook that automatically routes pods using LINSTOR CSI volumes to the custom scheduler. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added LINSTOR scheduler component to the platform, packaged as a Helm chart and installable package. * Includes admission webhook, scheduler extender, autoscaling support, pod disruption budget, RBAC and service account integration, and Helm test hooks. * **Documentation** * Added chart README and default configuration values for easy deployment and customization. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## What this PR does Add linstor-scheduler-extender for optimal pod placement on nodes with LINSTOR storage. ### How it works The linstor-scheduler consists of two components: #### 1. Scheduler Extender A custom Kubernetes scheduler that works alongside the default kube-scheduler. When a pod requests LINSTOR-backed storage, the scheduler extender communicates with the LINSTOR controller to find nodes that have local replicas of the requested volumes, prioritizing placement on nodes with existing data to minimize network traffic. #### 2. Admission Webhook A MutatingWebhookConfiguration that automatically sets `schedulerName: linstor-scheduler` on pods that use LINSTOR CSI driver volumes. This ensures that only pods with LINSTOR PVCs are routed through the custom scheduler, while all other pods continue using the default scheduler. The webhook inspects pod creation requests and checks if any of the pod's PVCs use a StorageClass with the LINSTOR CSI provisioner (`linstor.csi.linbit.com`). If so, it mutates the pod spec to use the linstor-scheduler. ### Components - `linstor-scheduler` package in `packages/system/linstor-scheduler/` - PackageSource definition in `packages/core/platform/sources/linstor-scheduler.yaml` - Integration into `paas-full` and `distro-full` bundles ### Upstream PRs - piraeusdatastore/helm-charts#67 — fix: KubeSchedulerConfiguration v1 support for Kubernetes 1.25+ - piraeusdatastore/helm-charts#68 — feat: admission webhook support with cert-manager - piraeusdatastore/helm-charts#69 — fix chart template issues ### Release note ```release-note [linstor] Add linstor-scheduler for optimal pod placement on nodes with local LINSTOR replicas. Includes admission webhook that automatically routes pods using LINSTOR CSI volumes to the custom scheduler. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added LINSTOR scheduler component to the platform, packaged as a Helm chart and installable package. * Includes admission webhook, scheduler extender, autoscaling support, pod disruption budget, RBAC and service account integration, and Helm test hooks. * **Documentation** * Added chart README and default configuration values for easy deployment and customization. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
What this PR does
Add linstor-scheduler-extender for optimal pod placement on nodes with LINSTOR storage.
How it works
The linstor-scheduler consists of two components:
1. Scheduler Extender
A custom Kubernetes scheduler that works alongside the default kube-scheduler. When a pod requests LINSTOR-backed storage, the scheduler extender communicates with the LINSTOR controller to find nodes that have local replicas of the requested volumes, prioritizing placement on nodes with existing data to minimize network traffic.
2. Admission Webhook
A MutatingWebhookConfiguration that automatically sets
schedulerName: linstor-scheduleron pods that use LINSTOR CSI driver volumes. This ensures that only pods with LINSTOR PVCs are routed through the custom scheduler, while all other pods continue using the default scheduler.The webhook inspects pod creation requests and checks if any of the pod's PVCs use a StorageClass with the LINSTOR CSI provisioner (
linstor.csi.linbit.com). If so, it mutates the pod spec to use the linstor-scheduler.Components
linstor-schedulerpackage inpackages/system/linstor-scheduler/packages/core/platform/sources/linstor-scheduler.yamlpaas-fullanddistro-fullbundlesUpstream PRs
Release note
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.