Open
Conversation
Contributor
WalkthroughAdds a new administrator guide for running and scheduling Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Poem
🚥 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/en/solutions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md (1)
14-14: Tighten wording for readabilityLine 14 reads awkwardly (
internal registry of cluster). A small wording cleanup improves clarity.💡 Suggested patch
-This document provides a practical and production-ready approach for cleaning images in ACP's internal registry of cluster and automating the cleanup process with `CronJob`. +This document provides a practical, production-ready approach for cleaning images in ACP's internal cluster registry and automating cleanup with `CronJob`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en/solutions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md` at line 14, Summary: The sentence "This document provides a practical and production-ready approach for cleaning images in ACP's internal registry of cluster and automating the cleanup process with `CronJob`." is awkwardly worded; change it to improve clarity. Replace "internal registry of cluster" with "the cluster's internal registry" (or "the internal registry of the cluster") and add a comma before "and automating" so the sentence reads smoothly, e.g., "This document provides a practical, production-ready approach for cleaning images in ACP's cluster internal registry, and automating the cleanup process with `CronJob`." Locate and edit the sentence in the document (the line starting with "This document provides a practical...") to apply this wording fix.
🤖 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/en/solutions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md`:
- Around line 256-257: The scenarios reference serviceAccountName: ac-tester-sa
but the runnable RBAC creates ac-images-pruner-sa, causing SA-not-found errors;
update each scenario YAML that sets serviceAccountName (instances containing
"serviceAccountName: ac-tester-sa") to use "ac-images-pruner-sa" instead so they
match the RBAC setup (replace all occurrences of ac-tester-sa with
ac-images-pruner-sa in the CronJob/Job scenario blocks).
- Line 385: The docs use the CronJob name ac-prune-images-daily when collecting
logs, but you must use an actual Job instance name; replace the example command
"ac logs job/ac-prune-images-daily -n cpaas-system" with a two-step approach:
list Jobs to find the real instance (e.g., run "kubectl get jobs -n
cpaas-system" and locate the job whose name starts with
ac-prune-images-daily-<timestamp>) and then run "kubectl logs
job/<actual-job-name> -n cpaas-system" (use the actual Job name that starts with
ac-prune-images-daily) to collect logs.
---
Nitpick comments:
In
`@docs/en/solutions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md`:
- Line 14: Summary: The sentence "This document provides a practical and
production-ready approach for cleaning images in ACP's internal registry of
cluster and automating the cleanup process with `CronJob`." is awkwardly worded;
change it to improve clarity. Replace "internal registry of cluster" with "the
cluster's internal registry" (or "the internal registry of the cluster") and add
a comma before "and automating" so the sentence reads smoothly, e.g., "This
document provides a practical, production-ready approach for cleaning images in
ACP's cluster internal registry, and automating the cleanup process with
`CronJob`." Locate and edit the sentence in the document (the line starting with
"This document provides a practical...") to apply this wording fix.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 877ab4b4-a49f-4c21-a730-ccd011c1cf29
📒 Files selected for processing (1)
docs/en/solutions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md
...tions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md
Outdated
Show resolved
Hide resolved
...tions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/en/solutions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md (1)
475-476:⚠️ Potential issue | 🟠 MajorUse a real Job instance name for log collection
Line 475 uses the CronJob name as a Job name. CronJob runs create generated Job names, so this command can fail during normal execution. Replace with a two-step lookup + logs flow.
💡 Suggested patch
-ac logs job/ac-prune-images-daily -n cpaas-system +ac get jobs -n cpaas-system --sort-by=.metadata.creationTimestamp +# Pick the latest Job created by CronJob `ac-prune-images-daily`, then: +ac logs job/<actual-job-name> -n cpaas-systemIn Kubernetes CronJob behavior, what naming pattern do spawned Jobs use, and should logs be fetched via `kubectl logs job/<cronjob-name>` or via the actual generated Job name?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en/solutions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md` around lines 475 - 476, The command uses the CronJob name directly (ac-prune-images-daily) but CronJobs spawn Jobs with generated names like ac-prune-images-daily-<timestamp|hash>, so replace the single-line "ac logs job/ac-prune-images-daily -n cpaas-system" with a two-step lookup + logs flow: first list or filter Jobs for the spawned name (e.g., use "kubectl get jobs -n cpaas-system" and find entries starting with "ac-prune-images-daily-" or use a label/selector for the CronJob), then run "kubectl logs job/<generated-job-name> -n cpaas-system" to fetch logs from the actual Job instance; update any references to ac-prune-images-daily accordingly.
🤖 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/en/solutions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md`:
- Around line 475-476: The command uses the CronJob name directly
(ac-prune-images-daily) but CronJobs spawn Jobs with generated names like
ac-prune-images-daily-<timestamp|hash>, so replace the single-line "ac logs
job/ac-prune-images-daily -n cpaas-system" with a two-step lookup + logs flow:
first list or filter Jobs for the spawned name (e.g., use "kubectl get jobs -n
cpaas-system" and find entries starting with "ac-prune-images-daily-" or use a
label/selector for the CronJob), then run "kubectl logs job/<generated-job-name>
-n cpaas-system" to fetch logs from the actual Job instance; update any
references to ac-prune-images-daily accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 705baf5c-4d3b-432a-95aa-d1e71d98997a
📒 Files selected for processing (1)
docs/en/solutions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md
vivindu-1213
left a comment
vivindu-1213
left a comment
There was a problem hiding this comment.
标题是不是可以稍微调整下？
Implementation Steps 算是 Quick Start: Manual Cleanup？
Configure Scheduled Cleanup with CronJob 定时执行 Automating Cleanup with CronJob？
Fully Runnable Example (Dry Run Recommended)”与“Scenario-Based Demos”内容感觉有点冗余...或者整合下，提供 Important Considerations & Best Practices？
...tions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md
Outdated
Show resolved
Hide resolved
...tions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md
Outdated
Show resolved
Hide resolved
Contributor
|
文档中既有 |
chinaran
reviewed
Mar 25, 2026
...tions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/en/solutions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md (2)
35-38: Tighten repeated sentence openings for readability.Several nearby bullets/steps repeat the same opener (“You can…”, “For…”, “Confirm…”). Consider light rewording to improve scanability.
Also applies to: 220-223, 563-567, 650-653
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en/solutions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md` around lines 35 - 38, The listed bullet points repeatedly start with the same opener (e.g., "You can…" and "Confirm…"), hurting scanability; edit the bullets around the shown diff (and the other noted ranges 220-223, 563-567, 650-653) to vary sentence openings by using imperative verbs or concise noun phrases (e.g., "Install ACP CLI (`ac`)", "Have administrative privileges on the target ACP cluster", "Access the target cluster", "Access the internal registry"), or combine related items into a single sentence to reduce repetition while preserving meaning.
660-667: Unify cleanup commands to one CLI (ac) across the guide.The document consistently teaches
ac, but the cleanup section switches tokubectl. Please standardize to avoid operator confusion and align with the existing review request on command unification.💡 Suggested patch
-# Delete namespace-scoped resources (such as CronJob, Job, Pod, ServiceAccount, etc.) -kubectl -n cpaas-system delete cronjob,job,pod,serviceaccount \ +# Delete namespace-scoped resources (such as CronJob, Job, Pod, ServiceAccount, etc.) +ac -n cpaas-system delete cronjob,job,pod,serviceaccount \ -l cpaas.io/cleanup=ac-images-pruner --ignore-not-found # Delete cluster-scoped resources (such as ClusterRole, ClusterRoleBinding, etc.) -kubectl delete clusterrole,clusterrolebinding \ +ac delete clusterrole,clusterrolebinding \ -l cpaas.io/cleanup=ac-images-pruner --ignore-not-found🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en/solutions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md` around lines 660 - 667, Replace the direct kubectl calls with the unified ac CLI while preserving the same resource lists, namespace (-n cpaas-system), label selector (-l cpaas.io/cleanup=ac-images-pruner) and --ignore-not-found behavior; specifically change the two commands invoking "kubectl -n cpaas-system delete cronjob,job,pod,serviceaccount ..." and "kubectl delete clusterrole,clusterrolebinding ..." to use the "ac" CLI (e.g., "ac delete -n cpaas-system cronjob,job,pod,serviceaccount -l cpaas.io/cleanup=ac-images-pruner --ignore-not-found" and "ac delete clusterrole,clusterrolebinding -l cpaas.io/cleanup=ac-images-pruner --ignore-not-found") so the semantics and labels remain identical but the documented CLI is consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@docs/en/solutions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md`:
- Around line 35-38: The listed bullet points repeatedly start with the same
opener (e.g., "You can…" and "Confirm…"), hurting scanability; edit the bullets
around the shown diff (and the other noted ranges 220-223, 563-567, 650-653) to
vary sentence openings by using imperative verbs or concise noun phrases (e.g.,
"Install ACP CLI (`ac`)", "Have administrative privileges on the target ACP
cluster", "Access the target cluster", "Access the internal registry"), or
combine related items into a single sentence to reduce repetition while
preserving meaning.
- Around line 660-667: Replace the direct kubectl calls with the unified ac CLI
while preserving the same resource lists, namespace (-n cpaas-system), label
selector (-l cpaas.io/cleanup=ac-images-pruner) and --ignore-not-found behavior;
specifically change the two commands invoking "kubectl -n cpaas-system delete
cronjob,job,pod,serviceaccount ..." and "kubectl delete
clusterrole,clusterrolebinding ..." to use the "ac" CLI (e.g., "ac delete -n
cpaas-system cronjob,job,pod,serviceaccount -l cpaas.io/cleanup=ac-images-pruner
--ignore-not-found" and "ac delete clusterrole,clusterrolebinding -l
cpaas.io/cleanup=ac-images-pruner --ignore-not-found") so the semantics and
labels remain identical but the documented CLI is consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 922a275b-a7dc-4bd7-8f2a-34b1d4fe5714
📒 Files selected for processing (1)
docs/en/solutions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (5)
docs/en/solutions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md (5)
219-223: Minor: Consider varying sentence structure for readabilityThree consecutive bullet points begin with "For". While clear, varying the structure slightly improves flow:
- For **dry-run only**, `get` access to `registry.alauda.io/images` is sufficient; `delete` is not required. -- For **confirmed pruning**, both `get` and `delete` are required. -- For **registry GC**, `pods/exec` permission is additionally required. +- **Confirmed pruning** requires both `get` and `delete`. +- **Registry GC** additionally requires `pods/exec` permission.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en/solutions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md` around lines 219 - 223, Three bullets all start with "For" which is repetitive; reword them to vary sentence structure while keeping the same meaning — e.g. "Dry-run only: `get` access to `registry.alauda.io/images` is sufficient (no `delete` needed).", "Confirmed pruning: both `get` and `delete` permissions are required.", and "Registry GC: in addition to `get`/`delete`, `pods/exec` permission is required." Update the three lines replacing the current "For ..." phrasing and keep references to `registry.alauda.io/images`, `get`, `delete`, and `pods/exec`.
36-38: Minor: Consider varying sentence structure for readabilityThree consecutive sentences begin with "You can". While clear, varying the structure slightly improves flow. For example:
- You have administrative privileges on the target ACP cluster. -- You can access the target cluster. -- You can access the internal registry of the target cluster. +- Access to the target cluster is available. +- Access to the internal registry of the target cluster is available.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en/solutions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md` around lines 36 - 38, The three bullet lines in the checklist currently start with the same phrase ("You can"), so rephrase them to vary sentence structure for readability; update the three bullets from the diff (the checklist lines about administrative privileges, access to the target cluster, and access to the internal registry) to more varied forms (e.g., make one passive/requirements-style like "Administrative privileges on the target ACP cluster are required.", another as a noun phrase like "Access to the target cluster.", and the third as an imperative or requirement phrase like "Ensure access to the internal registry of the target cluster.") to improve flow while preserving meaning.
83-83: Optional: Clarify "recently created images" descriptionThe description "Keep recently created images" could be more precise. Consider:
-| `--keep-younger-than=<duration>` | Keep recently created images | `168h` | +| `--keep-younger-than=<duration>` | Keep images created within the specified duration | `168h` |This makes it clearer that the duration parameter defines what "recently" means.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en/solutions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md` at line 83, Update the table row for the flag `--keep-younger-than=<duration>` to replace the vague description "Keep recently created images" with a precise phrase such as "Keep images created within the specified duration (e.g., 168h)" so it clearly states that the duration parameter defines the cutoff window for retaining images and gives an example.
414-416: Minor: Avoid repeating "successfully"The adverb "successfully" appears twice in consecutive lines. Consider varying the expression:
- cluster image scanning completed successfully -- registry metadata was fetched successfully +- registry metadata was retrieved - candidate images were listed🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en/solutions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md` around lines 414 - 416, The two consecutive list items "cluster image scanning completed successfully" and "registry metadata was fetched successfully" repeat the adverb "successfully"; update one of them (e.g., change "registry metadata was fetched successfully" to "registry metadata was fetched" or "registry metadata fetched") so the wording is varied while preserving meaning—modify the exact list lines shown ("cluster image scanning completed successfully", "registry metadata was fetched successfully", "candidate images were listed").
499-502: Minor: Vary sentence structure in diagnostic workflowThree consecutive steps begin with "Confirm". Consider varying the structure:
1. Confirm the CronJob exists and the schedule is valid. -2. Confirm a Job was created successfully. -3. Confirm the Pod was created and inspect Pod events. +2. Verify that a Job was created successfully. +3. Check that the Pod was created and inspect Pod events.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en/solutions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md` around lines 499 - 502, Summary: Three consecutive checklist items all start with "Confirm", making the flow repetitive. Edit the numbered steps (items 1–3) to vary verbs while preserving meaning: e.g., change step 1 to "Verify the CronJob exists and the schedule is valid", step 2 to "Ensure a Job was created successfully", and step 3 to "Inspect the Pod and review Pod events"; keep step 4 as-is. Make only minimal wording changes to improve cadence and clarity without altering the diagnostic checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@docs/en/solutions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md`:
- Around line 219-223: Three bullets all start with "For" which is repetitive;
reword them to vary sentence structure while keeping the same meaning — e.g.
"Dry-run only: `get` access to `registry.alauda.io/images` is sufficient (no
`delete` needed).", "Confirmed pruning: both `get` and `delete` permissions are
required.", and "Registry GC: in addition to `get`/`delete`, `pods/exec`
permission is required." Update the three lines replacing the current "For ..."
phrasing and keep references to `registry.alauda.io/images`, `get`, `delete`,
and `pods/exec`.
- Around line 36-38: The three bullet lines in the checklist currently start
with the same phrase ("You can"), so rephrase them to vary sentence structure
for readability; update the three bullets from the diff (the checklist lines
about administrative privileges, access to the target cluster, and access to the
internal registry) to more varied forms (e.g., make one
passive/requirements-style like "Administrative privileges on the target ACP
cluster are required.", another as a noun phrase like "Access to the target
cluster.", and the third as an imperative or requirement phrase like "Ensure
access to the internal registry of the target cluster.") to improve flow while
preserving meaning.
- Line 83: Update the table row for the flag `--keep-younger-than=<duration>` to
replace the vague description "Keep recently created images" with a precise
phrase such as "Keep images created within the specified duration (e.g., 168h)"
so it clearly states that the duration parameter defines the cutoff window for
retaining images and gives an example.
- Around line 414-416: The two consecutive list items "cluster image scanning
completed successfully" and "registry metadata was fetched successfully" repeat
the adverb "successfully"; update one of them (e.g., change "registry metadata
was fetched successfully" to "registry metadata was fetched" or "registry
metadata fetched") so the wording is varied while preserving meaning—modify the
exact list lines shown ("cluster image scanning completed successfully",
"registry metadata was fetched successfully", "candidate images were listed").
- Around line 499-502: Summary: Three consecutive checklist items all start with
"Confirm", making the flow repetitive. Edit the numbered steps (items 1–3) to
vary verbs while preserving meaning: e.g., change step 1 to "Verify the CronJob
exists and the schedule is valid", step 2 to "Ensure a Job was created
successfully", and step 3 to "Inspect the Pod and review Pod events"; keep step
4 as-is. Make only minimal wording changes to improve cadence and clarity
without altering the diagnostic checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7849dae0-6f7b-416b-a9c9-e773eb18bbb5
📒 Files selected for processing (1)
docs/en/solutions/How_to_Cleanup_Alauda_Container_Platform_Cluster_Image_Registry_Administrator_Guide.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit