openshift-pipelines / pipelines-as-code

Pipelines-as-Code for Tekton
https://pipelinesascode.com
Apache License 2.0
124 stars 81 forks source link

Setup pac config syncer #1662

Closed sm43 closed 2 months ago

sm43 commented 2 months ago

this setups a configmap syncer which uses informer and loooks for configmap changes and update the run.Info.

Changes

Submitter Checklist

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 69.56522% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 64.37%. Comparing base (d291f32) to head (15bc57d). Report is 3 commits behind head on main.

Files Patch % Lines
pkg/configutil/config.go 78.94% 2 Missing and 2 partials :warning:
pkg/adapter/adapter.go 0.00% 2 Missing :warning:
pkg/reconciler/controller.go 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1662 +/- ## ========================================== + Coverage 64.30% 64.37% +0.07% ========================================== Files 141 141 Lines 10922 10923 +1 ========================================== + Hits 7023 7032 +9 + Misses 3382 3376 -6 + Partials 517 515 -2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

chmouel commented 2 months ago

/test go-testing

sm43 commented 2 months ago

/test go-testing

Screenshot 2024-04-15 at 15 46 29

is this a bug? no change in the icon ? and Failing status

chmouel commented 2 months ago

was the issue, when it failed first it didn't change the icon and kept as failed until succeeded right?

we merged some change related to that on friday cc @savitaashture

sm43 commented 2 months ago

was the issue, when it failed first it didn't change the icon and kept as failed until succeeded right?

yes. when it was restarted using /test. the status icon didn't changed to in-progress

chmouel commented 2 months ago

/test go-testing

chmouel commented 2 months ago

it seems to work here image

maybe we should get it to fail first ๐Ÿค”

sm43 commented 2 months ago

/test linters

sm43 commented 2 months ago

Screenshot 2024-04-15 at 17 50 23

same behaviour .. CI has started but no change in status icon

sm43 commented 2 months ago

/cancel go-testing

chmouel commented 2 months ago

@savitaashture can you have a look ? i think it's related to this commit https://github.com/openshift-pipelines/pipelines-as-code/commit/1fe9f45d7d35eea6915471450fd6e2dd145f583a

chmouel commented 2 months ago

I have done some stress testing and i didn't see any crash ๐Ÿพ

I did try the feature and it has been working fine:

i change the value twice and that worked (or at least from the log)

๐Ÿ’ก 06:55:36 pac-controller updating value for field MaxKeepRunsUpperLimit: from '0' to '2'
๐Ÿ’ก 06:55:46 pac-controller updating value for field MaxKeepRunsUpperLimit: from '2' to '10'

but if i delete the value and then added back after that doesn't seem to get detected

ie: if i delete max-keep-run-upper-limit and then edit and add a non default max-keep-run-upper-limit: "10" it will not detect something new (or at least there is no logging)

sm43 commented 2 months ago

ie: if i delete max-keep-run-upper-limit and then edit and add a non default max-keep-run-upper-limit: "10" it will not detect something new (or at least there is no logging)

Yeah. it was a bug. updated the code

chmouel commented 2 months ago

This looks good to me, thanks a bunch cc @enarha for info..

i'll go ahead and merge and squash unless you wan tto do it ?

savitaashture commented 2 months ago

yes. when it was restarted using /test. the status icon didn't changed to in-progress

@sm43 can you give me the steps which you tried

I know the issue as that i made it intentionally for some reason