Skip to content

chore: add ci to test deploy with helm #377

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

liangyuanpeng
Copy link

What this PR does / why we need it

Yesterday, I attempted to deploy llmaz using helm, but encountered some issues. This PR fixed the problems and added a CI to ensure that the helm deployment was problem-free.

/kind cleanup

Which issue(s) this PR fixes

Fixes #

Special notes for your reviewer

Does this PR introduce a user-facing change?

NONE

@InftyAI-Agent InftyAI-Agent added needs-triage Indicates an issue or PR lacks a label and requires one. needs-priority Indicates a PR lacks a label and requires one. do-not-merge/needs-kind Indicates a PR lacks a label and requires one. labels Apr 24, 2025
@InftyAI-Agent InftyAI-Agent requested a review from kerthcet April 24, 2025 08:37
go-version-file: go.mod

- name: Set up Helm
uses: azure/setup-helm@v4
Copy link
Author

Choose a reason for hiding this comment

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

Perhaps the preference is to complete the installation of helm in the Makefile. Here, it's just a quick start. I'd be more than happy to update. like add a command: make helm to install helm.

Copy link
Member

Choose a reason for hiding this comment

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

Feel free to polish this in the follow up.

Comment on lines +9 to +40
export CWD=$(pwd)
function cleanup {
if [ $USE_EXISTING_CLUSTER == 'false' ]
then
$KIND delete cluster --name $KIND_CLUSTER_NAME
fi
}
function startup {
if [ $USE_EXISTING_CLUSTER == 'false' ]
then
$KIND create cluster --name $KIND_CLUSTER_NAME --image $E2E_KIND_NODE_VERSION --config ./hack/kind-config.yaml
fi
}
function kind_load {
$KIND load docker-image $IMAGE_TAG --name $KIND_CLUSTER_NAME
}
function deploy {
cd $CWD
HELM_EXT_OPTS='--namespace=llmaz-system --create-namespace --set controllerManager.manager.image.tag=${LOADER_IMAGE_TAG}' make helm-install
$KUBECTL wait --timeout=3m --for=condition=ready pods --namespace=llmaz-system -l app!=certgen
echo "all pods of llmaz-system is ready..."
$KUBECTL get pod -n llmaz-system
}
function deploy_kube_prometheus {
LATEST=$(curl -s https://api.github.com./repos/prometheus-operator/prometheus-operator/releases/latest | jq -cr .tag_name)
curl -sL https://github.com./prometheus-operator/prometheus-operator/releases/download/${LATEST}/bundle.yaml | $KUBECTL create -f -
}
trap cleanup EXIT
startup
kind_load
deploy_kube_prometheus
deploy
Copy link
Author

Choose a reason for hiding this comment

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

Just quickly work, I am very willing to reuse it in util.sh in this PR or the next PR.

Comment on lines -30 to +34
repository: "oci://registry-1.docker.io/envoyproxy/"
repository: "oci://registry-1.docker.io/envoyproxy"
condition: envoy-gateway.enabled
- name: ai-gateway-helm
version: v0.0.0-latest
repository: "oci://registry-1.docker.io/envoyproxy/"
repository: "oci://registry-1.docker.io/envoyproxy"
Copy link
Author

@liangyuanpeng liangyuanpeng Apr 24, 2025

Choose a reason for hiding this comment

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

I just got the error when I run make helm-install , but this change fixed it.

...
Downloading open-webui from repo https://helm.openwebui.com/
Downloading gateway-helm from repo oci://registry-1.docker.io/envoyproxy/
Save error occurred:  could not download oci://registry-1.docker.io/envoyproxy//gateway-helm: invalid_reference: invalid repository
Error: could not download oci://registry-1.docker.io/envoyproxy//gateway-helm: invalid_reference: invalid repository

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, it's strange, I didn't encounter this. I will ask someone else to verify this because it may break users like you.

Copy link
Member

@kerthcet kerthcet left a comment

Choose a reason for hiding this comment

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

General LGTM.

branches-ignore:
- 'dependabot/**'
pull_request:
# paths:
Copy link
Member

Choose a reason for hiding this comment

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

Should we only check the chart/** ? I think we're only verifying that the helm chart is deployed as expected not the function, right?

go-version-file: go.mod

- name: Set up Helm
uses: azure/setup-helm@v4
Copy link
Member

Choose a reason for hiding this comment

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

Feel free to polish this in the follow up.

$KUBECTL get pod -n llmaz-system
}
function deploy_kube_prometheus {
LATEST=$(curl -s https://api.github.com./repos/prometheus-operator/prometheus-operator/releases/latest | jq -cr .tag_name)
Copy link
Member

Choose a reason for hiding this comment

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

We may make the prometheus operator as a dependency of helm chart as well in the future.

cd $CWD
HELM_EXT_OPTS='--namespace=llmaz-system --create-namespace --set controllerManager.manager.image.tag=${LOADER_IMAGE_TAG}' make helm-install
$KUBECTL wait --timeout=3m --for=condition=ready pods --namespace=llmaz-system -l app!=certgen
echo "all pods of llmaz-system is ready..."
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
echo "all pods of llmaz-system is ready..."
echo "All pods of llmaz are ready..."

@kerthcet
Copy link
Member

/kind cleanup

@InftyAI-Agent InftyAI-Agent added cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. and removed do-not-merge/needs-kind Indicates a PR lacks a label and requires one. labels Apr 24, 2025
contents: read

jobs:
chart-lint-test:
Copy link
Member

Choose a reason for hiding this comment

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

chart-deploy-verify?

@kerthcet
Copy link
Member

kindly ping @liangyuanpeng

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. needs-priority Indicates a PR lacks a label and requires one. needs-triage Indicates an issue or PR lacks a label and requires one.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants