ADR-0009: Pipeline Service via Operator #141
Conversation
Roming22
requested review from
adambkaplan,
vdemeester,
chmouel,
sbose78 and
ralphbean
|
@chmouel Good catch. I've added a small blurb on the subject in the ADR. |
There was a problem hiding this comment.
A couple of questions/suggestions but overall really good job of getting our ADR up to date @Roming22 - thanks
|
|
||
| - Disable Tekton Triggers. Pipelines as Code will be the favored mechanism for | ||
| event-based triggering of pipelines for now. This decision can be revisited | ||
| in the future based on need. |
There was a problem hiding this comment.
we are not yet doing this
this is as much a "generic how you maintain ADRs" question, but I'll ask anway, as I see this in TEPs, KEPs ... I know there is the "proposed" status, and presumably when everything is done, the status becomes "implemented", but shouldn't we provide some indication of what bits are done and what bits are pending?
There was a problem hiding this comment.
I believe this question applies to other bullets here, but I'll refrain from making updates to each line
There was a problem hiding this comment.
My feeling is that the ADR should capture the destination, and the "current" state should be documented in the service itself.
There was a problem hiding this comment.
I agree with Romain - ADR is structured as "this is what we intend to do, here is the why and what is impacted."
- Deploy "Pipeline Service" using the stock OpenShift Pipelines operator, ensuring that the "latest" code is deployed and that all configuration is driven by the operator. - Mark ADR-0001 as obsolete, superceded by this decision. - Require disabling of Pipelines pruner in favor of Results-driven pruning. - Add requirements for hot fixing of operators and controlling operator versions.
|
/lgtm |
|
@ralphbean @chmouel @adambkaplan Can you please take the time to review/approve? I'd like to merge this PR. |
|
I can only speak for Pipelines-as-Code and this looks good to me... /cc @concaf @vdemeester (for service first osp operator part) |
|
|
||
| Furthermore, as the service will be accessed through CodeReadyToolchain (CRT), the | ||
| following changes are also specific to RHTAP: | ||
| - Deploying a proxy (known as `SprayProxy`) on the CRT host cluster that redirects |
There was a problem hiding this comment.
(nit) maybe not part of this PR but it would be nice there was a document on how the redirection mechanism works, what is the reasoning behind it and if there is further plans to improve this..
There was a problem hiding this comment.
I've opened a story so we create a ADR (or update this one) to include more information about the SprayProxy.
| [infra-deployments](https://github.com/redhat-appstudio/infra-deployments) as | ||
| an ArgoCD application. | ||
|
|
||
| Not all metrics required for operating the service are exposed natively by the |
There was a problem hiding this comment.
@vdemeester I was going to add tektoncd/experimental#953 to the huddle agenda for Oct 11 but it looks like we have an all hands meeting at the same time.
but from what I've seen so far, theoretically I could see us transitioning at least some of the metrics we currently maintain in our "exporter" to that if it got promoted out of experimental, but I think there is a fair bit of logistics and roadmapping we need to do.
At least for this iteration of the ADR, are you OK @vdemeester (are are you OK @Roming22 ) if we add a bullet stating we are monitoring the experimental project?
I could any of these following worlds
- we transition everything in exporter to that new operator
- we live with exporter at least for some time
- we live in a world where we want both the operator, but also the exporter for those metrics we think we need, but get too much pushback
- I'm also curious about how our associated grafana dashboards will get consumed wrt either upstream on downstream, assuming there is agreement that our dashboards would provide value outside of our RHTAP service specific use cases ... which if I had to bet, I bet pipeline users would find them useful.
There was a problem hiding this comment.
I agree with Gabe - we can keep our metrics exporter for now, but work upstream to bring our findings to the experimental controller.
| [infra-deployments](https://github.com/redhat-appstudio/infra-deployments) as | ||
| an ArgoCD application. | ||
|
|
||
| Not all metrics required for operating the service are exposed natively by the |
There was a problem hiding this comment.
I agree with Gabe - we can keep our metrics exporter for now, but work upstream to bring our findings to the experimental controller.
|
|
||
| - Disable Tekton Triggers. Pipelines as Code will be the favored mechanism for | ||
| event-based triggering of pipelines for now. This decision can be revisited | ||
| in the future based on need. |
There was a problem hiding this comment.
I agree with Romain - ADR is structured as "this is what we intend to do, here is the why and what is impacted."
This is a rework of #26. Most of those changes have already been implemented, so this PR to catch up with the current design.