search

openshift-pipelines / pipeline-service

SaaS for Tekton Pipelines
Apache License 2.0
23 stars 44 forks source link

disable pruning temporarily, work around weird operator validation error #1036

Closed gabemontero closed 2 months ago

gabemontero commented 2 months ago

after fixing the pruner config typo, using disabled instead of disable, kept getting this inexplicable error in the operator validating webhook, even though only 'keep-since' was used and not 'keep'

validation failed: expected exactly one, got both: spec.pruner.keep, spec.pruner.keep-since,Reason:BadRequest,Details:nil,Code:400,

only disabling pruning seems to bypass this error in my fix-pruner-cfg branch here.

Currently main branch fails dev_setup.sh with the typo.

so @enarha @divyansh42 yeah there is some weird impl detail with gitops in conjunction with tekton operator

In fact after first changing line 197 in the file to say disabled vs. disable and seeing the validation error above, and then just disabling the pruner, when I ran

oc get tektonconfig config -o yaml -w I would see yaml bits like 

  pruner:
    disabled: true
    keep: 100

where maybe keep: 100 is some default setting the operator does ??? Note that last use of keep with https://github.com/openshift-pipelines/pipeline-service/commit/c12b16367788ca6e4f67e4cdd41ff891abcba77e set it to 10 so I don't thing this is gitops looking at older commits or something

my best guess this is an operator bug @enarha where maybe there is a version difference between the operator in main branch of this repo and what we have in konflux prod, hence we don't see this error with https://github.com/redhat-appstudio/infra-deployments/blob/main/components/pipeline-service/production/base/update-tekton-config-performance.yaml#L34-L41 .... or it is a timing issue where gitops replacing the operator default somehow luckily works @enarha ... something to keep an eye on the next time prod is bumped

rh-pre-commit.version: 2.3.1 rh-pre-commit.check-secrets: ENABLED

enarha commented 2 months ago

Yes, 100 is the default set by the operator https://github.com/tektoncd/operator/blob/main/pkg/apis/operator/v1alpha1/const.go#L64 .

The change is LGTM. Not sure how do you want to promote it though given we have the other change in infra-deployments which removes the ref to this repo.

gabemontero commented 2 months ago

Yes, 100 is the default set by the operator https://github.com/tektoncd/operator/blob/main/pkg/apis/operator/v1alpha1/const.go#L64 .

The change is LGTM. Not sure how do you want to promote it though given we have the other change in infra-deployments which removes the ref to this repo.

Yeah @enarha this change is only needed for running dev_setup.sh out of clones of this repo. Infra deployments will handle pruner separately when the other PR merges as we know.

To make it a bit simpler for him, I'm having @divyansh42 educate / practice grafana panels from dev_setup.sh deployed clusters.

We'll see moving forward after I leave if you all continue using it as a dev env, or you all pivot to however you bootstrap konflux onto OCP.