Skip to content

Conversation

@Xunzhuo
Copy link
Member

@Xunzhuo Xunzhuo commented Nov 14, 2025

What type of PR is this?

Bug fix - This PR fixes a Helm chart installation issue related to namespace creation.

What this PR does / why we need it:

This PR removes the namespace.yaml template from the Helm chart to resolve installation conflicts when the namespace already exists.

Problem:

  • The namespace.yaml template uses the lookup function to check if a namespace exists
  • This causes installation failures with the error: namespaces "vllm-semantic-router-system" already exists
  • The lookup function doesn't work reliably in all scenarios (OCI registries, certain Helm versions)

Solution:

  • Remove the namespace.yaml template entirely
  • Let Helm's built-in --create-namespace flag handle namespace creation
  • This approach is more reliable and follows Helm best practices

Testing:
After this change, users can install the chart using:

helm install semantic-router oci://ghcr.io/xunzhuo/charts/semantic-router \
  --version v0.0.0-latest \
  --namespace vllm-semantic-router-system \
  --create-namespace

The installation will work whether the namespace exists or not.


Pull Request opened by Augment Code with guidance from the PR author

The namespace.yaml template with lookup function causes installation
failures when the namespace already exists. Removing this template
allows Helm's --create-namespace flag to handle namespace creation
properly, avoiding conflicts with existing namespaces.

Fixes the error: namespaces "vllm-semantic-router-system" already exists

Signed-off-by: bitliu <bitliu@tencent.com>
@Xunzhuo Xunzhuo requested a review from rootfs as a code owner November 14, 2025 06:47
@netlify
Copy link

netlify bot commented Nov 14, 2025

Deploy Preview for vllm-semantic-router ready!

Name Link
🔨 Latest commit 8aca920
🔍 Latest deploy log https://app.netlify.com/projects/vllm-semantic-router/deploys/6916d88e24065d0007dd75e6
😎 Deploy Preview https://deploy-preview-651--vllm-semantic-router.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Contributor

@yuluo-yx yuluo-yx left a comment

Choose a reason for hiding this comment

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

/lgtm

ci need fix.

Remove Namespace from required resources check since namespace.yaml
template has been removed. The namespace is now managed by Helm's
--create-namespace flag instead of being part of the chart templates.

Signed-off-by: bitliu <bitliu@tencent.com>
@github-actions
Copy link

github-actions bot commented Nov 14, 2025

👥 vLLM Semantic Team Notification

The following members have been identified for the changed files in this PR and have been automatically assigned:

📁 Root Directory

Owners: @rootfs, @Xunzhuo
Files changed:

  • .github/workflows/helm-ci.yml

📁 deploy

Owners: @rootfs, @Xunzhuo
Files changed:

  • deploy/helm/validate-chart.sh

vLLM

🎉 Thanks for your contributions!

This comment was automatically generated based on the OWNER files in the repository.

Remove namespace.yaml from required files list and Namespace from
required resources check in the validation script. The namespace
is now managed by Helm's --create-namespace flag.

Signed-off-by: bitliu <bitliu@tencent.com>
@Xunzhuo Xunzhuo merged commit 90ad936 into main Nov 14, 2025
16 checks passed
@Xunzhuo
Copy link
Member Author

Xunzhuo commented Nov 14, 2025

done @yuluo-yx thanks

@Xunzhuo Xunzhuo deleted the fix/helm-namespace-conflict branch November 16, 2025 15:02
szedan-rh pushed a commit to szedan-rh/semantic-router that referenced this pull request Nov 19, 2025
vllm-project#651)

* fix(helm): remove namespace template to resolve installation conflicts

The namespace.yaml template with lookup function causes installation
failures when the namespace already exists. Removing this template
allows Helm's --create-namespace flag to handle namespace creation
properly, avoiding conflicts with existing namespaces.

Fixes the error: namespaces "vllm-semantic-router-system" already exists

Signed-off-by: bitliu <bitliu@tencent.com>

* fix(ci): update helm-ci workflow to remove namespace check

Remove Namespace from required resources check since namespace.yaml
template has been removed. The namespace is now managed by Helm's
--create-namespace flag instead of being part of the chart templates.

Signed-off-by: bitliu <bitliu@tencent.com>

* fix(ci): update validate-chart.sh to remove namespace checks

Remove namespace.yaml from required files list and Namespace from
required resources check in the validation script. The namespace
is now managed by Helm's --create-namespace flag.

Signed-off-by: bitliu <bitliu@tencent.com>

---------

Signed-off-by: bitliu <bitliu@tencent.com>
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.

4 participants