Skip to content
This repository has been archived by the owner on Dec 5, 2024. It is now read-only.

Webhooks extension PipelineRun sometimes doesn't have PipelineResources ready in time #240

Closed
a-roberts opened this issue Sep 18, 2019 · 8 comments
Closed

Webhooks extension PipelineRun sometimes doesn't have PipelineResources ready in time #240

a-roberts opened this issue Sep 18, 2019 · 8 comments
Labels
kind/bug Categorizes issue or PR as related to a bug. webhooks

Comments

@a-roberts
Copy link
Member

Expected Behavior

PipelineRuns should be reliably constructed and kicked off

Actual Behavior

Suffers from tektoncd/dashboard#469 also, it's not 100% of the time though (committed again and I didn't see the issue)

Steps to Reproduce the Problem

  1. Create a webhook
  2. Push code to that repository
  3. Observe that on occasion (first time for me the problem observed, second time it was fine) the PipelineRun fails because the PipelineResources don't yet exist
@a-roberts a-roberts added the kind/bug Categorizes issue or PR as related to a bug. label Sep 18, 2019
@a-roberts
Copy link
Member Author

Can't reproduce this in 30 webhook push events 🤔 suggesting this not hold up our upcoming release but something we should certainly look into (I'm using a particularly slow OpenShift cluster as I believe @dibbles was when importing resources and noticed this problem)

@a-roberts
Copy link
Member Author

Can't reproduce this despite valiant efforts - if someone can please reopen this 😄

@a-roberts
Copy link
Member Author

Asked about this in Slack, @dibyom mentioned tektoncd/pipeline#1324 as a potential solution and Andrew's noticed this too for Jenkins X happening

I'm gonna reopen this

@a-roberts a-roberts reopened this Sep 27, 2019
@ncskier
Copy link
Member

ncskier commented Oct 16, 2019

/assign
Just to clarify, the goal of this issue is now to embed the PipelineResource in the PipelineRun that the webhooks extension is creating. Correct?

@kvijai82 kvijai82 mentioned this issue Oct 16, 2019
Closed
@a-roberts
Copy link
Member Author

@ncskier yep

@ncskier
Copy link
Member

ncskier commented Oct 17, 2019

We don't have github.com/tektoncd/pipeline in our Gopkg.toml file. I'm going to add it because we will explicitly need to be on a version including tektoncd/pipeline#1324.

ncskier pushed a commit to ncskier/experimental that referenced this issue Oct 17, 2019
Embed PipelineResources in PipelineRun and TaskRun
5684b28 
Fixes tektoncd#240
PR tektoncd/pipeline#1324 makes it possible to
embed PipelineResourceSpecs into a PipelineRun.

This PR updates our PipelineRuns and TaskRuns to use an embedded
PipelineResourceSpec. So, we will no longer encounter race conditions
between the PipelineResources and PipelineRun/TaskRun.

The Gopkg.toml file was updated to use revision
8871979dfc08fb65ae544c6ad2de83f8bab617b0 of tektoncd/pipeline, so we can
ensure PR 1324 will be available.
@ncskier ncskier mentioned this issue Oct 17, 2019
Closed
2 tasks
@ncskier
Copy link
Member

ncskier commented Oct 18, 2019

PR ready, but holding on this issue. See this comment.
/unassign

@stephenkinder stephenkinder mentioned this issue Nov 1, 2019
Closed
@a-roberts
Copy link
Member Author

Closing this off, we now run Trigger templated code through our managed EventListener, we could update our pipeline examples in the hotel (embedding the resource in the PipelineRun spec) but that's completely separate from this really and can be done whenever we like, closing

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
kind/bug Categorizes issue or PR as related to a bug. webhooks
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Embed PipelineResources in PipelineRun and TaskRun ncskier/experimental
2 participants
@a-roberts @ncskier