Skip to content

Conversation

@jiridanek
Copy link
Member

@jiridanek jiridanek commented Oct 24, 2025

https://issues.redhat.com/browse/RHAIENG-287

kubernetes namespace events
  [INFO] Running command (('kubectl get events',), {'shell': True})
  LAST SEEN   TYPE      REASON         OBJECT                                                                MESSAGE
  71s         Warning   FailedCreate   statefulset/jupyter-pytorch-llmcompressor-ubi9-python-3-12-notebook   create Pod jupyter-pytorch-llmcompressor-ubi9-python-3-12-notebook-0 in StatefulSet jupyter-pytorch-llmcompressor-ubi9-python-3-12-notebook failed error: Pod "jupyter-pytorch-llmcompressor-ubi9-python-3-12-notebook-0" is invalid: metadata.labels: Invalid value: "jupyter-pytorch-llmcompressor-ubi9-python-3-12-notebook-7d69c59fc6": must be no more than 63 characters

Description

How Has This Been Tested?

Self checklist (all need to be checked):

  • Ensure that you have run make test (gmake on macOS) before asking for review
  • Changes to everything except Dockerfile.konflux files should be done in odh/notebooks and automatically synced to rhds/notebooks. For Konflux-specific changes, modify Dockerfile.konflux files directly in rhds/notebooks as these require special attention in the downstream repository and flow to the upcoming RHOAI release.

Merge criteria:

  • The commits are squashed in a cohesive manner and have meaningful messages.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

Summary by CodeRabbit

  • Chores
    • Shortened deployment name prefix to meet naming limits and updated image tag for the PyTorch LLM Compressor Jupyter environment.
    • Added service and stateful set resources to improve deployment resource management.
  • Bug Fixes
    • Acceptance of a shortened notebook identifier during test runs by mapping it to the full notebook ID.

@openshift-ci openshift-ci bot requested review from atheo89 and dibryant October 24, 2025 14:14
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 24, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign andyatmiami for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 24, 2025

Walkthrough

Kustomize base kustomization was edited: namePrefix shortened, resources and images blocks added. A test script was updated to add two notebook ID variables and extend _get_notebook_id logic to recognize the shortened notebook identifier and map it to the full notebook ID with optional accelerator prefix.

Changes

Cohort / File(s) Change Summary
Kustomize configuration
jupyter/pytorch+llmcompressor/ubi9-python-3.12/kustomize/base/kustomization.yaml
Shortened namePrefix; added resources block referencing service.yaml and statefulset.yaml; added images block setting name/newName to quay.io/opendatahub/workbench-images and newTag to jupyter-pytorch-llmcompressor-ubi9-python-3.12.
Test script updates
scripts/test_jupyter_with_papermill.sh
Added two variables: jupyter_pytorch_llmcompressor_notebook_shortened_id and jupyter_pytorch_llmcompressor_notebook_full_id; extended _get_notebook_id to map the shortened ID (with optional accelerator prefix) to the full notebook ID.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR description does not adequately fulfill the template requirements. While the description correctly includes the issue reference (RHAIENG-287 and GitHub issue #2616) and provides helpful context through the Kubernetes error event, the main "Description" section is largely empty—it should detail what changes were made and how they solve the problem but instead only provides error context and links. Additionally, the "How Has This Been Tested?" section lacks substantive testing details; it appears the developer left this section empty beyond the self-checklist items. Furthermore, two of the three merge criteria checkboxes remain unchecked: testing instructions have not been added to the PR body, and the developer has not verified completing manual testing of the changes. To meet the description requirements, the developer should add a proper "Description" section explaining the specific changes made to kustomization.yaml (such as the namePrefix shortening and the addition of resources and images blocks) and how these changes resolve the Kubernetes label character limit issue. Additionally, the "How Has This Been Tested?" section should document the specific testing performed beyond running make test, and the developer should check the remaining merge criteria boxes or explicitly explain if they cannot be satisfied. Consider adding testing instructions to the PR body if the changes are not immediately obvious to reviewers.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "RHAIENG-287: fix(kustomization.yaml): abbreviate namePrefix to resolve Kubernetes label character limit issue" directly addresses the primary change in the pull request. The title clearly identifies the specific file being modified (kustomization.yaml), describes the action taken (abbreviate namePrefix), and explains the problem being solved (Kubernetes label character limit). According to the raw summary, the namePrefix was updated from "jupyter-pytorch-llmcompressor-ubi9-python-3-12-" to "jupyter-pt-llmcompress-ubi9-python-3-12-" precisely to address this 63-character label limit issue referenced in the Kubernetes error event. While the PR also includes changes to test_jupyter_with_papermill.sh to support the shortened identifier, the primary fix and core purpose of the PR is accurately captured in the title.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b1f50dd and 33805b8.

📒 Files selected for processing (2)
  • jupyter/pytorch+llmcompressor/ubi9-python-3.12/kustomize/base/kustomization.yaml (1 hunks)
  • scripts/test_jupyter_with_papermill.sh (2 hunks)
⏰ 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). (3)
  • GitHub Check: Red Hat Konflux / odh-workbench-jupyter-pytorch-llmcompressor-cuda-py312-ubi9-on-pull-request
  • GitHub Check: build (cuda-jupyter-pytorch-llmcompressor-ubi9-python-3.12, 3.12, linux/amd64, false) / build
  • GitHub Check: code-static-analysis
🔇 Additional comments (4)
scripts/test_jupyter_with_papermill.sh (2)

393-394: Correct variable declarations for shortened and full notebook IDs.

The new variables clearly distinguish between the abbreviated namePrefix form (pt-llmcompress) and the canonical notebook identifier (pytorch-llmcompressor), enabling correct mapping in downstream logic.


349-351: No pattern conflicts detected. The code change correctly distinguishes abbreviated and full notebook identifiers.

Pattern analysis confirms:

  • *pt-llmcompress-* matches LLM compressor pod names (e.g., "pt-llmcompress-notebook")
  • *pytorch-* matches regular pytorch pod names (e.g., "pytorch-notebook")
  • These substrings are mutually exclusive; no overlap occurs

The case statement mapping at lines 349–351 correctly returns the full notebook ID (pytorch-llmcompressor) for abbreviated pod names, maintaining consistency with downstream functions.

jupyter/pytorch+llmcompressor/ubi9-python-3.12/kustomize/base/kustomization.yaml (2)

4-6: NamePrefix abbreviation correctly addresses Kubernetes 63-character label limit.

The namePrefix is shortened from jupyter-pytorch-llmcompressor-ubi9-python-3-12- (46 chars) to jupyter-pt-llmcompress-ubi9-python-3-12- (42 chars), reducing the final pod label value by ~6 characters. Combined with the pod suffix and hash, this keeps the label under the 63-character limit (approx. 60 chars vs. the previous ~66 chars that triggered the error). The inline comment referencing the specific error message is helpful for future context.


11-17: Code changes approved - verification confirms all referenced resources exist.

The explicit declaration of resources and images in the kustomization improves maintainability. Verification confirms both service.yaml and statefulset.yaml are present in the base directory.


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

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@openshift-ci openshift-ci bot added size/xs and removed size/xs labels Oct 24, 2025
…e Kubernetes label character limit issue

```
kubernetes namespace events
  [INFO] Running command (('kubectl get events',), {'shell': True})
  LAST SEEN   TYPE      REASON         OBJECT                                                                MESSAGE
  71s         Warning   FailedCreate   statefulset/jupyter-pytorch-llmcompressor-ubi9-python-3-12-notebook   create Pod jupyter-pytorch-llmcompressor-ubi9-python-3-12-notebook-0 in StatefulSet jupyter-pytorch-llmcompressor-ubi9-python-3-12-notebook failed error: Pod "jupyter-pytorch-llmcompressor-ubi9-python-3-12-notebook-0" is invalid: metadata.labels: Invalid value: "jupyter-pytorch-llmcompressor-ubi9-python-3-12-notebook-7d69c59fc6": must be no more than 63 characters
```

```
+ notebook_workload_name=jupyter-pt-llmcompress-ubi9-python-3-12-notebook-0
+ _handle_test
+ local notebook_id=
++ _get_notebook_id
++ local notebook_id=
++ local accelerator=cuda
++ accelerator=
++ case "${notebook_workload_name}" in
++ printf '%s\n' 'No matching condition found for jupyter-pt-llmcompress-ubi9-python-3-12-notebook-0.'
++ exit 1
```
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 24, 2025

@jiridanek: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/images 33805b8 link true /test images

Full PR test history. Your PR dashboard.

Instructions 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-requested GitHub Bot creates notification on #pr-review-ai-ide-team slack channel size/xs

Projects

None yet

1 participant