Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe changes remove k8s-monitoring-dependent deletion protection from Loki and Tempo templates, consolidate kube-prometheus-stack and grafana-operator protection into a single Usage resource, update GitHub Actions workflow references to v2.15.0 (with one e2e reference to a feature branch), and restructure README documentation to emphasize creation order over deletion order. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
Published Crossplane PackageThe following Crossplane package was published as part of this PR: Package: ghcr.io/hops-ops/stack-observe:pr-2-446ce57af2935a77e6fd64ece1c096b5be96b038 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@functions/render/260-grafana-dashboards.yaml.gotmpl`:
- Around line 79-215: The Usage blocks currently gate on readiness (e.g.
conditions like and $state.observed.dashboardOpencostOverview.ready
$state.observed.datasourcePrometheus.ready), which causes the Usage to be
omitted while resources exist but are not "ready"; update each conditional to
gate on resource existence instead (for example replace checks of
$state.observed.dashboardOpencostOverview.ready and
$state.observed.datasourcePrometheus.ready with the corresponding existence
flags such as $state.observed.dashboardOpencostOverview.exists and
$state.observed.datasourcePrometheus.exists or another boolean that reflects
presence), applying the same change to all six blocks (overview/namespace ×
prometheus/loki/tempo) so the Usage objects (names like {{ $state.name
}}-delete-dashboard-overview-before-ds-prometheus, annotations
setResourceNameAnnotation "usage-dsprom-dash-overview", and similar) are
rendered when the resources exist regardless of ready state.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
functions/render/010-state-status.yaml.gotmplfunctions/render/200-kube-prometheus-stack.yaml.gotmplfunctions/render/210-loki.yaml.gotmplfunctions/render/220-tempo.yaml.gotmplfunctions/render/260-grafana-dashboards.yaml.gotmpl
💤 Files with no reviewable changes (3)
- functions/render/200-kube-prometheus-stack.yaml.gotmpl
- functions/render/210-loki.yaml.gotmpl
- functions/render/220-tempo.yaml.gotmpl
| {{- if and $state.observed.dashboardOpencostOverview.ready $state.observed.datasourcePrometheus.ready }} | ||
| --- | ||
| apiVersion: protection.crossplane.io/v1beta1 | ||
| kind: Usage | ||
| metadata: | ||
| name: {{ $state.name }}-delete-dashboard-overview-before-ds-prometheus | ||
| annotations: | ||
| {{ setResourceNameAnnotation "usage-dsprom-dash-overview" }} | ||
| labels: {{ $state.labels | toJson }} | ||
| spec: | ||
| replayDeletion: true | ||
| of: | ||
| apiVersion: kubernetes.m.crossplane.io/v1alpha1 | ||
| kind: Object | ||
| resourceRef: | ||
| name: {{ $state.name }}-datasource-prometheus | ||
| by: | ||
| apiVersion: kubernetes.m.crossplane.io/v1alpha1 | ||
| kind: Object | ||
| resourceRef: | ||
| name: {{ $state.name }}-dashboard-opencost-overview | ||
| {{- end }} | ||
|
|
||
| {{- if and $state.observed.dashboardOpencostOverview.ready $state.observed.datasourceLoki.ready }} | ||
| --- | ||
| apiVersion: protection.crossplane.io/v1beta1 | ||
| kind: Usage | ||
| metadata: | ||
| name: {{ $state.name }}-delete-dashboard-overview-before-ds-loki | ||
| annotations: | ||
| {{ setResourceNameAnnotation "usage-dsloki-dash-overview" }} | ||
| labels: {{ $state.labels | toJson }} | ||
| spec: | ||
| replayDeletion: true | ||
| of: | ||
| apiVersion: kubernetes.m.crossplane.io/v1alpha1 | ||
| kind: Object | ||
| resourceRef: | ||
| name: {{ $state.name }}-datasource-loki | ||
| by: | ||
| apiVersion: kubernetes.m.crossplane.io/v1alpha1 | ||
| kind: Object | ||
| resourceRef: | ||
| name: {{ $state.name }}-dashboard-opencost-overview | ||
| {{- end }} | ||
|
|
||
| {{- if and $state.observed.dashboardOpencostOverview.ready $state.observed.datasourceTempo.ready }} | ||
| --- | ||
| apiVersion: protection.crossplane.io/v1beta1 | ||
| kind: Usage | ||
| metadata: | ||
| name: {{ $state.name }}-delete-dashboard-overview-before-ds-tempo | ||
| annotations: | ||
| {{ setResourceNameAnnotation "usage-dstempo-dash-overview" }} | ||
| labels: {{ $state.labels | toJson }} | ||
| spec: | ||
| replayDeletion: true | ||
| of: | ||
| apiVersion: kubernetes.m.crossplane.io/v1alpha1 | ||
| kind: Object | ||
| resourceRef: | ||
| name: {{ $state.name }}-datasource-tempo | ||
| by: | ||
| apiVersion: kubernetes.m.crossplane.io/v1alpha1 | ||
| kind: Object | ||
| resourceRef: | ||
| name: {{ $state.name }}-dashboard-opencost-overview | ||
| {{- end }} | ||
|
|
||
| {{- if and $state.observed.dashboardOpencostNamespace.ready $state.observed.datasourcePrometheus.ready }} | ||
| --- | ||
| apiVersion: protection.crossplane.io/v1beta1 | ||
| kind: Usage | ||
| metadata: | ||
| name: {{ $state.name }}-delete-dashboard-namespace-before-ds-prometheus | ||
| annotations: | ||
| {{ setResourceNameAnnotation "usage-dsprom-dash-namespace" }} | ||
| labels: {{ $state.labels | toJson }} | ||
| spec: | ||
| replayDeletion: true | ||
| of: | ||
| apiVersion: kubernetes.m.crossplane.io/v1alpha1 | ||
| kind: Object | ||
| resourceRef: | ||
| name: {{ $state.name }}-datasource-prometheus | ||
| by: | ||
| apiVersion: kubernetes.m.crossplane.io/v1alpha1 | ||
| kind: Object | ||
| resourceRef: | ||
| name: {{ $state.name }}-dashboard-opencost-namespace | ||
| {{- end }} | ||
|
|
||
| {{- if and $state.observed.dashboardOpencostNamespace.ready $state.observed.datasourceLoki.ready }} | ||
| --- | ||
| apiVersion: protection.crossplane.io/v1beta1 | ||
| kind: Usage | ||
| metadata: | ||
| name: {{ $state.name }}-delete-dashboard-namespace-before-ds-loki | ||
| annotations: | ||
| {{ setResourceNameAnnotation "usage-dsloki-dash-namespace" }} | ||
| labels: {{ $state.labels | toJson }} | ||
| spec: | ||
| replayDeletion: true | ||
| of: | ||
| apiVersion: kubernetes.m.crossplane.io/v1alpha1 | ||
| kind: Object | ||
| resourceRef: | ||
| name: {{ $state.name }}-datasource-loki | ||
| by: | ||
| apiVersion: kubernetes.m.crossplane.io/v1alpha1 | ||
| kind: Object | ||
| resourceRef: | ||
| name: {{ $state.name }}-dashboard-opencost-namespace | ||
| {{- end }} | ||
|
|
||
| {{- if and $state.observed.dashboardOpencostNamespace.ready $state.observed.datasourceTempo.ready }} | ||
| --- | ||
| apiVersion: protection.crossplane.io/v1beta1 | ||
| kind: Usage | ||
| metadata: | ||
| name: {{ $state.name }}-delete-dashboard-namespace-before-ds-tempo | ||
| annotations: | ||
| {{ setResourceNameAnnotation "usage-dstempo-dash-namespace" }} | ||
| labels: {{ $state.labels | toJson }} | ||
| spec: | ||
| replayDeletion: true | ||
| of: | ||
| apiVersion: kubernetes.m.crossplane.io/v1alpha1 | ||
| kind: Object | ||
| resourceRef: | ||
| name: {{ $state.name }}-datasource-tempo | ||
| by: | ||
| apiVersion: kubernetes.m.crossplane.io/v1alpha1 | ||
| kind: Object | ||
| resourceRef: | ||
| name: {{ $state.name }}-dashboard-opencost-namespace | ||
| {{- end }} |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Crossplane protection.crossplane.io v1beta1 Usage deletion protection Ready status
💡 Result:
In Crossplane, deletion protection using protection.crossplane.io/v1beta1 is done with a Usage (namespaced) or ClusterUsage (cluster-scoped) object. A Usage declares that some resource is “in use”, and Crossplane will reject deletes of the protected (“of”) resource while that usage exists. (docs.crossplane.io)
Minimal deletion-protection example (namespaced Usage)
apiVersion: protection.crossplane.io/v1beta1
kind: Usage
metadata:
namespace: default
name: protect-production-database
spec:
of:
apiVersion: rds.aws.m.upbound.io/v1beta1
kind: Instance
resourceRef:
name: my-database
reason: "Production Database - should never be deleted!"Crossplane also supports a “dependency ordering” form by setting spec.by (the “using” resource). (docs.crossplane.io)
What actually blocks deletion
When the Usage is active, Crossplane marks the referenced resource as in-use (e.g., via an crossplane.io/in-use: "true" label) and a validating webhook denies DELETE requests for resources marked in-use. (deepwiki.com)
What READY means on a Usage
kubectl get usage ... shows a READY column that reflects .status.conditions[type=="Ready"].status. (pkg.go.dev)
Practically:
Ready=True: Crossplane successfully resolved thespec.of(and optionalspec.by) reference/selector and applied the protection mechanism (i.e., the “of” resource is considered protected/in-use). (docs.crossplane.io)Ready=FalseorUnknown: Crossplane can’t make the usage effective yet (most commonly it can’t resolve the referenced resource(s), or can’t update/label them due to RBAC). Usekubectl describe usage ...to see the conditionReason/Message. (Crossplane troubleshooting guidance: inspectstatus.conditionsand Events.) (docs.crossplane.io)
Extra fields that matter
spec.replayDeletion: truere-triggers a previously-blocked deletion of the protected resource after theUsageis removed (useful to avoid long GC backoff delays). (docs.crossplane.io)- Use
ClusterUsageto protect cluster-scoped resources (like CRDs). (docs.crossplane.io)
Citations:
- 1: https://docs.crossplane.io/latest/managed-resources/usages/?utm_source=openai
- 2: https://docs.crossplane.io/latest/managed-resources/usages/?utm_source=openai
- 3: https://deepwiki.com/grafana/crossplane/2.4-rbac-management?utm_source=openai
- 4: https://pkg.go.dev/github.com/crossplane/crossplane/v2/apis/protection/v1beta1?utm_source=openai
- 5: https://docs.crossplane.io/latest/managed-resources/usages/?utm_source=openai
- 6: https://docs.crossplane.io/latest/guides/troubleshoot-crossplane/?utm_source=openai
- 7: https://docs.crossplane.io/latest/managed-resources/usages/?utm_source=openai
- 8: https://docs.crossplane.io/latest/managed-resources/usages/?utm_source=openai
🏁 Script executed:
# Check the broader context of this file to understand the pattern
head -100 functions/render/260-grafana-dashboards.yaml.gotmpl
# Also check if there are other similar Usage patterns elsewhere
find . -type f -name "*.gotmpl" -o -name "*.yaml" | head -20Repository: hops-ops/stack-observe
Length of output: 4547
🏁 Script executed:
# Search for how $state.observed is structured in the codebase
rg 'state\.observed\.' functions/render/260-grafana-dashboards.yaml.gotmpl | head -30
# Look for other patterns that check existence vs readiness
rg 'hasKey' functions/ | head -20
# Check the beginning of the file to understand the state structure
sed -n '1,50p' functions/render/260-grafana-dashboards.yaml.gotmpl
# Search for any existing existence-based checks
rg 'if.*observed' functions/render/260-grafana-dashboards.yaml.gotmpl | head -20Repository: hops-ops/stack-observe
Length of output: 3121
🏁 Script executed:
# Search for the definition of observed resources and their structure
rg 'observed.*resources' functions/ | head -10
# Look for hasKey usage to see if it's a pattern used in this codebase
rg 'hasKey' . --type yaml --type gotmpl
# Check for any resource registration or observed key definitions
rg 'dashboard-opencost-overview|datasource-prometheus' functions/
# Search for how the state/observed structure is defined in the composition
grep -r "observed" apis/ --include="*.yaml" | head -20Repository: hops-ops/stack-observe
Length of output: 2338
🏁 Script executed:
# Look at the 010-state-status file to understand the observed structure better
cat functions/render/010-state-status.yaml.gotmpl | grep -A 20 "observed.resources"
# Check if hasKey is valid Sprig function
rg 'hasKey' functions/ --no-filename -A 2 -B 2Repository: hops-ops/stack-observe
Length of output: 1112
🏁 Script executed:
# Check the complete state-status file to see the full structure
cat functions/render/010-state-status.yaml.gotmpl
# Search for the complete state object definition
rg 'state.*:=.*dict' functions/render/ -A 10 | head -40Repository: hops-ops/stack-observe
Length of output: 3328
🏁 Script executed:
# Verify: when does a resource appear in $.observed.resources?
# Check the composition definition to understand resource observation
head -200 apis/observes/composition.yaml | grep -A 50 "observed"Repository: hops-ops/stack-observe
Length of output: 48
🏁 Script executed:
# Check if resources appear in observed.resources only when they are created
rg 'observed\.resources' apis/ functions/ | head -20Repository: hops-ops/stack-observe
Length of output: 167
Gate Usage protection on resource existence, not readiness.
Lines 79–215 gate each Usage on both resources being ready. During transient non-ready states—even when both resources exist and the dependency is active—the Usage is omitted from the rendered template. On the next composition reconcile, the absent Usage is deleted from the cluster, immediately dropping deletion protection while the dashboard still references the datasource.
For deletion ordering, protect based on resource existence in the cluster, not readiness status.
🔧 Proposed direction (apply to all six blocks)
-{{- if and $state.observed.dashboardOpencostOverview.ready $state.observed.datasourcePrometheus.ready }}
+{{- if and (hasKey $.observed.resources "dashboard-opencost-overview") (hasKey $.observed.resources "datasource-prometheus") }}
---
apiVersion: protection.crossplane.io/v1beta1
kind: Usage
...
{{- end }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@functions/render/260-grafana-dashboards.yaml.gotmpl` around lines 79 - 215,
The Usage blocks currently gate on readiness (e.g. conditions like and
$state.observed.dashboardOpencostOverview.ready
$state.observed.datasourcePrometheus.ready), which causes the Usage to be
omitted while resources exist but are not "ready"; update each conditional to
gate on resource existence instead (for example replace checks of
$state.observed.dashboardOpencostOverview.ready and
$state.observed.datasourcePrometheus.ready with the corresponding existence
flags such as $state.observed.dashboardOpencostOverview.exists and
$state.observed.datasourcePrometheus.exists or another boolean that reflects
presence), applying the same change to all six blocks (overview/namespace ×
prometheus/loki/tempo) so the Usage objects (names like {{ $state.name
}}-delete-dashboard-overview-before-ds-prometheus, annotations
setResourceNameAnnotation "usage-dsprom-dash-overview", and similar) are
rendered when the resources exist regardless of ready state.
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 @.github/workflows/on-pr.yaml:
- Line 47: The workflow is referencing a mutable feature branch in the uses:
directive
(unbounded-tech/workflows-crossplane/.github/workflows/e2e.yaml@feat/more-usage-debugging);
replace that branch ref with an immutable identifier (a released tag like
`@vX.Y.Z` or a commit SHA) so the PR e2e job points to a stable, reproducible
workflow; update the uses: value accordingly while keeping the same path to
.github/workflows/e2e.yaml.
In @.github/workflows/on-push-main.yaml:
- Line 26: Update the three workflow usage references that point to
unbounded-tech/workflows-crossplane/.github/workflows/validate.yaml@2.15.0 so
they match the annotated Git tag by changing the suffix to `@v2.15.0` (replace
occurrences of "@2.15.0" with "@v2.15.0"); alternatively, for stronger
supply-chain security you can pin those usages to the commit SHA by using
"@bfaaaf9" and leave a trailing comment mentioning "v2.15.0" for readability.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/on-pr.yaml.github/workflows/on-push-main.yaml.github/workflows/on-version-tagged.yaml
|
|
||
| e2e: | ||
| uses: unbounded-tech/workflows-crossplane/.github/workflows/e2e.yaml@v2.14.2 | ||
| uses: unbounded-tech/workflows-crossplane/.github/workflows/e2e.yaml@feat/more-usage-debugging |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# First, check the actual file content
echo "=== Checking .github/workflows/on-pr.yaml line 47 ==="
if [ -f ".github/workflows/on-pr.yaml" ]; then
sed -n '45,50p' ".github/workflows/on-pr.yaml"
else
echo "File not found at .github/workflows/on-pr.yaml"
fi
echo ""
echo "=== Verifying upstream refs ==="
repo="https://github.com/unbounded-tech/workflows-crossplane.git"
echo "Checking feature branch ref:"
git ls-remote --refs "$repo" "refs/heads/feat/more-usage-debugging" || echo "Feature branch not found"
echo ""
echo "Checking stable tag ref:"
git ls-remote --refs "$repo" "refs/tags/2.15.0" || echo "Tag not found"
echo ""
echo "List recent tags for reference:"
git ls-remote --tags "$repo" | grep -E '2\.[0-9]+\.[0-9]+' | tail -10Repository: hops-ops/stack-observe
Length of output: 1029
Avoid feature-branch refs in PR e2e workflow.
Line 47 points to a mutable feature branch, which makes CI non-reproducible and brittle (branch force-push/deletion risk). Use a stable release tag or immutable commit SHA.
Suggested fix
- uses: unbounded-tech/workflows-crossplane/.github/workflows/e2e.yaml@feat/more-usage-debugging
+ uses: unbounded-tech/workflows-crossplane/.github/workflows/e2e.yaml@v2.15.0📝 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.
| uses: unbounded-tech/workflows-crossplane/.github/workflows/e2e.yaml@feat/more-usage-debugging | |
| uses: unbounded-tech/workflows-crossplane/.github/workflows/e2e.yaml@v2.15.0 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/on-pr.yaml at line 47, The workflow is referencing a
mutable feature branch in the uses: directive
(unbounded-tech/workflows-crossplane/.github/workflows/e2e.yaml@feat/more-usage-debugging);
replace that branch ref with an immutable identifier (a released tag like
`@vX.Y.Z` or a commit SHA) so the PR e2e job points to a stable, reproducible
workflow; update the uses: value accordingly while keeping the same path to
.github/workflows/e2e.yaml.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
functions/render/200-kube-prometheus-stack.yaml.gotmpl (1)
102-123:⚠️ Potential issue | 🟡 MinorUsage render condition drops protection transiently if
kubePrometheusStackbecomes not-readyThe guard
{{- if and $state.observed.grafanaOperator.ready $state.observed.kubePrometheusStack.ready }}means the Usage is removed from desired state whenever either resource is not observed-and-ready (e.g., during a Helm upgrade of kps itself). Crossplane will garbage-collect the Usage, leaving kps unprotected until the next reconciliation cycle restores readiness.
replayDeletion: trueis designed for the case where a deletion is already in flight — it replays the deletion right after the using resource disappears, significantly reducing wait time. But that doesn't cover the scenario where the Usage is transiently absent and a deletion request arrives in that window.Consider guarding only on
grafanaOperator.ready(thebyresource whose ref must exist), rather than both, since kps being theofresource doesn't need to be ready for the Usage to be valid:🛡️ Proposed tighter guard
-{{- if and $state.observed.grafanaOperator.ready $state.observed.kubePrometheusStack.ready }} +{{- if $state.observed.grafanaOperator.ready }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@functions/render/200-kube-prometheus-stack.yaml.gotmpl` around lines 102 - 123, The Usage resource is being removed whenever either GrafanaOperator or kubePrometheusStack is not-ready because of the guard "{{- if and $state.observed.grafanaOperator.ready $state.observed.kubePrometheusStack.ready }}", which transiently drops protection; change the conditional to only require the "grafanaOperator" readiness (i.e., check $state.observed.grafanaOperator.ready) so the Usage (kind: Usage, metadata name using setResourceNameAnnotation "usage-kps-grafop", spec.replayDeletion: true, spec.of.resourceRef.name = {{ $kps.name }}, spec.by.resourceRef.name = {{ $state.grafanaOperator.name }}) remains present even when kubePrometheusStack is temporarily not-ready.
♻️ Duplicate comments (1)
.github/workflows/on-pr.yaml (1)
47-47:⚠️ Potential issue | 🟠 Major
e2estill points to a mutable feature branch — use a stable tag or SHA.
feat/more-usage-debuggingcan be force-pushed or deleted, making CI non-reproducible and fragile. This was not addressed from the previous review cycle.🔧 Proposed fix
- uses: unbounded-tech/workflows-crossplane/.github/workflows/e2e.yaml@feat/more-usage-debugging + uses: unbounded-tech/workflows-crossplane/.github/workflows/e2e.yaml@v2.15.0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/on-pr.yaml at line 47, Replace the mutable branch ref used in the workflow invocation — update the uses reference "unbounded-tech/workflows-crossplane/.github/workflows/e2e.yaml@feat/more-usage-debugging" to point at a stable tag or a commit SHA (e.g., vX.Y.Z or <commit-sha>) instead of the feature branch so the on-pr workflow becomes reproducible and resilient to force-pushes or branch deletion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 202-217: The deletion graph currently shows k8smon, loki, and
tempo as free resources but omits the two Phase 1 dashboards (dash-overview and
dash-ns), causing inconsistency; update the mermaid graph by adding nodes for
dash-overview and dash-ns (e.g., dash-overview[dash-overview], dash-ns[dash-ns])
and connect them as free/unblocked like k8smon/loki/tempo (use the same -.-
freeX styling and style rules) so the diagram matches the Phase 1 table.
- Line 227: Update the README line describing the "grafana chain" to accurately
state that the grafana-operator Helm release (not kube-prometheus-stack/kps)
installs Grafana CRDs (e.g., GrafanaInstance, GrafanaDatasource), and clarify
the chain's purpose: enforce deletion ordering so GrafanaInstance and related
CRs are removed before the grafana-operator is uninstalled to avoid
finalizer/dangling resource issues; replace the inaccurate phrase
"CRDs...installed by kps" with this corrected description referencing the
grafana-operator and GrafanaInstance/GrafanaDatasource CRs.
---
Outside diff comments:
In `@functions/render/200-kube-prometheus-stack.yaml.gotmpl`:
- Around line 102-123: The Usage resource is being removed whenever either
GrafanaOperator or kubePrometheusStack is not-ready because of the guard "{{- if
and $state.observed.grafanaOperator.ready
$state.observed.kubePrometheusStack.ready }}", which transiently drops
protection; change the conditional to only require the "grafanaOperator"
readiness (i.e., check $state.observed.grafanaOperator.ready) so the Usage
(kind: Usage, metadata name using setResourceNameAnnotation "usage-kps-grafop",
spec.replayDeletion: true, spec.of.resourceRef.name = {{ $kps.name }},
spec.by.resourceRef.name = {{ $state.grafanaOperator.name }}) remains present
even when kubePrometheusStack is temporarily not-ready.
---
Duplicate comments:
In @.github/workflows/on-pr.yaml:
- Line 47: Replace the mutable branch ref used in the workflow invocation —
update the uses reference
"unbounded-tech/workflows-crossplane/.github/workflows/e2e.yaml@feat/more-usage-debugging"
to point at a stable tag or a commit SHA (e.g., vX.Y.Z or <commit-sha>) instead
of the feature branch so the on-pr workflow becomes reproducible and resilient
to force-pushes or branch deletion.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/on-pr.yaml.github/workflows/on-push-main.yaml.github/workflows/on-version-tagged.yamlREADME.mdfunctions/render/200-kube-prometheus-stack.yaml.gotmpl
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/on-push-main.yaml
| | 4 | grafana-operator | grafana-instance gone | | ||
| | 5 | kube-prometheus-stack | grafana-operator gone | | ||
|
|
||
| The grafana chain ensures CRDs (managed by grafana-operator, installed by kps) stay alive until all CRs are cleaned up. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's see the context around line 227 in README.md
sed -n '220,235p' README.mdRepository: hops-ops/stack-observe
Length of output: 658
🏁 Script executed:
# Search for CustomResourceDefinition files in the repository
find . -type f \( -name "*.yaml" -o -name "*.yml" \) -exec grep -l "kind: CustomResourceDefinition" {} \; | head -20Repository: hops-ops/stack-observe
Length of output: 48
🏁 Script executed:
# Get an overview of what grafana-related CRDs might exist in this repo
rg "kind: CustomResourceDefinition" -A 5 -B 1 | head -50Repository: hops-ops/stack-observe
Length of output: 48
🏁 Script executed:
# Check if this is a Helm chart and look at the structure
ls -la | head -20Repository: hops-ops/stack-observe
Length of output: 914
🏁 Script executed:
# Look for Chart.yaml or similar
find . -maxdepth 2 -name "Chart.yaml" -o -name "values*.yaml" | head -10Repository: hops-ops/stack-observe
Length of output: 48
🏁 Script executed:
# Search for grafana-related configuration or comments
rg "grafana" -i --type yaml -C 3 | head -60Repository: hops-ops/stack-observe
Length of output: 3979
🏁 Script executed:
# Look for the full context around line 227 and the chain table
sed -n '200,250p' README.mdRepository: hops-ops/stack-observe
Length of output: 1723
🏁 Script executed:
# Search for deletion, finalizers, or chain-related comments in composition/definition files
rg "deletion|finalizer|chain" -i --type yaml -C 2Repository: hops-ops/stack-observe
Length of output: 341
🏁 Script executed:
# Look at the composition definition to understand the dependency model
cat apis/observes/definition.yaml | head -100Repository: hops-ops/stack-observe
Length of output: 4382
🏁 Script executed:
# Look for composition resources that manage the deletion chain/finalizers
find . -name "*.yaml" -path "*/apis/*" -o -path "*/functions/*" | xargs grep -l "finalizer\|dependsOn" | head -5Repository: hops-ops/stack-observe
Length of output: 106
🏁 Script executed:
# Check if there are composition resources that manage the grafana resources
cat apis/observes/composition.yaml 2>/dev/null | head -150Repository: hops-ops/stack-observe
Length of output: 489
Clarify the purpose of the grafana deletion chain
The phrase "CRDs...installed by kps" is inaccurate. The grafana-operator Helm release installs grafana-operator's CRDs (like GrafanaInstance, GrafanaDatasource), not kube-prometheus-stack.
The grafana chain's actual purpose is ensuring cleanup ordering: grafana-instance custom resources must be deleted before the grafana-operator is removed, preventing issues with finalizers and dangling resources. The comment should reflect this accurately instead of implying kps manages the CRDs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 227, Update the README line describing the "grafana chain"
to accurately state that the grafana-operator Helm release (not
kube-prometheus-stack/kps) installs Grafana CRDs (e.g., GrafanaInstance,
GrafanaDatasource), and clarify the chain's purpose: enforce deletion ordering
so GrafanaInstance and related CRs are removed before the grafana-operator is
uninstalled to avoid finalizer/dangling resource issues; replace the inaccurate
phrase "CRDs...installed by kps" with this corrected description referencing the
grafana-operator and GrafanaInstance/GrafanaDatasource CRs.
patrickleet
force-pushed
the
feat/deletion-ordering
branch
from
cc05644 to
3c07530
Compare
1 participant
Summary by CodeRabbit
Documentation
Chores