Skip to content

SNOW-3190420: Add start_nbctl.sh validation check for Notebook custom…#2815

Open
sfc-gh-jijiang wants to merge 6 commits intoajiang-SNOW_3202210-cre_validatefrom
jijiang-SNOW-3190420-notebook-cre-validation
Open

SNOW-3190420: Add start_nbctl.sh validation check for Notebook custom…#2815
sfc-gh-jijiang wants to merge 6 commits intoajiang-SNOW_3202210-cre_validatefrom
jijiang-SNOW-3190420-notebook-cre-validation

Conversation

@sfc-gh-jijiang
Copy link

@sfc-gh-jijiang sfc-gh-jijiang commented Mar 13, 2026

Pre-review checklist

  • I've confirmed that instructions included in README.md are still correct after my changes in the codebase.
  • I've added or updated automated unit tests to verify correctness of my new code.
  • I've added or updated integration tests to verify correctness of my new code.
  • I've confirmed that my changes are working by executing CLI's commands manually on MacOS.
  • I've confirmed that my changes are working by executing CLI's commands manually on Windows.
  • I've confirmed that my changes are up-to-date with the target branch.
  • I've described my changes in the release notes.
  • I've described my changes in the section below.
  • I've described my changes in the documentation.

Changes description

Note: This PR is stacked on top of #2809 image validation framework) which is still under review. Please treat this as an early preview — it is ready for feedback but should not be merged until the parent PR lands.

SNOW-3190420: Add start_nbctl.sh validation check for Notebook custom images

This PR adds a Notebook-specific validation check to the snow custom-image validate CLI command.

For Notebook vNext, PEP injects ./start_nbctl.sh /shared/nbctl as the container entrypoint args. If this script is missing or not executable in the custom image, the container will crash immediately on startup. This check catches that before deployment.

What changed:

  • image_validation.yaml: Added required_scripts: [start_nbctl.sh] config entry
  • manager.py: Added _check_required_scripts handler that runs docker run ... test -x <script> to verify each script exists and is executable
  • test_manager.py: Added unit tests for pass (script present) and fail (script missing) scenarios, updated mock helper to support missing_scripts parameter

Validation:

  • All 19 unit tests pass (python3 -m pytest tests/custom_images/ -v)
  • Manually verified start_nbctl.sh exists and is executable in the official snowflake-notebook:ml-cpu image

@sfc-gh-jijiang sfc-gh-jijiang self-assigned this Mar 13, 2026
@sfc-gh-jijiang sfc-gh-jijiang marked this pull request as ready for review March 13, 2026 22:31
@sfc-gh-jijiang sfc-gh-jijiang requested a review from a team as a code owner March 13, 2026 22:31
@sfc-gh-ajiang sfc-gh-ajiang force-pushed the ajiang-SNOW_3202210-cre_validate branch 2 times, most recently from 3f91024 to 3e8901b Compare March 17, 2026 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants