Skip to content

Comments

✨ test lightspeed [ do not merge ]#521

Closed
dymurray wants to merge 5 commits intomainfrom
rhImage
Closed

✨ test lightspeed [ do not merge ]#521
dymurray wants to merge 5 commits intomainfrom
rhImage

Conversation

@dymurray
Copy link
Collaborator

@dymurray dymurray commented Jan 27, 2026

Summary by CodeRabbit

  • Chores
    • Updated container image registry sources for deployment components to use Red Hat's container registry instead of Docker Hub.
    • Enhanced CI/CD pipeline with registry authentication for secure image building and deployment.

✏️ Tip: You can customize this high-level summary in your review settings.

Signed-off-by: Dylan <dymurray@redhat.com>
Signed-off-by: Dylan <dymurray@redhat.com>
Signed-off-by: Dylan <dymurray@redhat.com>
@coderabbitai
Copy link

coderabbitai bot commented Jan 27, 2026

Warning

Rate limit exceeded

@dymurray has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 10 minutes and 0 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📝 Walkthrough

Walkthrough

The pull request updates container image sources to use registry.redhat.io instead of docker.io and environment-based defaults, and adds registry authentication credentials to the CI/CD workflow to support these registry changes.

Changes

Cohort / File(s) Summary
Registry Configuration Updates
.github/workflows/llm-proxy-test.yml, helm/values.yaml, roles/tackle/defaults/main.yml
Updates llama_stack and LLM proxy images to reference registry.redhat.io with explicit image tags instead of docker.io and environment-based selectors. Adds registry authentication step in CI/CD workflow using RH_REGISTRY_USER and RH_REGISTRY_PASS secrets for registry.redhat.io access.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A hop to the Red Hat registry we go,
No more docker.io's flow,
With secrets so tight, our images take flight,
From lightspeed-core they glow,
Our proxy now fixed, no env-mix! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title is vague and generic, using non-descriptive terms like 'test lightspeed' without clearly conveying what the actual changes accomplish. Replace with a descriptive title that summarizes the main change, such as 'Update LLM proxy and llama_stack images to registry.redhat.io' or similar.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@helm/values.yaml`:
- Line 29: The values key "llama_stack" was changed to registry.redhat.io but
the ClusterServiceVersion manifest
(konveyor-operator.clusterserviceversion.yaml) still references the docker.io
image; reconcile them by either updating the CSV image references to the
registry.redhat.io image or revert the "llama_stack" value back to the docker.io
image. If you choose to keep registry.redhat.io, add imagePullSecrets to the
operator Deployment/CSV spec (ensure the CSV's deployment spec includes
imagePullSecrets and the corresponding Secret is created) so authenticated pulls
succeed; update the CSV image fields that currently point to
docker.io/llamastack/distribution-starter:latest to match the llama_stack value.

In `@roles/tackle/defaults/main.yml`:
- Around line 330-332: Replace the hardcoded kai_llm_proxy_image_fqin value with
the same env-lookup pattern used by other image vars: set
kai_llm_proxy_image_fqin to lookup('env', 'RELATED_IMAGE_KAI_LLM_PROXY',
'registry.redhat.io/lightspeed-core/lightspeed-stack-rhel9:latest') so
deployments can override via RELATED_IMAGE_KAI_LLM_PROXY while preserving the
Red Hat registry image as the default.
🧹 Nitpick comments (1)
.github/workflows/llm-proxy-test.yml (1)

23-28: Registry authentication approach looks correct.

The --password-stdin pattern is the secure way to pass credentials to docker login. However, ensure the repository secrets RH_REGISTRY_USER and RH_REGISTRY_PASS are configured in the GitHub repository settings, otherwise this step will fail silently or with unclear errors.

Consider adding error handling to fail fast if authentication fails:

🔧 Suggested improvement for explicit failure on auth error
       eval $(minikube docker-env)
-      echo "${{ secrets.RH_REGISTRY_PASS }}" | docker login -u ${{ secrets.RH_REGISTRY_USER }} --password-stdin registry.redhat.io
+      echo "${{ secrets.RH_REGISTRY_PASS }}" | docker login -u ${{ secrets.RH_REGISTRY_USER }} --password-stdin registry.redhat.io || exit 1
       IMG=ttl.sh/konveyor-tackle-operator-${{ github.run_id }}:2h make docker-build docker-push

kantra: quay.io/konveyor/kantra:latest
kai: quay.io/konveyor/kai-solution-server:latest
llama_stack: docker.io/llamastack/distribution-starter:latest
llama_stack: registry.redhat.io/lightspeed-core/lightspeed-stack-rhel9:latest
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if the image reference is used elsewhere and if pull secrets are configured
echo "=== Checking for imagePullSecrets configuration ==="
rg -n "imagePullSecret" --type yaml

echo ""
echo "=== Checking for registry.redhat.io references ==="
rg -n "registry.redhat.io" --type yaml

echo ""
echo "=== Checking for docker.io llama references ==="
rg -n "llamastack" --type yaml

echo ""
echo "=== Checking helm/values.yaml context around line 29 ==="
cat -n helm/values.yaml | sed -n '20,40p'

Repository: konveyor/operator

Length of output: 1447


Update ClusterServiceVersion manifests and configure imagePullSecrets for authenticated registry.

The helm/values.yaml change to registry.redhat.io/lightspeed-core/lightspeed-stack-rhel9:latest is out of sync with bundle/manifests/konveyor-operator.clusterserviceversion.yaml (lines 363, 567), which still references docker.io/llamastack/distribution-starter:latest. Additionally, no imagePullSecrets are configured anywhere in the codebase, yet registry.redhat.io requires authentication to pull images. Either:

  • Update the bundle manifests to match the new registry.redhat.io image, or
  • Revert to the upstream docker.io image

If the Red Hat registry image is intentional, ensure imagePullSecrets are configured in deployments to enable authentication.

🤖 Prompt for AI Agents
In `@helm/values.yaml` at line 29, The values key "llama_stack" was changed to
registry.redhat.io but the ClusterServiceVersion manifest
(konveyor-operator.clusterserviceversion.yaml) still references the docker.io
image; reconcile them by either updating the CSV image references to the
registry.redhat.io image or revert the "llama_stack" value back to the docker.io
image. If you choose to keep registry.redhat.io, add imagePullSecrets to the
operator Deployment/CSV spec (ensure the CSV's deployment spec includes
imagePullSecrets and the corresponding Secret is created) so authenticated pulls
succeed; update the CSV image fields that currently point to
docker.io/llamastack/distribution-starter:latest to match the llama_stack value.

Comment on lines 330 to +332
# LLM Proxy configuration
kai_llm_proxy_enabled: false
kai_llm_proxy_image_fqin: "{{ lookup('env', 'RELATED_IMAGE_LLAMA_STACK') | default('docker.io/llamastack/distribution-starter:latest', true) }}"
kai_llm_proxy_image_fqin: "registry.redhat.io/lightspeed-core/lightspeed-stack-rhel9:latest"
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Hardcoded image breaks the environment override pattern.

All other *_image_fqin variables in this file use the lookup('env', 'RELATED_IMAGE_*') pattern (e.g., lines 28, 84, 106, 136, 173, etc.), allowing OLM and custom deployments to override images via environment variables. This hardcoded value removes that flexibility.

🔧 Suggested fix to maintain consistency with other image variables
 # LLM Proxy configuration
 kai_llm_proxy_enabled: false
-kai_llm_proxy_image_fqin: "registry.redhat.io/lightspeed-core/lightspeed-stack-rhel9:latest"
+kai_llm_proxy_image_fqin: "{{ lookup('env', 'RELATED_IMAGE_LLM_PROXY') or 'registry.redhat.io/lightspeed-core/lightspeed-stack-rhel9:latest' }}"

This maintains consistency with the established pattern while still using the Red Hat registry image as the default.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# LLM Proxy configuration
kai_llm_proxy_enabled: false
kai_llm_proxy_image_fqin: "{{ lookup('env', 'RELATED_IMAGE_LLAMA_STACK') | default('docker.io/llamastack/distribution-starter:latest', true) }}"
kai_llm_proxy_image_fqin: "registry.redhat.io/lightspeed-core/lightspeed-stack-rhel9:latest"
# LLM Proxy configuration
kai_llm_proxy_enabled: false
kai_llm_proxy_image_fqin: "{{ lookup('env', 'RELATED_IMAGE_LLM_PROXY') or 'registry.redhat.io/lightspeed-core/lightspeed-stack-rhel9:latest' }}"
🤖 Prompt for AI Agents
In `@roles/tackle/defaults/main.yml` around lines 330 - 332, Replace the hardcoded
kai_llm_proxy_image_fqin value with the same env-lookup pattern used by other
image vars: set kai_llm_proxy_image_fqin to lookup('env',
'RELATED_IMAGE_KAI_LLM_PROXY',
'registry.redhat.io/lightspeed-core/lightspeed-stack-rhel9:latest') so
deployments can override via RELATED_IMAGE_KAI_LLM_PROXY while preserving the
Red Hat registry image as the default.

@dymurray dymurray closed this Feb 19, 2026
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.

1 participant