feat: update catalog source spec with config #173
Conversation
perdasilva
force-pushed
the
bug_1927478
branch
from
b723681 to
9cf6ee3
Compare
perdasilva
changed the title
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Please refrain from merging this until we've all aligned on operator-framework/operator-lifecycle-manager#2452 |
|
It seems after the discussion this may not be the right approach. Closing the PR for now. |
openshift-ci
bot
added
the
needs-rebase
perdasilva
force-pushed
the
bug_1927478
branch
from
2e65d95 to
5efebfe
Compare
openshift-ci
bot
removed
the
needs-rebase
perdasilva
force-pushed
the
bug_1927478
branch
2 times, most recently
from
e790a7f to
dcbc254
Compare
|
@kevinrizza @anik120 wdyt? I've tried to keep the number of exposed knobs to a minimum. Is there anything else you guys think we need in there? |
There was a problem hiding this comment.
@perdasilva I think this list looks good. This should be more than enough for now, and we can always add more if we need later, although I haven't seen any request for anything more than what you've included so far.
| type: object | ||
| properties: | ||
| nodeSelector: | ||
| description: 'NodeSelector is a selector which must be true for the pod to fit on a node. Selector which must match a node''s labels for the pod to be scheduled on that node. More info: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/' |
There was a problem hiding this comment.
I can see in the comment above the struct field that you used the single quote, not sure why it rendered it here as double quotes.
| description: 'NodeSelector is a selector which must be true for the pod to fit on a node. Selector which must match a node''s labels for the pod to be scheduled on that node. More info: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/' | |
| description: 'NodeSelector is a selector which must be true for the pod to fit on a node. Selector which must match a node's labels for the pod to be scheduled on that node. More info: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/' |
There was a problem hiding this comment.
I've removed the reference to external documentation. That fixed the underlying issue.
| // +optional | ||
| Tolerations []corev1.Toleration `json:"tolerations,omitempty"` | ||
|
|
||
| // If specified, indicates the pod's priority. "system-node-critical" and |
There was a problem hiding this comment.
nit: This comment feels ripe for rot to me. Do we need to call out the PriorityClass implementation details as they exist today?
There was a problem hiding this comment.
Great call out! I've updated!
perdasilva
force-pushed
the
bug_1927478
branch
from
dcbc254 to
2de8ece
Compare
|
/assign @joelanford |
perdasilva
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
Thank you @kevinrizza and @anik120! I've addressed your comments ^^ |
| @@ -69,6 +70,11 @@ type CatalogSourceSpec struct { | |||
| // +Optional | |||
| Image string `json:"image,omitempty"` | |||
|
|
|||
| // GrpcPodConfig exposes different overrides for the pod spec of the CatalogSource Pod. | |||
| // Only used when SourceType = SourceTypeGrpc and Image is set. | |||
| // +Optional | |||
There was a problem hiding this comment.
Is the +Optional comment marker case insensitive? I've only ever seen it written as +optional.
There was a problem hiding this comment.
That's a great question. I just kept what was around. I've updated everything to be lowercase to keep it with the general lower-case convention.
Signed-off-by: Per G. da Silva <[email protected]>
perdasilva
force-pushed
the
bug_1927478
branch
from
2de8ece to
8de8874
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: joelanford, perdasilva The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
Signed-off-by: Per G. da Silva [email protected]
Description
This PR adds a new optional
grpcPodConfigfield to theCatalogSourcespec. The config includes: node selector and toleration configuration, which will eventually make its way to the CatalogSource pod. With this change, and subsequent catalog operator update, users will be able to specify node selectors and tolerations for the underlying catalog source pod.Testing
I need some guidance here if we have a test suite to exercise the creation of the objects. So far, I've manually tested the creation against the spec.