Adding affinity parameter in deployments#307
Conversation
|
rooftopcellist
left a comment
rooftopcellist
left a comment
There was a problem hiding this comment.
Thanks for adding affinity support across the EDA components — clean implementation that follows the right patterns. Two items to address before this is ready.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughThis PR adds an empty ChangesAffinity support across EDA deployments
Estimated code review effort: 2 (Simple) | ~10 minutes Related PRs: None found. Suggested labels: enhancement, templates Suggested reviewers: None specified. 🐰 A rabbit hops through templates neat, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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.
🧹 Nitpick comments (1)
config/samples/eda_v1alpha1_eda.yaml (1)
28-37: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNice addition addressing prior feedback; label selector value is only valid when
deployment_typeiseda.The example hardcodes
values: [eda-default-worker], but the rendered component label is{{ deployment_type }}-default-worker(pereda-default-worker.deployment.yaml.j2). Since this is commented-out sample text it won't break anything, just worth a caveat comment for non-defaultdeployment_typedeployments.🤖 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 `@config/samples/eda_v1alpha1_eda.yaml` around lines 28 - 37, The sample anti-affinity example in eda_v1alpha1_eda.yaml hardcodes the worker label value for the default deployment type, so add a caveat in the commented example that the app.kubernetes.io/component value corresponds to the rendered {{ deployment_type }}-default-worker label from eda-default-worker.deployment.yaml.j2 and only matches directly when deployment_type is eda. Keep the existing sample structure, but clarify in the comment that non-default deployment_type installations must adjust the selector value accordingly.
🤖 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.
Nitpick comments:
In `@config/samples/eda_v1alpha1_eda.yaml`:
- Around line 28-37: The sample anti-affinity example in eda_v1alpha1_eda.yaml
hardcodes the worker label value for the default deployment type, so add a
caveat in the commented example that the app.kubernetes.io/component value
corresponds to the rendered {{ deployment_type }}-default-worker label from
eda-default-worker.deployment.yaml.j2 and only matches directly when
deployment_type is eda. Keep the existing sample structure, but clarify in the
comment that non-default deployment_type installations must adjust the selector
value accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: dce53cc3-f3f2-49a2-af39-0cddaf278dbe
📒 Files selected for processing (8)
config/crd/bases/eda.ansible.com_edas.yamlconfig/samples/eda_v1alpha1_eda.yamlroles/eda/defaults/main.ymlroles/eda/templates/eda-activation-worker.deployment.yaml.j2roles/eda/templates/eda-api.deployment.yaml.j2roles/eda/templates/eda-default-worker.deployment.yaml.j2roles/eda/templates/eda-event-stream.deployment.yaml.j2roles/eda/templates/eda-ui.deployment.yaml.j2
Head branch was pushed to by a user without write access
|
3 participants
Summary
This PR adds the capability of managing the affinity of the different component deployments being used by EDA.
Changes
Tests
I tested it using simple podAntiAffinity rules within the EDA CR under spec.event_stream with the following and it worked just fine (against the task pod of controller):
Summary by CodeRabbit