Conversation
📝 WalkthroughWalkthroughTwo Tekton configuration files are updated to modify compute resource specifications. One file restructures resources from task-level to step-level configuration via taskRunSpecs, while the other adds explicit resource overrides for pipeline tasks. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan for PR comments
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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.tekton/odh-workbench-codeserver-datascience-cpu-py312-ubi9-push.yaml (1)
191-198:⚠️ Potential issue | 🟠 MajorUse
stepSpecsforbuild-imagesinstead of task-levelcomputeResources.Task-level
computeResourcesallocate resources as a total budget divided equally across all the task's steps. Since thebuildah-remote-oci-tatask has multiple steps, the actual build container will receive only a fraction of the8 CPU / 32Giallocation, which will not fix the OOM issue. Your pull-request YAML already uses the correct pattern withstepSpecstargeting thebuildstep—apply the same fix to this push file.🔧 Suggested change
- pipelineTaskName: build-images - computeResources: - requests: - cpu: "8" - memory: "32Gi" - limits: - cpu: "8" - memory: "32Gi" + stepSpecs: + - name: build + computeResources: + requests: + cpu: "8" + memory: "32Gi" + limits: + cpu: "8" + memory: "32Gi"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.tekton/odh-workbench-codeserver-datascience-cpu-py312-ubi9-push.yaml around lines 191 - 198, The pipelineTaskName "build-images" currently sets task-level computeResources which are split across steps; replace that block with a stepSpecs entry for the build step of the buildah-remote-oci-ta task so the actual build container gets the full resources: remove the task-level computeResources under pipelineTaskName "build-images" and add a stepSpecs mapping that targets the step named "build" and supplies requests.cpu "8", requests.memory "32Gi", limits.cpu "8", limits.memory "32Gi" so the build step (not the whole task) receives the intended allocation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.tekton/odh-workbench-codeserver-datascience-cpu-py312-ubi9-push.yaml:
- Around line 191-198: The pipelineTaskName "build-images" currently sets
task-level computeResources which are split across steps; replace that block
with a stepSpecs entry for the build step of the buildah-remote-oci-ta task so
the actual build container gets the full resources: remove the task-level
computeResources under pipelineTaskName "build-images" and add a stepSpecs
mapping that targets the step named "build" and supplies requests.cpu "8",
requests.memory "32Gi", limits.cpu "8", limits.memory "32Gi" so the build step
(not the whole task) receives the intended allocation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8b5c8460-0153-4154-a8f0-1d9934ab49ea
📒 Files selected for processing (2)
.tekton/odh-workbench-codeserver-datascience-cpu-py312-ubi9-pull-request.yaml.tekton/odh-workbench-codeserver-datascience-cpu-py312-ubi9-push.yaml
|
/kfbuild codeserver/ubi9-python-3.12 |
76698e2 to
48b35bb
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.tekton/odh-workbench-codeserver-datascience-cpu-py312-ubi9-odh-main-push.yaml (1)
180-198:⚠️ Potential issue | 🟠 MajorMigrate push PipelineRun to step-level
computeResourcesoverrides to match the paired pull-request PipelineRun.The paired pull-request PipelineRun was switched to
stepSpecsbecause task-levelcomputeResourcesmay be ignored when that beta feature is disabled. This push PipelineRun still relies on task-level overrides only. Since Pipelines-as-Code triggers separate PipelineRun definitions per event type (pull_request vs push), and step-level overrides must be specified in each to guarantee they apply, this file needs the same migration.Update
taskRunSpecsto usestepSpecs[].computeResourcesfor bothprefetch-dependenciesandbuild-imagestasks to match the PR variant.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.tekton/odh-workbench-codeserver-datascience-cpu-py312-ubi9-odh-main-push.yaml around lines 180 - 198, Update the taskRunSpecs entries so the computeResources overrides are applied at the step level: for both pipelineTaskName values "prefetch-dependencies" and "build-images" replace the task-level computeResources block with a stepSpecs array that contains a stepSpecs[].computeResources entry (matching the same cpu/memory requests and limits), ensuring the keys and values are preserved but moved under stepSpecs to mirror the pull-request PipelineRun migration; keep the existing resource numbers and pipelineTaskName identifiers unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
@.tekton/odh-workbench-codeserver-datascience-cpu-py312-ubi9-odh-main-push.yaml:
- Around line 180-198: Update the taskRunSpecs entries so the computeResources
overrides are applied at the step level: for both pipelineTaskName values
"prefetch-dependencies" and "build-images" replace the task-level
computeResources block with a stepSpecs array that contains a
stepSpecs[].computeResources entry (matching the same cpu/memory requests and
limits), ensuring the keys and values are preserved but moved under stepSpecs to
mirror the pull-request PipelineRun migration; keep the existing resource
numbers and pipelineTaskName identifiers unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0d75beb2-0cf6-4165-b42e-4eb0b1a3de68
📒 Files selected for processing (2)
.tekton/odh-workbench-codeserver-datascience-cpu-py312-ubi9-odh-main-pull-request.yaml.tekton/odh-workbench-codeserver-datascience-cpu-py312-ubi9-odh-main-push.yaml
|
@ysok: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/kfbuild codeserver/ubi9-python-3.12 |
3 similar comments
|
/kfbuild codeserver/ubi9-python-3.12 |
|
/kfbuild codeserver/ubi9-python-3.12 |
|
/kfbuild codeserver/ubi9-python-3.12 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: atheo89, jiridanek The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
chore: Update task.step resource
Description
chore: Update task.step resource
How Has This Been Tested?
Self checklist (all need to be checked):
make test(gmakeon macOS) before asking for reviewDockerfile.konfluxfiles should be done inodh/notebooksand automatically synced torhds/notebooks. For Konflux-specific changes, modifyDockerfile.konfluxfiles directly inrhds/notebooksas these require special attention in the downstream repository and flow to the upcoming RHOAI release.Merge criteria:
Summary by CodeRabbit
Release Notes