-
Notifications
You must be signed in to change notification settings - Fork 39
Adding Selector field to the MIC image spec #1022
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
Adding Selector field to the MIC image spec #1022
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yevgeny-shnaidman The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/assign @ybettan |
✅ Deploy Preview for kubernetes-sigs-kmm ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1022 +/- ##
==========================================
- Coverage 79.09% 73.45% -5.65%
==========================================
Files 51 72 +21
Lines 5109 6046 +937
==========================================
+ Hits 4041 4441 +400
- Misses 882 1425 +543
+ Partials 186 180 -6 ☔ View full report in Codecov by Sentry. |
| KernelVersion string `json:"kernelVersion"` | ||
|
|
||
| // selector for pod that should run builds | ||
| Selector map[string]string `json:"selector,omitempty"` |
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.
We already have a selector in the build object. Why do we need another one?
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.
the selector should be taken from mld: its value is decided based on Selector in Spec and Selector in Build. If Selector in Build is present, then it is taken, otherwise the Spec's Selector is used
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.
Then I think it's the Module controller's job to update the MIC.build.selector (to be passed to MBSC) with the correct value (from module's spec/selector).
I don't think we should add a new selector in MIC and MBSC just to take that decision in MBSC controller. Let's just create the correct build object in MIC to begging with.
WDYT?
|
@yevgeny-shnaidman: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. |
MIC image spec is being propagated as is to the MBSC image spec. As part of creating sign/build pod, one of the inputs is the selector field, which defines where the pod should be scheduled. This PR does the following: 1. Add Selector field to the ModuleImageSpec 2. module reconciler sets the Selector in the imageSpec variable 3. changing unit-tests
3064e57 to
7433298
Compare
…sigs#1022) This test will ensure the bundle built in the PR can be used to upgrade from the last bundle available of KMM. Signed-off-by: Yoni Bettan <[email protected]>
MIC image spec is being propagated as is to the MBSC image spec. As part of creating sign/build pod, one of the inputs is the selector field, which defines where the pod should be scheduled. This PR does the following: