feat: support for additional pod scheduling definition on IntegrationSource #8601
Conversation
|
Welcome @S-Mann! It looks like this is your first PR to knative/eventing 🎉 |
|
Hi @S-Mann. Thanks for your PR. I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
|
/cc @Cali0707 |
|
/ok-to-test |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8601 +/- ##
==========================================
- Coverage 52.32% 52.29% -0.03%
==========================================
Files 398 398
Lines 25036 25040 +4
==========================================
- Hits 13099 13094 -5
- Misses 11139 11145 +6
- Partials 798 801 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| source.Spec.Template.ObjectMeta = metav1.ObjectMeta{ | ||
| Labels: integration.Labels(source.Name), | ||
| } |
There was a problem hiding this comment.
Here instead of setting the full ObjectMeta, could we maybe just append the integration.Labels(source.Name) to any existing labels set in the template?
There was a problem hiding this comment.
oh my god you are right!! I missed this, overriding the whole metadata is a bad idea I think
thank you so much!!
There was a problem hiding this comment.
if you are happy with my latest code you can mark this resolved
| if source.Spec.Template == nil { | ||
| source.Spec.Template = &corev1.PodTemplateSpec{} | ||
| } | ||
| source.Spec.Template.ObjectMeta.Labels = integration.Labels(source.Name) |
There was a problem hiding this comment.
This will still overwrite any other existing labels defined by the user. I think we need something along the lines of:
| source.Spec.Template.ObjectMeta.Labels = integration.Labels(source.Name) | |
| if source.Spec.Template.ObjectMeta.Labels == nil { | |
| source.Spec.Template.ObjectMeta.Labels = integration.Labels(source.Name) | |
| } else { | |
| source.Spec.Template.ObjectMeta.Labels = append(source.Spec.Template.ObjectMeta.Labels, integration.Labels(source.Name) | |
| } |
There was a problem hiding this comment.
I updated the code, so that labels that we are defining will be overwritten and other labels the users define will stay untouched
Signed-off-by: S-Mann <29407727+S-Mann@users.noreply.github.com>
Signed-off-by: S-Mann <29407727+S-Mann@users.noreply.github.com>
Signed-off-by: S-Mann <29407727+S-Mann@users.noreply.github.com>
Signed-off-by: S-Mann <29407727+S-Mann@users.noreply.github.com>
…ersource CRDs Signed-off-by: S-Mann <29407727+S-Mann@users.noreply.github.com>
Signed-off-by: S-Mann <29407727+S-Mann@users.noreply.github.com>
Signed-off-by: S-Mann <29407727+S-Mann@users.noreply.github.com>
Signed-off-by: S-Mann <29407727+S-Mann@users.noreply.github.com>
Signed-off-by: S-Mann <29407727+S-Mann@users.noreply.github.com>
Signed-off-by: S-Mann <29407727+S-Mann@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Cali0707, S-Mann 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 |
| source.Spec.Template.Spec.Containers = []corev1.Container{ | ||
| { | ||
| Name: "source", | ||
| Image: selectImage(source), |
There was a problem hiding this comment.
can we add a test to make sure the image can not be overriden?
There was a problem hiding this comment.
sure @matzew
can you please create an issue for this? I will add the test case
|
@S-Mann the scaling would also make sense on the IntegrationSink ... |
Proposed Changes
Pre-review Checklist
Release Note
Docs