refactor(apps/prod): remove cloudevents-server and redis from prod; update kustomization #1814
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Summary of ChangesHello @wuhuizuo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request streamlines the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![ti-chi-bot[bot]](https://avatars.githubusercontent.com/in/214286?s=60&v=4){kind=small}
There was a problem hiding this comment.
I have already done a preliminary review for you, and I hope to help you do a better job.
Summary
This PR removes the cloudevents-server and redis workloads from the production environment by deleting their directories and manifests under apps/prod/, and updates the apps/prod/kustomization.yaml to no longer reference these components. The approach is straightforward and clean, focusing on eliminating unused resources to reduce maintenance overhead. The changes are well-scoped and the removal appears complete, with no dangling references in the kustomization. Overall, the quality is good, with no obvious errors in the removal process.
Code Improvements
-
Validation of resource dependencies and impact beyond kustomization
- File: N/A (general)
- Why: While the PR removes these components from prod kustomization and deletes their manifests, it’s important to ensure there are no other prod components or HelmReleases depending on
cloudevents-serverorredis(e.g., via service references, configmaps, secrets, or downstream manifests). - Suggestion:
- Double-check other prod workloads for references or dependencies on these services to avoid runtime failures.
- Consider adding a note or checklist in the PR about verifying no downstream dependencies remain.
- Optionally, run a prod integration or smoke test after removal to validate no disruptions.
-
Namespace cleanup for redis
- File:
apps/prod/redis/namespace.yaml(deleted) - Why: The redis namespace and its annotations are deleted, which is correct if redis is not used; however, verify that no other prod workloads rely on this namespace or its node selectors/tolerations.
- Suggestion: Confirm that the redis namespace is safe to remove and that no workloads or secrets remain tied to it.
- File:
Best Practices
-
Documentation update
- File:
apps/prod/README.mdor any relevant prod documentation (not shown in diff) - Why: The removal of major components like redis and cloudevents-server should be reflected in any prod environment documentation or onboarding guides to avoid confusion for operators or developers.
- Suggestion: Add a note about the migration and removal of these components from prod in relevant docs.
- File:
-
Testing coverage confirmation
- File: N/A
- Why: The PR mentions local kustomize render validation, which is good. However, ensure that CI pipelines or integration tests (if any) are updated to reflect the absence of these components and that no tests fail due to missing resources.
- Suggestion: Verify CI tests do not expect these components and update/remove tests accordingly.
-
Commit message clarity
- File: N/A
- Why: The PR commit messages are clear but could explicitly mention the migration target for these workloads (e.g.,
prod2) for traceability. - Suggestion: Add brief comments or commit message footers referencing where the workloads moved.
No critical issues or bugs were found. The removal is clean and consistent with good operational hygiene. The above suggestions will help ensure no hidden dependencies remain and documentation is kept up to date.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly removes the cloudevents-server and redis components from the prod environment by deleting their respective manifest files and updating the main kustomization.yaml. The changes are straightforward and align with the goal of reducing the maintenance surface. I have one minor suggestion to improve the formatting of the kustomization.yaml file for better readability, specifically by removing empty lines.
| - goproxy | ||
| - prow-worker | ||
| - kafka | ||
| - cloudevents-server | ||
|
|
||
| - boskos | ||
| - git-cdn | ||
| - dl | ||
| - harbor | ||
| - redis | ||
|
|
||
| - chatops-lark |
There was a problem hiding this comment.
For better readability and to keep the YAML file clean, it's best to remove the empty lines left after deleting the resources.
- goproxy
- prow-worker
- kafka
- boskos
- git-cdn
- dl
- harbor
- chatops-lark
There was a problem hiding this comment.
I have already done a preliminary review for you, and I hope to help you do a better job.
Summary
This PR effectively removes the cloudevents-server and redis components from the production environment by deleting their directories and manifests, and updating the prod kustomization.yaml to no longer reference them. The approach is straightforward and clean, ensuring no leftover references remain. The overall quality is good, with a clear rationale and validation steps described. The changes align with the stated goal of reducing maintenance surface for unused components.
Code Improvements
-
apps/prod/jenkins/release/values-JCasC.yaml (lines ~311-332)
The PR comments out the cloudevents plugin configuration but leaves the commented block in place. Since the cloudevents-server is removed from prod, this config is no longer relevant and may cause confusion or clutter.Suggestion:
Remove the commented-out cloudevents plugin config block entirely unless there is an explicit plan to re-enable or rewrite it soon. This keeps the config cleaner and reduces cognitive overhead for future maintainers. -
apps/prod/kustomization.yaml (lines 11-22)
The removal of references tocloudevents-serverandredisis correct. However, it would be good to verify that no other overlays or environment-specific kustomizations (like staging, prod2, etc.) still reference these components to avoid deployment inconsistencies.Suggestion:
Add a note or checklist to confirm these components are removed or intentionally retained in other environments.
Best Practices
-
Documentation
The PR description is clear, but the in-repo documentation (e.g., README or docs related to production manifests) might need updates to reflect these removals.Suggestion:
Verify and update any documentation that referencescloudevents-serverorredisin prod, to avoid stale information. -
Testing
Validation is described as running a localkustomize buildto confirm no resources from the removed components are rendered. However, no explicit tests or CI checks are mentioned to prevent regressions (e.g., accidentally re-adding these components).Suggestion:
Consider adding or updating CI tests that verify the prod deployment does not include these components, e.g., snapshot tests of rendered manifests or integration tests confirming absence.
Minor
- Deleted files
The deletions cleanly remove the components without orphaned resources, which is good. No further action needed here.
Overall, this PR achieves its goal well. The main actionable points are to clean up leftover commented config in Jenkins values and ensure documentation and tests are updated to prevent reintroduction or confusion around these removed components.
There was a problem hiding this comment.
I have already done a preliminary review for you, and I hope to help you do a better job.
Summary
This PR removes the cloudevents-server and redis components from the prod environment by deleting their directories and manifests, and updating the apps/prod/kustomization.yaml to no longer include these components. The approach is straightforward and consistent with the stated goal of reducing maintenance surface for unused production workloads. The changes appear well scoped and cleanly remove all references, with validation steps described. The overall quality is good; the PR is focused and low risk.
Code Improvements
-
apps/prod/jenkins/release/values-JCasC.yaml, lines 310-332
- The PR comments out the cloudevents plugin config snippet but leaves it in the file as commented-out YAML. Since the
cloudevents-serveris removed, it would be cleaner to fully remove this commented block or add a clear comment explaining why it is kept. - Suggested:
Or remove entirely if no longer relevant.
# Remove outdated cloudevents plugin config now that cloudevents-server is removed. # Kept for historical reference or future cdevents plugin usage if needed.
- The PR comments out the cloudevents plugin config snippet but leaves it in the file as commented-out YAML. Since the
-
apps/prod/kustomization.yaml
- The diff shows the removal of the
cloudevents-serverandredisentries fromresources. Double-check that no other overlays or environments reference these components, to avoid broken references elsewhere.
- The diff shows the removal of the
Best Practices
-
Documentation
- Add a brief note in
apps/prod/README.mdor equivalent documentation describing the removal of these components and pointing toprod2for any continued usage. This will help future maintainers understand the environment split and workload migration rationale.
- Add a brief note in
-
Testing
- The PR mentions manual validation with
kustomize buildor render. Consider adding automated tests or CI checks that verifykustomizerendering forproddoes not include deleted components to prevent regressions in the future.
- The PR mentions manual validation with
-
Style / Naming
- No issues found. Naming and file structure are consistent.
Critical Issues
None identified. The PR is a safe removal of unused components with no apparent risk of breaking existing functionality.
Summary of actionable suggestions:
- Clean up or document the commented-out cloudevents config in Jenkins values file.
- Add documentation note about removal and migration rationale in prod environment docs.
- Add or update automated tests or CI checks for
kustomizerendering to catch missing resources early. - Verify no other overlays reference the removed components.
These will improve maintainability and clarity without changing the current PR scope significantly.
|
Cleanup complete: removed the cloudevents block from Jenkins values (apps/prod/jenkins/release/values-JCasC.yaml). This resolves the review item and keeps prod Jenkins values clean going forward. Let me know if you want me to run a local kustomize render to double-check prod without cloudevents-related config. |
wuhuizuo
changed the title
Summary
Remove cloudevents-server and redis from prod since workloads have migrated elsewhere. Update prod kustomization accordingly.
Changes
apps/prod/cloudevents-server/andapps/prod/redis/(and their manifest files).apps/prod/kustomization.yamlto drop cloudevents-server and redis references.Rationale
Eliminate unused prod components to reduce maintenance surface.
Validation
Impact