-
Notifications
You must be signed in to change notification settings - Fork 5
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
[SRVKE-1119] Install eventing from midstream or from serverless operator #528
[SRVKE-1119] Install eventing from midstream or from serverless operator #528
Conversation
Signed-off-by: Ahmed Abdalla <aabdelre@redhat.com>
/assign @matzew |
Its a lot of changes, that do IMO complicate the script to some extent.
Question:
Cant we just install matching midstream eventing, instead of from the
operator? And just have serving from the operator?
I understand the desire to run a pre-installed “productized” eventing-core.
But since we run all productized bits already on SO, is that enough? 🤷🏻♂️
I personally see the most value in midstream to actually test against new
openshift fixes/versions
Just one thought, after reading the new script additions
On Wed 26. Jan 2022 at 13:03, openshift-ci[bot] ***@***.***> wrote:
[APPROVALNOTIFIER] This PR is *APPROVED*
This pull-request has been approved by: *devguyio
<#528#>*
The full list of commands accepted by this bot can be found here
<https://go.k8s.io/bot-commands?repo=openshift-knative%2Feventing-kafka>.
The pull request process is described here
<https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process>
Needs approval from an approver in each of these files:
- OWNERS
<https://github.com/openshift-knative/eventing-kafka/blob/main/OWNERS>
[devguyio]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
—
Reply to this email directly, view it on GitHub
<#528 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABGPTRF3VP5TL5LYZV3CALUX7PIDANCNFSM5M23ETDQ>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
--
Sent from Gmail Mobile
|
# Outputs: | ||
# Writes the calculated release version to stdout | ||
################################################################################ | ||
function calculate_so_release() { |
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.
Noticed this is different than in #524 (for release-next)
shoudl we name it here calculate_so_release_branch
?
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.
this one is the more accurate one, cause it just calculates the release number "1.21" etc regardless of it being a branch or not.
Overall good! Just some thoughts for moving this forward (can be totally done in a separate PR).
Looking here: Will you create a different We could as you did ether parameterize the invocation or even "separate" the "install serverless" into two functions. Thanks for the refactoring here! /lgtm /hold |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: devguyio, matzew 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 |
yeah something similar to that indeed. I'd make a new make target that calls |
/unhold |
I'm gonna cherry pick this to v1.1 only. The older ones are correctly testing against SO. |
@devguyio: new pull request created: #530 In response to this:
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/test-infra repository. |
Signed-off-by: Knative Automation <automation@knative.team>
This will install the eventing version which matches the current eventing-kafka version. If such version is not provided by Serverless Operator (since SO is ~ 2 releases behind upstream) then the midstream eventing repo will be used.
Followup: