[scheduler] Make cozystack-scheduler storage-aware via LINSTOR extender#2330
[scheduler] Make cozystack-scheduler storage-aware via LINSTOR extender#2330
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
✅ Files skipped from review due to trivial changes (7)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds Helm-based scheduler extender support and LINSTOR integration: bumps cozystack-scheduler chart versions, introduces an Changes
Sequence Diagram(s)sequenceDiagram
participant PodCreator
participant KubernetesAPI
participant KubeScheduler
participant LinstorExtenderSvc
participant LinstorExtenderPod
PodCreator->>KubernetesAPI: create Pod
KubernetesAPI->>KubeScheduler: schedule Pod
KubeScheduler->>LinstorExtenderSvc: POST /filter/prioritize (extender request)
LinstorExtenderSvc->>LinstorExtenderPod: forward request (ClusterIP)
LinstorExtenderPod-->>LinstorExtenderSvc: response (scores/filters)
LinstorExtenderSvc-->>KubeScheduler: return extender results
KubeScheduler->>KubernetesAPI: bind Pod to Node
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Code Review
This pull request introduces support for scheduler extenders in the cozystack-scheduler and adds a component label to the linstor-scheduler deployment. The review identified two critical issues: the image digest in the cozystack-scheduler values file was not updated for the new version, and the template path in the test suite configuration is incorrect, which will prevent the tests from running successfully.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/system/cozystack-scheduler/tests/configmap_test.yaml (1)
19-27: Consider adding assertion for full extender structure.The regex assertions verify
urlPrefixandignorablebut don't validate other critical fields likefilterVerborprioritizeVerb. Consider adding assertions for these to catch rendering issues.Additional assertions
- matchRegex: path: data["scheduler-config.yaml"] pattern: "ignorable: true" + - matchRegex: + path: data["scheduler-config.yaml"] + pattern: "filterVerb: filter" + - matchRegex: + path: data["scheduler-config.yaml"] + pattern: "prioritizeVerb: prioritize"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/system/cozystack-scheduler/tests/configmap_test.yaml` around lines 19 - 27, The test only asserts urlPrefix and ignorable in data["scheduler-config.yaml"]; add assertions that validate the extender's other required fields (e.g., filterVerb and prioritizeVerb) and, if possible, the full extender object structure to catch rendering regressions: update the assertions that target data["scheduler-config.yaml"] to include matchRegex checks for "filterVerb: <expected>", "prioritizeVerb: <expected>", and/or a stricter match for the extender block (or a JSON/YAML equality/assertContains for the extender) so ConfigMap content produced by the template includes the complete extender configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/system/cozystack-scheduler/tests/configmap_test.yaml`:
- Around line 8-18: The test's top-level values key "cozy-cozystack-scheduler"
doesn't match the chart key used by the outer values.yaml, so the extenders
block won't be applied; update the key in this test (replace
"cozy-cozystack-scheduler" with whichever canonical chart key you decided in the
outer fix — e.g., "cozystack-scheduler" if using the directory name) so the
extenders, httpTimeout, nodeCacheCapable, etc. are correctly routed to the
subchart.
- Around line 3-4: Update the template path string in the tests config map
entry: replace the incorrect subchart name "cozy-cozystack-scheduler" with the
actual subchart name "cozystack-scheduler" in the templates list inside
configmap_test.yaml so the template reference matches the real chart directory
and the test can locate templates/configmap.yaml.
In `@packages/system/linstor-scheduler/templates/extender-service.yaml`:
- Line 4: The templated metadata.name line currently emits an unquoted YAML
scalar ("name: {{ include "linstor-scheduler.fullname" . }}-extender") which
breaks linters; update the metadata.name value to be a single quoted scalar that
concatenates the template and suffix (i.e., quote the entire expansion including
{{ include "linstor-scheduler.fullname" . }}-extender) so the YAML parser treats
it as one string; locate the template expression using include
"linstor-scheduler.fullname" . and the "-extender" suffix and wrap the whole RHS
in quotes.
---
Nitpick comments:
In `@packages/system/cozystack-scheduler/tests/configmap_test.yaml`:
- Around line 19-27: The test only asserts urlPrefix and ignorable in
data["scheduler-config.yaml"]; add assertions that validate the extender's other
required fields (e.g., filterVerb and prioritizeVerb) and, if possible, the full
extender object structure to catch rendering regressions: update the assertions
that target data["scheduler-config.yaml"] to include matchRegex checks for
"filterVerb: <expected>", "prioritizeVerb: <expected>", and/or a stricter match
for the extender block (or a JSON/YAML equality/assertContains for the extender)
so ConfigMap content produced by the template includes the complete extender
configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d33b3de7-d9aa-498a-8639-f9763211bbb9
📒 Files selected for processing (10)
packages/system/cozystack-scheduler/Chart.yamlpackages/system/cozystack-scheduler/charts/cozystack-scheduler/Chart.yamlpackages/system/cozystack-scheduler/charts/cozystack-scheduler/templates/configmap.yamlpackages/system/cozystack-scheduler/charts/cozystack-scheduler/values.yamlpackages/system/cozystack-scheduler/tests/configmap_test.yamlpackages/system/cozystack-scheduler/values.yamlpackages/system/linstor-scheduler/Makefilepackages/system/linstor-scheduler/charts/linstor-scheduler/templates/deployment.yamlpackages/system/linstor-scheduler/patches/add-scheduler-component-label.patchpackages/system/linstor-scheduler/templates/extender-service.yaml
|
Hey @lllamnyp could you fix DCO please? |
ac4d777 to
d3d71af
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/system/linstor-scheduler/templates/extender-service.yaml (1)
4-4:⚠️ Potential issue | 🟠 MajorQuote the templated
metadata.nameto fix YAML lint error.The unquoted template expression is causing YAMLlint to fail. Wrap the entire value in single quotes.
🔧 Proposed fix
- name: {{ include "linstor-scheduler.fullname" . }}-extender + name: '{{ include "linstor-scheduler.fullname" . }}-extender'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/system/linstor-scheduler/templates/extender-service.yaml` at line 4, The YAML lint error is due to an unquoted templated metadata.name; wrap the entire value in single quotes so the template expression is treated as a string (change the metadata.name value that uses {{ include "linstor-scheduler.fullname" . }}-extender to be quoted, i.e. '...'), updating the metadata.name field in the extender-service.yaml template where the include "linstor-scheduler.fullname" . }}-extender is used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/system/linstor-scheduler/templates/extender-service.yaml`:
- Line 4: The YAML lint error is due to an unquoted templated metadata.name;
wrap the entire value in single quotes so the template expression is treated as
a string (change the metadata.name value that uses {{ include
"linstor-scheduler.fullname" . }}-extender to be quoted, i.e. '...'), updating
the metadata.name field in the extender-service.yaml template where the include
"linstor-scheduler.fullname" . }}-extender is used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5a2303fd-c0e5-455a-b7c2-c9f303b88c7b
📒 Files selected for processing (10)
packages/system/cozystack-scheduler/Chart.yamlpackages/system/cozystack-scheduler/charts/cozystack-scheduler/Chart.yamlpackages/system/cozystack-scheduler/charts/cozystack-scheduler/templates/configmap.yamlpackages/system/cozystack-scheduler/charts/cozystack-scheduler/values.yamlpackages/system/cozystack-scheduler/tests/configmap_test.yamlpackages/system/cozystack-scheduler/values.yamlpackages/system/linstor-scheduler/Makefilepackages/system/linstor-scheduler/charts/linstor-scheduler/templates/deployment.yamlpackages/system/linstor-scheduler/patches/add-scheduler-component-label.patchpackages/system/linstor-scheduler/templates/extender-service.yaml
✅ Files skipped from review due to trivial changes (7)
- packages/system/linstor-scheduler/patches/add-scheduler-component-label.patch
- packages/system/linstor-scheduler/Makefile
- packages/system/linstor-scheduler/charts/linstor-scheduler/templates/deployment.yaml
- packages/system/cozystack-scheduler/charts/cozystack-scheduler/Chart.yaml
- packages/system/cozystack-scheduler/values.yaml
- packages/system/cozystack-scheduler/tests/configmap_test.yaml
- packages/system/cozystack-scheduler/Chart.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/system/cozystack-scheduler/charts/cozystack-scheduler/templates/configmap.yaml
- packages/system/cozystack-scheduler/charts/cozystack-scheduler/values.yaml
d3d71af to
e440e8c
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/system/linstor-scheduler/templates/extender-service.yaml (1)
4-4:⚠️ Potential issue | 🟠 MajorQuote the templated
metadata.nameto fix YAML parser errors.The unquoted template expression breaks YAML linters and parsers. Wrap the entire value in quotes.
🔧 Proposed fix
- name: {{ include "linstor-scheduler.fullname" . }}-extender + name: '{{ include "linstor-scheduler.fullname" . }}-extender'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/system/linstor-scheduler/templates/extender-service.yaml` at line 4, The metadata.name template value is unquoted and can break YAML parsers; update the metadata.name entry that uses the include "linstor-scheduler.fullname" template plus the -extender suffix by wrapping the entire templated expression and suffix in double quotes so the whole value is a quoted string (i.e., quote the name field containing the include "linstor-scheduler.fullname" . template and the -extender suffix).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/system/linstor-scheduler/templates/extender-service.yaml`:
- Line 4: The metadata.name template value is unquoted and can break YAML
parsers; update the metadata.name entry that uses the include
"linstor-scheduler.fullname" template plus the -extender suffix by wrapping the
entire templated expression and suffix in double quotes so the whole value is a
quoted string (i.e., quote the name field containing the include
"linstor-scheduler.fullname" . template and the -extender suffix).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a074d655-5a1c-400f-8adc-f971a9bc772b
📒 Files selected for processing (11)
packages/system/cozystack-scheduler/Chart.yamlpackages/system/cozystack-scheduler/Makefilepackages/system/cozystack-scheduler/charts/cozystack-scheduler/Chart.yamlpackages/system/cozystack-scheduler/charts/cozystack-scheduler/templates/configmap.yamlpackages/system/cozystack-scheduler/charts/cozystack-scheduler/values.yamlpackages/system/cozystack-scheduler/tests/configmap_test.yamlpackages/system/cozystack-scheduler/values.yamlpackages/system/linstor-scheduler/Makefilepackages/system/linstor-scheduler/charts/linstor-scheduler/templates/deployment.yamlpackages/system/linstor-scheduler/patches/add-scheduler-component-label.patchpackages/system/linstor-scheduler/templates/extender-service.yaml
✅ Files skipped from review due to trivial changes (8)
- packages/system/cozystack-scheduler/Makefile
- packages/system/linstor-scheduler/patches/add-scheduler-component-label.patch
- packages/system/linstor-scheduler/charts/linstor-scheduler/templates/deployment.yaml
- packages/system/linstor-scheduler/Makefile
- packages/system/cozystack-scheduler/values.yaml
- packages/system/cozystack-scheduler/charts/cozystack-scheduler/values.yaml
- packages/system/cozystack-scheduler/tests/configmap_test.yaml
- packages/system/cozystack-scheduler/charts/cozystack-scheduler/Chart.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/system/cozystack-scheduler/Chart.yaml
- packages/system/cozystack-scheduler/charts/cozystack-scheduler/templates/configmap.yaml
lexfrei
left a comment
There was a problem hiding this comment.
Reviewed the storage-aware scheduling changes. Found issues in wrapper chart templates that will prevent the extender integration from working correctly.
| apiVersion: v1 | ||
| kind: Service | ||
| metadata: | ||
| name: {{ include "linstor-scheduler.fullname" . }}-extender |
There was a problem hiding this comment.
linstor-scheduler.fullname is a subchart helper, but this template lives in the wrapper chart. The . context here is the parent chart — .Values.fullnameOverride resolves to the parent's top-level value (undefined), not the subchart's linstor-scheduler.fullnameOverride: linstor-scheduler.
Result: Service name renders as something like RELEASE-cozy-linstor-scheduler-extender instead of linstor-scheduler-extender. Similarly, linstor-scheduler.selectorLabels on line 16 will produce labels that don't match the subchart deployment's pods.
The extender Service will never route traffic to the actual pods. Consider either hardcoding the name/selectors or moving this Service into the subchart via a patch (consistent with how add-scheduler-component-label.patch works).
| @@ -0,0 +1,10 @@ | |||
| cozy-cozystack-scheduler: | |||
There was a problem hiding this comment.
This wrapper default enables the LINSTOR extender for all cozystack-scheduler deployments, including clusters without linstor-scheduler. While ignorable: true suppresses the scheduling error, it only does so after httpTimeout (10s) elapses — every scheduling cycle on clusters without LINSTOR will incur this delay.
Consider defaulting extenders: [] in the wrapper values and letting clusters with LINSTOR opt in explicitly.
| suite: scheduler configmap tests | ||
|
|
||
| templates: | ||
| - charts/cozy-cozystack-scheduler/templates/configmap.yaml |
There was a problem hiding this comment.
Template path charts/cozy-cozystack-scheduler/templates/configmap.yaml doesn't match the actual subchart directory. The subchart is downloaded by make update into charts/cozystack-scheduler/ (without cozy- prefix). This test will fail with "template not found".
e440e8c to
c9d42ea
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/system/linstor-scheduler/Makefile (1)
9-15: Optional: splitupdateinto smaller targets to satisfy checkmakeLine 9 now trips
maxbodylength; extracting patch/apply steps into helper targets will keep lint clean and improve readability.Proposed refactor
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 - patch --no-backup-if-mismatch -p4 < patches/disable-ca-key-rotation.patch - patch --no-backup-if-mismatch -p4 < patches/add-scheduler-component-label.patch + $(MAKE) update-chart + $(MAKE) apply-patches + +update-chart: + 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 + +apply-patches: + patch --no-backup-if-mismatch -p4 < patches/disable-ca-key-rotation.patch + patch --no-backup-if-mismatch -p4 < patches/add-scheduler-component-label.patch🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/system/linstor-scheduler/Makefile` around lines 9 - 15, The current Makefile target "update" is too long and triggers checkmake's maxbodylength; split it into smaller targets (e.g., add-repo, pull-chart, untar-chart, apply-patches or a charts-prep and charts-patch helper) and make "update" depend on them so each target contains only a couple of commands. Move the helm repo add/helm repo update into an "add-repo" target, the helm pull into a "pull-chart" (or "untar-chart") target, and the two patch invocations into an "apply-patches" target (referencing the existing patch files disable-ca-key-rotation.patch and add-scheduler-component-label.patch), then update the "update" target to simply depend on these helper targets to satisfy checkmake and improve readability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/platform/templates/bundles/system.yaml`:
- Line 133: The current include always selects the "linstor" variant for
cozystack.cozystack-scheduler which forces LINSTOR packages for all system
variants; change the template to choose the scheduler variant conditionally
(e.g., use a template variable or variant membership check) so that
cozystack.cozystack-scheduler is passed "linstor" only when the current system
variant or a flag indicates LINSTOR should be enabled (otherwise pass the
default variant or omit the variant), by updating the include expression that
references cozystack.cozystack-scheduler/"linstor" to a conditional expression
using your template language (e.g., if/else or ternary) based on a flag like
enable_linstor or membership in a list of variants such as isp-hosted.
---
Nitpick comments:
In `@packages/system/linstor-scheduler/Makefile`:
- Around line 9-15: The current Makefile target "update" is too long and
triggers checkmake's maxbodylength; split it into smaller targets (e.g.,
add-repo, pull-chart, untar-chart, apply-patches or a charts-prep and
charts-patch helper) and make "update" depend on them so each target contains
only a couple of commands. Move the helm repo add/helm repo update into an
"add-repo" target, the helm pull into a "pull-chart" (or "untar-chart") target,
and the two patch invocations into an "apply-patches" target (referencing the
existing patch files disable-ca-key-rotation.patch and
add-scheduler-component-label.patch), then update the "update" target to simply
depend on these helper targets to satisfy checkmake and improve readability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1d376fc3-a49e-415c-9bb5-06a8ec60d82b
📒 Files selected for processing (14)
packages/core/platform/sources/cozystack-scheduler.yamlpackages/core/platform/templates/bundles/system.yamlpackages/system/cozystack-scheduler/Chart.yamlpackages/system/cozystack-scheduler/Makefilepackages/system/cozystack-scheduler/charts/cozystack-scheduler/Chart.yamlpackages/system/cozystack-scheduler/charts/cozystack-scheduler/templates/configmap.yamlpackages/system/cozystack-scheduler/charts/cozystack-scheduler/values.yamlpackages/system/cozystack-scheduler/tests/configmap_test.yamlpackages/system/cozystack-scheduler/values-linstor.yamlpackages/system/linstor-scheduler/Makefilepackages/system/linstor-scheduler/charts/linstor-scheduler/templates/deployment.yamlpackages/system/linstor-scheduler/patches/add-scheduler-component-label.patchpackages/system/linstor-scheduler/templates/extender-service.yamlpackages/system/linstor-scheduler/tests/extender-service_test.yaml
✅ Files skipped from review due to trivial changes (6)
- packages/system/linstor-scheduler/patches/add-scheduler-component-label.patch
- packages/system/cozystack-scheduler/Makefile
- packages/system/cozystack-scheduler/charts/cozystack-scheduler/Chart.yaml
- packages/system/cozystack-scheduler/values-linstor.yaml
- packages/system/linstor-scheduler/charts/linstor-scheduler/templates/deployment.yaml
- packages/system/cozystack-scheduler/tests/configmap_test.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/system/cozystack-scheduler/Chart.yaml
- packages/system/cozystack-scheduler/charts/cozystack-scheduler/templates/configmap.yaml
- packages/system/cozystack-scheduler/charts/cozystack-scheduler/values.yaml
| {{include "cozystack.platform.package.default" (list "cozystack.backupstrategy-controller" $) }} | ||
| {{include "cozystack.platform.package.default" (list "cozystack.backup-controller" $) }} | ||
| {{include "cozystack.platform.package.default" (list "cozystack.cozystack-scheduler" $) }} | ||
| {{include "cozystack.platform.package" (list "cozystack.cozystack-scheduler" "linstor" $) }} |
There was a problem hiding this comment.
Conditionally select the linstor scheduler variant to avoid forcing LINSTOR on all system variants.
At Line 133, the bundle now always uses cozystack.cozystack-scheduler variant linstor. Since that variant depends on cozystack.linstor-scheduler (which itself depends on cozystack.linstor), this can unintentionally pull or block LINSTOR-related packages for variants like isp-hosted that don’t explicitly include LINSTOR in this bundle.
Suggested change
-{{include "cozystack.platform.package" (list "cozystack.cozystack-scheduler" "linstor" $) }}
+{{- if or (eq .Values.bundles.system.variant "isp-full") (eq .Values.bundles.system.variant "isp-full-generic") }}
+{{include "cozystack.platform.package" (list "cozystack.cozystack-scheduler" "linstor" $) }}
+{{- else }}
+{{include "cozystack.platform.package.default" (list "cozystack.cozystack-scheduler" $) }}
+{{- end }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/platform/templates/bundles/system.yaml` at line 133, The
current include always selects the "linstor" variant for
cozystack.cozystack-scheduler which forces LINSTOR packages for all system
variants; change the template to choose the scheduler variant conditionally
(e.g., use a template variable or variant membership check) so that
cozystack.cozystack-scheduler is passed "linstor" only when the current system
variant or a flag indicates LINSTOR should be enabled (otherwise pass the
default variant or omit the variant), by updating the include expression that
references cozystack.cozystack-scheduler/"linstor" to a conditional expression
using your template language (e.g., if/else or ternary) based on a flag like
enable_linstor or membership in a list of variants such as isp-hosted.
lexfrei
left a comment
There was a problem hiding this comment.
Previous review comments are addressed — thanks for the rework. One remaining issue flagged by coderabbit at system.yaml:133 that I want to second with code references.
| {{include "cozystack.platform.package.default" (list "cozystack.backupstrategy-controller" $) }} | ||
| {{include "cozystack.platform.package.default" (list "cozystack.backup-controller" $) }} | ||
| {{include "cozystack.platform.package.default" (list "cozystack.cozystack-scheduler" $) }} | ||
| {{include "cozystack.platform.package" (list "cozystack.cozystack-scheduler" "linstor" $) }} |
There was a problem hiding this comment.
This line sits outside the variant-specific blocks (the isp-hosted block ends at line 32, the isp-full-generic block at line 95), so the linstor variant is selected for every system bundle — including isp-hosted.
isp-hosted does not invoke cozystack.platform.system.common-packages (that helper is where cozystack.linstor-scheduler is declared — see _helpers.tpl:70), so the cozystack.linstor-scheduler Package is never created in that bundle.
The linstor variant declared in sources/cozystack-scheduler.yaml:22 has dependsOn: cozystack.linstor-scheduler. buildDependsOn in internal/operator/package_reconciler.go:509 returns dependent Package %s not found when a variant's dependsOn target is missing, which will keep cozystack-scheduler permanently unreconciled on isp-hosted clusters. Before this PR the default variant had no such dependency and installed cleanly.
Suggest gating the linstor variant on the bundle variant (isp-full / isp-full-generic) and falling back to cozystack.platform.package.default for isp-hosted, along the lines of coderabbit's suggestion above.
Expose the existing linstor-scheduler-extender sidecar as a ClusterIP Service and configure cozystack-scheduler to call it during the scheduling cycle. Pods with both a SchedulingClass and LINSTOR PVCs now get storage-locality-aware placement. - Add extender Service (port 8099) for linstor-scheduler - Patch vendored deployment to label pods for Service selector - Bump cozystack-scheduler to v0.3.0 (configurable extenders) - Add "linstor" PackageSource variant with extender values - Default variant ships without extender for non-LINSTOR clusters - Select linstor variant in system bundle - Add helm-unittest tests for both packages Ref: #2328 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> Signed-off-by: Timofei Larkin <[email protected]>
c9d42ea to
37229aa
Compare
lexfrei
left a comment
There was a problem hiding this comment.
All previously raised concerns are addressed in the latest revision:
- Extender Service no longer relies on subchart helpers from the wrapper context — name and selectors are hardcoded and covered by a helm-unittest cross-check against the deployment pod labels.
- The LINSTOR extender is now opt-in via the
linstorvariant (values-linstor.yaml) instead of being the default for everycozystack-schedulerinstall. isp-hostedbundle uses thedefaultvariant (nodependsOn: cozystack.linstor-scheduler), so reconciliation works on clusters without LINSTOR.isp-fullandisp-full-genericboth includelinstor-schedulerviacommon-packages, so thelinstorvariants dependency resolves cleanly there.
Minor non-blocker: with httpTimeout: 10s + ignorable: true, scheduling cycles can incur a 10s delay if the extender Service is briefly unreachable — acceptable since the extender is only consulted for pods with a SchedulingClass.
LGTM.
What this PR does
Makes
cozystack-schedulerLINSTOR storage-aware by configuring it to call the existinglinstor-scheduler-extenderas a scheduler extender. This fixes #2328 (phase 1): pods with both aSchedulingClassand LINSTOR PVCs now get storage-locality-aware placement instead of bypassing LINSTOR's filter/prioritize logic.Changes:
app.kubernetes.io/component: schedulerlabel for clean Service targetingextenderssupport; set the LINSTOR extender URL in wrapper valuesRelease note
Summary by CodeRabbit
New Features
Chores
Bundles