fix(hack): enhance hack/propagate.sh with git optimization, support custom argocd-operator branch, and inclusion in Makefile targets#1196
Conversation
…ustom argocd-operator branch, and inclusion in Makefile targets Signed-off-by: Cheng Fang <cfang@redhat.com> Co-authored-by: Claude <noreply@anthropic.com>
|
[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 |
|
Hi @chengfang. Thanks for your PR. I'm waiting for a redhat-developer member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe ChangesManifest Propagation Enhancement
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
|
/ok-to-test |
|
@chengfang: Cannot trigger testing until a trusted user reviews the PR and leaves an DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
![coderabbitai[bot]](https://cdn.statically.io/img/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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Makefile`:
- Line 270: The default bundle generation path is too tightly coupled to remote
manifest syncing because the bundle target invokes propagate-manifests, which
shells into the live git clone flow in hack/propagate.sh. Update the Makefile so
bundle only performs local, reproducible generation steps (operator-sdk,
manifests, kustomize), and move the remote sync behavior behind a separate
explicit target or opt-in step. Keep the existing bundle-related targets
consistent by adjusting any adjacent bundle/propagate-manifests definitions so
make bundle no longer depends on GitHub availability or upstream branch tip.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: b935e3b2-694b-44fa-b487-102e637d23af
📒 Files selected for processing (2)
Makefilehack/propagate.sh
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
argoproj-labs/argocd-operator(manual)
|
|
||
| .PHONY: bundle | ||
| bundle: operator-sdk manifests kustomize ## Generate bundle manifests and metadata, then validate generated files. | ||
| bundle: operator-sdk propagate-manifests manifests kustomize ## Generate bundle manifests and metadata, then validate generated files. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Decouple remote manifest sync from the default bundle target.
Because this target now shells into hack/propagate.sh, and that script does a live git clone from argocd-operator (Line 42 in hack/propagate.sh), every make bundle depends on GitHub availability and the current tip of $(ARGOCD_OPERATOR_BRANCH)/master. That makes bundle generation non-reproducible and can break existing CI/E2E/docs workflows that already call make bundle as a normal local generation step.
Possible fix
-bundle: operator-sdk propagate-manifests manifests kustomize ## Generate bundle manifests and metadata, then validate generated files.
+bundle: operator-sdk manifests kustomize ## Generate bundle manifests and metadata, then validate generated files.
@@
.PHONY: propagate-manifests
propagate-manifests: ## compare and propagate manifests from argocd-operator repo
./hack/propagate.sh --skip-make --from-branch $(ARGOCD_OPERATOR_BRANCH)
+
+.PHONY: bundle-with-propagation
+bundle-with-propagation: propagate-manifests bundleAlso applies to: 284-293
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Makefile` at line 270, The default bundle generation path is too tightly
coupled to remote manifest syncing because the bundle target invokes
propagate-manifests, which shells into the live git clone flow in
hack/propagate.sh. Update the Makefile so bundle only performs local,
reproducible generation steps (operator-sdk, manifests, kustomize), and move the
remote sync behavior behind a separate explicit target or opt-in step. Keep the
existing bundle-related targets consistent by adjusting any adjacent
bundle/propagate-manifests definitions so make bundle no longer depends on
GitHub availability or upstream branch tip.
Source: Linked repositories
There was a problem hiding this comment.
@chengfang, any special reason to hook it from bundle? I like the rest, but I am unsure about this one. I run it to update the manifests based on go-code changes in PRs, and pulling updates from argocd-operator master would be undesirable.
There was a problem hiding this comment.
@olivergondza Agreed — that’s the core concern I was trying to flag.
Hooking propagate-manifests into bundle changes make bundle from a local regeneration step into one that can also pull upstream state from argocd-operator, which makes the result depend on GitHub availability and the current tip of $(ARGOCD_OPERATOR_BRANCH). That seems at odds with your existing PR workflow and with the expectation that bundle only reflects local Go/manifests changes.
Keeping propagation as a separate explicit target would preserve the rest of the improvement without changing the meaning of make bundle.
📝 Learnings were identified and not saved (knowledge base disabled). Enable
There was a problem hiding this comment.
The main reason was to make propagate-manifests target more visible and accessible. So when users running the usual bundle target, the propagate-manifests would be auto included to detect any drift. But based on the above concerns and potential disruption it may cause, I've removed it from bundle target, so it remains an independent target.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
|
/ok-to-test |
…rget to avoid affecting existing usage Signed-off-by: Cheng Fang <cfang@redhat.com>
2 participants
What type of PR is this?
/kind enhancement
What does this PR do / why we need it:
enhance
hack/propagate.shto detect any manifest drift between argocd-operator and gitops-operatorHave you updated the necessary documentation?
Which issue(s) this PR fixes:
Fixes #?
https://redhat.atlassian.net/browse/GITOPS-10196
Test acceptance criteria:
How to test changes / Special notes to the reviewer: