HYPERFLEET-711 - docs: clarify GKE Helm deployment for Quay images - #61
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis documentation update revises the Helm deployment instructions in Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/running-sentinel.md (1)
384-402:⚠️ Potential issue | 🟠 MajorOption B can inherit stale
global.imageRegistryfrom prior release values.Because
helm upgrademerges persisted values, a previous Option A install (global.imageRegistry=quay.io) may stick when running Option B with the same release name. That can produce an invalid image path for the GCR flow unless registry is explicitly reset.Proposed doc fix
helm upgrade --install sentinel-test ./deployments/helm/sentinel \ --namespace ${NAMESPACE} \ --create-namespace \ + --set global.imageRegistry=gcr.io \ --set image.repository=gcr.io/${GCP_PROJECT}/sentinel \ --set image.tag=${IMAGE_TAG} \ --set broker.type=googlepubsub \ --set broker.googlepubsub.projectId=${GCP_PROJECT} \ --set monitoring.podMonitoring.enabled=true # For Prometheus Operator environments (OpenShift, vanilla Kubernetes): helm upgrade --install sentinel-test ./deployments/helm/sentinel \ --namespace ${NAMESPACE} \ --create-namespace \ + --set global.imageRegistry=gcr.io \ --set image.repository=gcr.io/${GCP_PROJECT}/sentinel \ --set image.tag=${IMAGE_TAG} \ --set broker.type=googlepubsub \ --set broker.googlepubsub.projectId=${GCP_PROJECT} \ --set monitoring.serviceMonitor.enabled=true \ --set monitoring.serviceMonitor.additionalLabels.release=prometheus🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/running-sentinel.md` around lines 384 - 402, The docs note that Option B can inherit a stale global.imageRegistry from a prior release, causing an invalid GCR image path; update the helm upgrade examples (the helm upgrade --install sentinel-test ... commands) to either recommend using --reset-values or explicitly override global.imageRegistry (e.g., set global.imageRegistry="" or set it to the correct registry) so the GCR flow uses the intended registry instead of a persisted quay.io value; mention the exact flag (--reset-values) and the value override (global.imageRegistry) so readers can locate and fix their release.
🧹 Nitpick comments (1)
docs/running-sentinel.md (1)
367-373: Image repository name appears inconsistent withmake image-devoutput.Option A sets
--set image.repository=${USER}/hyperfleet-sentinel, but the docs above statemake image-devbuilds/pushesquay.io/${QUAY_USER}/sentinel:dev-<commit>. If users copy both blocks as-is, Helm may reference a different repo than what was pushed.Proposed doc fix
helm upgrade --install sentinel-test ./deployments/helm/sentinel \ --namespace ${NAMESPACE} \ --create-namespace \ --set global.imageRegistry=quay.io \ - --set image.repository=${USER}/hyperfleet-sentinel \ + --set image.repository=${USER}/sentinel \ --set image.tag=dev-$(git rev-parse --short HEAD) \ --set broker.type=googlepubsub \ --set broker.googlepubsub.projectId=${GCP_PROJECT} \ --set monitoring.podMonitoring.enabled=true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/running-sentinel.md` around lines 367 - 373, The helm command uses --set image.repository=${USER}/hyperfleet-sentinel which conflicts with the earlier make image-dev output (quay.io/${QUAY_USER}/sentinel:dev-<commit>); update the deployment docs so the helm flag --set image.repository matches the built/pushed image name (e.g., use quay.io/${QUAY_USER}/sentinel or make the make image-dev target push to quay.io/${QUAY_USER}/hyperfleet-sentinel), and ensure the --set image.tag uses the same dev-$(git rev-parse --short HEAD) or dev-<commit> variable referenced by make image-dev so both image.repository and image.tag are consistent with the build step.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@docs/running-sentinel.md`:
- Around line 384-402: The docs note that Option B can inherit a stale
global.imageRegistry from a prior release, causing an invalid GCR image path;
update the helm upgrade examples (the helm upgrade --install sentinel-test ...
commands) to either recommend using --reset-values or explicitly override
global.imageRegistry (e.g., set global.imageRegistry="" or set it to the correct
registry) so the GCR flow uses the intended registry instead of a persisted
quay.io value; mention the exact flag (--reset-values) and the value override
(global.imageRegistry) so readers can locate and fix their release.
---
Nitpick comments:
In `@docs/running-sentinel.md`:
- Around line 367-373: The helm command uses --set
image.repository=${USER}/hyperfleet-sentinel which conflicts with the earlier
make image-dev output (quay.io/${QUAY_USER}/sentinel:dev-<commit>); update the
deployment docs so the helm flag --set image.repository matches the built/pushed
image name (e.g., use quay.io/${QUAY_USER}/sentinel or make the make image-dev
target push to quay.io/${QUAY_USER}/hyperfleet-sentinel), and ensure the --set
image.tag uses the same dev-$(git rev-parse --short HEAD) or dev-<commit>
variable referenced by make image-dev so both image.repository and image.tag are
consistent with the build step.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 00f3e163-49fe-4325-9169-d11a7a856568
📒 Files selected for processing (1)
docs/running-sentinel.md
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/running-sentinel.md`:
- Around line 372-373: The deployment helm values use --set
image.repository=${USER}/hyperfleet-sentinel which doesn't match the image
pushed by make image-dev (quay.io/${QUAY_USER}/sentinel:dev-...), so update the
docs to use the same repository and tag variables: change the image.repository
value referenced in the deployment example from ${USER}/hyperfleet-sentinel to
quay.io/${QUAY_USER}/sentinel (or change the earlier push target to match), and
ensure --set image.tag remains dev-$(git rev-parse --short HEAD) so
image.repository and image.tag (the image.repository and image.tag symbols)
point to the exact image produced by make image-dev and QUAY_USER is used
consistently.
- Around line 387-389: The Option B Helm values currently set both
global.imageRegistry and image.repository with the same registry, causing
duplication; change the Option B invocation to keep --set
global.imageRegistry=gcr.io but remove the registry prefix from --set
image.repository so it becomes --set image.repository=${GCP_PROJECT}/sentinel
(i.e., update the image.repository value referenced in the docs so it does not
include "gcr.io/") so Helm’s template which prepends global.imageRegistry will
produce gcr.io/${GCP_PROJECT}/sentinel:${IMAGE_TAG}.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0441466b-b402-4cbc-a9b2-fc2600653e44
📒 Files selected for processing (1)
docs/running-sentinel.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Makefile`:
- Around line 197-198: The quay-login target currently executes
"$(CONTAINER_TOOL) login quay.io" with no guard; if CONTAINER_TOOL is empty that
runs the system login. Update the Makefile's quay-login target to check for a
non-empty CONTAINER_TOOL before attempting to run it (e.g. fail early or print a
clear error and exit 1 when CONTAINER_TOOL is unset/empty), then invoke
"$(CONTAINER_TOOL) login quay.io" only when the variable is present; reference
the quay-login target and the CONTAINER_TOOL variable when making this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ea1ef1c3-636a-494d-b6f2-edce4e9e98da
📒 Files selected for processing (2)
Makefiledocs/running-sentinel.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/running-sentinel.md
include image registry in the command
3b0eaea to
e9a3a34
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/running-sentinel.md (1)
371-373:⚠️ Potential issue | 🟠 MajorOption A image reference still mismatches the
make image-devoutput.
make image-devdocuments push targetquay.io/${QUAY_USER}/sentinel:dev-..., but Option A deploys${USER}/hyperfleet-sentinel, which points to a different image.Suggested docs fix
helm upgrade --install sentinel-test ./deployments/helm/sentinel \ --namespace ${NAMESPACE} \ --create-namespace \ --set global.imageRegistry=quay.io \ - --set image.repository=${USER}/hyperfleet-sentinel \ + --set image.repository=${QUAY_USER}/sentinel \ --set image.tag=dev-$(git rev-parse --short HEAD) \ --set broker.type=googlepubsub \ --set broker.googlepubsub.projectId=${GCP_PROJECT} \ --set monitoring.podMonitoring.enabled=true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/running-sentinel.md` around lines 371 - 373, The Option A image reference in the docs is inconsistent with the make image-dev output; update the Helm values shown (the image.repository and image.tag lines) so they match the make target (make image-dev) by using quay.io/${QUAY_USER}/sentinel for the repository and the same tag format used by make (dev-$(git rev-parse --short HEAD)); ensure the registry stays quay.io and replace ${USER}/hyperfleet-sentinel with the QUAY_USER sentinel image name and tag to align image.repository and image.tag with make image-dev.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/running-sentinel.md`:
- Around line 371-373: The Option A image reference in the docs is inconsistent
with the make image-dev output; update the Helm values shown (the
image.repository and image.tag lines) so they match the make target (make
image-dev) by using quay.io/${QUAY_USER}/sentinel for the repository and the
same tag format used by make (dev-$(git rev-parse --short HEAD)); ensure the
registry stays quay.io and replace ${USER}/hyperfleet-sentinel with the
QUAY_USER sentinel image name and tag to align image.repository and image.tag
with make image-dev.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8d821c15-950d-4865-bbc6-900c74cb8124
📒 Files selected for processing (1)
docs/running-sentinel.md
and update runbook
runbook comment
missing container tool in quay login target
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rafabene The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
3 participants
Jira ticket
Summary
Test Plan
[x] Validate Quay-based Helm deploy/upgrade flow in the shared hyperfleet-dev cluster.
Summary by CodeRabbit