-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: update PodDecoration API design #122
Conversation
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #122 +/- ##
===========================================
+ Coverage 46.17% 59.58% +13.40%
===========================================
Files 45 46 +1
Lines 3558 3489 -69
===========================================
+ Hits 1643 2079 +436
+ Misses 1700 1190 -510
- Partials 215 220 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
6c02e7b
to
8422f3b
Compare
Oth proposal,
selector: # Select the Pods to inject config.
matchCollaSet:
- collaSet-a
- collaSet-b
matchLabels: # Select a group of Pods by labelSelector.
...
matchExpressions: # Or select specific Pods by matchExpressions.
...
updateStrategy:
rollingUpdate:
collaSet:
- name: collaSet-a
partition: 2 |
- name: init | ||
image: init-docker:0.1.0 | ||
primaryContainers: # Overwrite old container | ||
- targetPolicy: ByName # ByName | All | First | Last, default by name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to use All
as the default value. There usually is only one primary container.
I think PD users need not to provide the CollaSet name. Telling PD which pods to inject could be enough. About the item 2, I can't get your meaning. Why does PD need to know the CollaSet partition? |
If PD selects Pods under multiple CollaSet, can I still use |
How about sharing the same data between CollaSet and PD controller? PD controller decides the Pod partition, and store this info into the shared memory. Then CollaSet controller is able to know the Pod partition info under each CollaSet from the shared memory. Take the reference from this part of the doc cons: CollaSet controller can not run separately from PD controller |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
1. Does this PR affect any open issues?(Y/N) and add issue references:
2. What is the scope of this PR (e.g. component or file name):
PodDecoration Proposal
3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):
4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):
5. Release note
Please refer to Release Notes Language Style Guide to write a quality release note.