operator icon indicating copy to clipboard operation
operator copied to clipboard

TektonConfig Fails To Reconcile if Pruner Settings Are Missing

Open adambkaplan opened this issue 1 year ago • 7 comments

Expected Behavior

If a TektonConfig object is created without the .spec.pruner.keep or .spec.pruner.keep-since setting, the operator applies a default setting to one of these values.

Actual Behavior

If the .spec.pruner.keep and .spec.pruner.keep-since settings are missing, the operator fails to reconcile because any update request is blocked by the validating webhook

Steps to Reproduce the Problem

Observed when deploying the Shipwright Operator with OLM, which uses API dependency resolution to deploy the Tekton Operator.

Additional Info

  • Kubernetes version:

    Output of kubectl version:

Version 1.30.z


- Tekton Pipeline version: 0.62.x

**Output of `tkn version` or `kubectl get pods -n tekton-pipelines -l app=tekton-pipelines-controller -o=jsonpath='{.items[0].metadata.labels.version}'`**


<!-- Any other additional information -->

Log from Tekton Operator:

{"level":"info","timestamp":"2024-11-25T18:36:53.401Z","logger":"tekton-operator-lifecycle.event-broadcaster","caller":"record/event.go:364","msg":"Event(v1.ObjectReference{Kind:"TektonConfig", Namespace:"", Name:"config", UID:"de70e9b6-ec63-4b25-b377-add833640a29", APIVersion:"operator.tekton.dev/v1alpha1", ResourceVersion:"17433", FieldPath:""}): type: 'Warning' reason: 'UpdateFailed' Failed to update status for "config": admission webhook "validation.webhook.operator.tekton.dev" denied the request: validation failed: expected exactly one, got neither: spec.pruner.keep, spec.pruner.keep-since\nmissing field(s): spec.pruner.resources","commit":"391a4f0","knative.dev/pod":"openshift-pipelines-operator-648cb677d9-vv4fc"}


Red Hat issue tracker: https://issues.redhat.com/browse/SRVKP-6832

Related Shipwright operator bug: https://github.com/shipwright-io/operator/issues/231

adambkaplan avatar Dec 09 '24 16:12 adambkaplan

@adambkaplan defaulting one of these values is already there https://github.com/tektoncd/operator/blob/v0.73.0/pkg/apis/operator/v1alpha1/tektonconfig_defaults.go#L79 Can this be a race between shipwright operator trying to create tektonconfig without spec.pruner.keep and tekton operator trying to default it ? There is also an alternative to have a default tektonconfig created by tekton operator via AUTOINSTALL_COMPONENTS https://github.com/tektoncd/operator/blob/v0.73.0/operatorhub/openshift/release-artifacts/bundle/manifests/tekton-config-defaults_v1_configmap.yaml#L3

jkhelil avatar Dec 17 '24 07:12 jkhelil

Can this be a race between shipwright operator trying to create tektonconfig without spec.pruner.keep and tekton operator trying to default it ?

I can't rule that out for certain. In any case we added a pruner default setting on the Shipwright operator side, and plan to release it soon: https://github.com/shipwright-io/operator/pull/233

adambkaplan avatar Dec 17 '24 18:12 adambkaplan

Even in case of race condition tektonconfig should be able to reconcile when keep and keepsince are absent.

#pkg/apis/operator/v1alpha1/tektonconfig_validation.go#L154

	if p.Keep == nil && p.KeepSince == nil {
		errs = errs.Also(apis.ErrMissingOneOf("spec.pruner.keep", "spec.pruner.keep-since"))
	}

I think in validation instead of throwing error, values should be defaulted.

As we are close to release new event based pruner so this legacy pruner will be deprecated anyways.

pramodbindal avatar May 08 '25 21:05 pramodbindal

/assign @pramodbindal

pramodbindal avatar Jun 27 '25 07:06 pramodbindal

@adambkaplan I tried the config my side by deleting pruner/keep from tektonconfig and it works fine. Can you please try this on latest operator your side as well. if still an issue. I will provide the fix

pramodbindal avatar Jul 01 '25 07:07 pramodbindal

I doubt we'll be able to reproduce this on the Shipwright side, since we now apply default pruner settings in our operator. It isn't worth the effort to spin up an old version of the operator to verify this fix.

adambkaplan avatar Jul 02 '25 14:07 adambkaplan

@adambkaplan If you can let me know the steps I can try it my side.

pramodbindal avatar Jul 03 '25 06:07 pramodbindal