Short term fix for Cloud Event Source #3761
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SelfLink is deprecated and is causing an error on Kubernetes v1.20 clusters as outlined in the issue. This is a short term fix that would set the source field to a value that should match the current source URI without the value of the auto-populated selfLink field. Another field could be used for the source field without issue, but could cause concerns about backwards compatibility.
tekton-robot
added
kind/bug
tekton-robot
added
size/S
|
Hi @jmcshane. Thanks for your PR. I'm waiting for a tektoncd 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/test-infra repository. |
jmcshane
changed the title
|
If the maintainers want a longer term solution that does not rely on self link, I'd be willing to do the work on this. As of today, Tekton pipelines installation on Kubernetes v1.20 clusters by default will not send cloud events. The error message can be seen outlined in the linked issue. |
|
/ok-to-test |
tekton-robot
added
ok-to-test
tekton-robot
added
the
approved
|
The following is the coverage report on the affected files.
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: afrittoli, vdemeester 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 |
afrittoli
added a commit
to waveywaves/tekton-experimental
that referenced
this pull request
Jun 14, 2021
Do not use the events package from pipeline as it will send both k8s and cloud events. Send cloud events only instead. The pipeline controller as enough context to decide whether to send an event or not, based on previous and new condition. The cloud events controller instead does not update the pipeline run condition, so it is only aware of the current resource condition. The only option for now is to send and event every time a resource is queued. Pull in pipelines v0.24.3 to get tektoncd/pipeline#3761 which fixes events on k8s 1.20. Extend the cluster role to be able to write k8s events, so that we may write an event in case of reconcile or cloud event delivery failure. Cloud events are now sent when a sink is specified. Note that since resources do not have a condition when they're just started, the start cloud event is not send for now. The controller will have to replicate the logic in the pipelinerun controller to set a condition before the event is sent. Unit tests are passing, but coverage needs to be added. Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
afrittoli
added a commit
to waveywaves/tekton-experimental
that referenced
this pull request
Jun 14, 2021
Do not use the events package from pipeline as it will send both k8s and cloud events. Send cloud events only instead. The pipeline controller as enough context to decide whether to send an event or not, based on previous and new condition. The cloud events controller instead does not update the pipeline run condition, so it is only aware of the current resource condition. The only option for now is to send and event every time a resource is queued. Pull in pipelines v0.24.3 to get tektoncd/pipeline#3761 which fixes events on k8s 1.20. Extend the cluster role to be able to write k8s events, so that we may write an event in case of reconcile or cloud event delivery failure. Cloud events are now sent when a sink is specified. Note that since resources do not have a condition when they're just started, the start cloud event is not send for now. The controller will have to replicate the logic in the pipelinerun controller to set a condition before the event is sent. Unit tests are passing, but coverage needs to be added. Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
tekton-robot
pushed a commit
to tektoncd/experimental
that referenced
this pull request
Jun 14, 2021
Do not use the events package from pipeline as it will send both k8s and cloud events. Send cloud events only instead. The pipeline controller as enough context to decide whether to send an event or not, based on previous and new condition. The cloud events controller instead does not update the pipeline run condition, so it is only aware of the current resource condition. The only option for now is to send and event every time a resource is queued. Pull in pipelines v0.24.3 to get tektoncd/pipeline#3761 which fixes events on k8s 1.20. Extend the cluster role to be able to write k8s events, so that we may write an event in case of reconcile or cloud event delivery failure. Cloud events are now sent when a sink is specified. Note that since resources do not have a condition when they're just started, the start cloud event is not send for now. The controller will have to replicate the logic in the pipelinerun controller to set a condition before the event is sent. Unit tests are passing, but coverage needs to be added. Signed-off-by: Andrea Frittoli <andrea.frittoli@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
Closes #2676
SelfLink is deprecated and is causing an error on Kubernetes v1.20
clusters as outlined in the issue. This is a short term fix that
would set the source field to a value that should match the current
source URI without the value of the auto-populated selfLink field.
Another field could be used for the source field without issue, but
could cause concerns about backwards compatibility.
/kind bug
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide for more details.
Double check this list of stuff that's easy to miss:
cmddir, please updatethe release Task to build and release this image.
Reviewer Notes
If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.
Release Notes