introduce upgrade mechanism and add upgrade patch to v1 storage migration

[jkandasa]

Changes

This PR introduces upgrade mechanism. Which will be executed as two part.

• PreUpgrade - triggers before applying manifests
• PostUpgrade - triggers once TektonConfig CR becomes ready state

marks last applied upgrade reference to TektonConfig CR(via status annotation)

In addition to upgrade base code, this PR includes the following fixes

• fixes: SRVKP-2917 - Handle v1 storage conversion
• fixes: SRVKP-2935 - Upgrade to version 1.9.x results in the following error: Install failed with message: ... status.storedVersions[0]: Invalid value: "v1alpha1": must appear in spec.versions
• fixes: SRVKP-3202 - TektonConfig CR displays message " Install failed with message: tektonpipeline pipeline is not in desired state" after pipelines operator upgrade

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

• Run make test lint before submitting a PR
• Includes tests (if functionality changed/added)
• Includes docs (if user facing)
• Commit messages follow commit message best practices

See the contribution guide for more details.

Release Notes

NONE

[jkandasa]

/retest

[vdemeester]

Few questions:

• Any reason why the generated injection code is removed ? (probably due to the knative.dev/pkg update ?)
• Any reason to have an upgrade/helper package instead of just having things in upgrade ? (helper is a package name we tend to "not use" because it doesn't really mean anything)

Otherwise I like the PR but I think we need to have some unit-tests at least 👼🏼

[jkandasa]

Few questions:

• Any reason why the generated injection code is removed ? (probably due to the knative.dev/pkg update ?)

Yes, I updated the knative.dev/pkg to get knative.dev/pkg/apiextensions

• Any reason to have an upgrade/helper package instead of just having things in upgrade ? (helper is a package name we tend to "not use" because it doesn't really mean anything)

In the future, If we want to call storage migration, we can reuse that function.
Thought of keeping reusable functions inside helper package.
Other than that I have no reason.

Otherwise I like the PR but I think we need to have some unit-tests at least 👼🏼

I was waiting for the initial confirmation about this feature to put effort on unit-tests.
Now I can work on unit-tests and other pending works on this PR.
Thanks @vdemeester 😄

[piyush-garg]

I don't think, this is what we wanted to keep on CR i.e. only 1 version

What I know about the issue is - the fresh installation and old installation can have different version, but we need to update the CRD by removing the old versions and using the new versions. We can't delete and create again, as it will remove user CRs.

[jkandasa]

@piyush-garg My understanding is,

• storage version should contain latest version and may contain old version too
• from the migrate package [1], first updating all the resources to latest version
• then keeps only the latest version under status.storedVersions

[1] - https://github.com/knative/pkg/blob/main/apiextensions/storageversion/migrator.go#L49-L56

[piyush-garg]

i am not sure if this will work like this, because just changing the version will not work for tekton resources, spec also needs to handled accordingly.

what i was expecting from this story was to handle the change of this field https://github.com/tektoncd/pipeline/blob/main/config/300-pipeline.yaml#L27

[concaf]

/kind bug

[piyush-garg]

/approve
/lgtm

we need to properly test this more

[tekton-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: piyush-garg

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [piyush-garg]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[tekton-robot]

The following is the coverage report on the affected files.
Say /test pull-tekton-operator-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/apis/operator/v1alpha1/tektonconfig_lifecycle.go	58.8%	69.6%	10.7
pkg/reconciler/shared/tektonconfig/upgrade/helper/storage_version_migrator.go	Do not exist	71.4%
pkg/reconciler/shared/tektonconfig/upgrade/post_upgrade.go	Do not exist	0.0%
pkg/reconciler/shared/tektonconfig/upgrade/pre_upgrade.go	Do not exist	88.5%
pkg/reconciler/shared/tektonconfig/upgrade/upgrade.go	Do not exist	85.0%

[jkandasa]

/cherry-pick release-v0.68.x

[tekton-robot]

@jkandasa: once the present PR merges, I will cherry-pick it on top of release-v0.68.x in a new PR and assign it to you.

In response to this:

/cherry-pick release-v0.68.x

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[tekton-robot]

@jkandasa: new pull request created: #1687

In response to this:

/cherry-pick release-v0.68.x

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.