Skip to content

Conversation

@Levi080513
Copy link
Collaborator

@Levi080513 Levi080513 commented Nov 25, 2025

Issues

Changes

Support empty repo image registry for better compatibility. After that, we can easily use Docker Hub public image, the example config like this:

apiVersion: v1
kind: ImageRegistry
metadata:
  name: empty-repo
  workspace: default
spec:
  repository: ''
  url: https://docker.io

Test

ssh cluster and kubernetes cluster test passed, also the endpoint deploy on kubernetes test passed.

@codecov
Copy link

codecov bot commented Nov 25, 2025

Codecov Report

❌ Patch coverage is 53.33333% with 21 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/registry/image.go 0.00% 13 Missing ⚠️
controllers/image_registry_controller.go 50.00% 5 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

@Levi080513 Levi080513 requested a review from Copilot November 26, 2025 05:55
Copilot finished reviewing on behalf of Levi080513 November 26, 2025 05:57
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds support for empty repository fields in image registries, enabling more flexible Docker image naming conventions. The change allows users to configure image registries with just a URL (e.g., registry.example.com) without requiring a repository prefix, while also standardizing image paths to include organization/project names.

Key Changes:

  • Modified GetImagePrefix utility to handle empty repository fields, returning just the registry host when repository is empty
  • Updated image naming convention to include organization names in the image path (e.g., /neutree/, /victoriametrics/, /vllm/)
  • Removed database validation that previously required the repository field to be non-empty

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
internal/util/image_registry.go Added validation for empty host and logic to return just the host when repository is empty
internal/util/image_registry_test.go Added test cases for empty repository and invalid URL validation
internal/cluster/ray_ssh_operation.go Refactored to use GetImagePrefix utility and updated image path to include /neutree/
internal/cluster/ray_ssh_operation_test.go Updated test expectations to match new image naming convention with organization names
internal/cluster/component/router/manifests.go Updated router image path to include /neutree/ prefix
internal/cluster/component/router/route_resource_test.go Updated test expectation for router image path
internal/cluster/component/metrics/manifests.go Updated vmagent image path to include /victoriametrics/ prefix
internal/cluster/component/metrics/metrics_resource_test.go Updated test expectation for vmagent image path
internal/accelerator/plugin/gpu.go Updated engine image names to use official image paths (e.g., vllm/vllm-openai)
internal/accelerator/plugin/amd_gpu.go Updated AMD GPU engine image names and versions for vLLM and llama-cpp
db/migrations/030_support_empty_repo.up.sql Removed trigger and function that validated repository field as required
db/migrations/030_support_empty_repo.down.sql Rollback migration to restore repository validation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Levi080513 Levi080513 force-pushed the hw/fix branch 12 times, most recently from 6fc3257 to 192a676 Compare November 26, 2025 08:14
@Levi080513 Levi080513 marked this pull request as ready for review November 26, 2025 10:50
@Levi080513 Levi080513 requested a review from Yuyz0112 November 26, 2025 10:58
@Yuyz0112 Yuyz0112 merged commit 0e9369a into main Nov 27, 2025
2 checks passed
@Yuyz0112 Yuyz0112 deleted the hw/fix branch November 27, 2025 02:36
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.

3 participants