Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove deprecated Conditions CRD/functionality #4942

Merged
merged 1 commit into from
Jun 7, 2022

Conversation

abayer
Copy link
Contributor

@abayer abayer commented Jun 6, 2022

Changes

Closes #3377

This was deprecated in v0.16.0, and is scheduled to be fully removed in v0.37.0, releasing late in June.

/kind cleanup

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

  • Docs included if any changes are user facing
  • Tests included if any functionality added or changed
  • Follows the commit message standard
  • Meets the Tekton contributor standards (including
    functionality, content, code)
  • Release notes block below has been filled in
    (if there are no user facing changes, use release note "NONE")

Release Notes

action required: Deprecated `conditions` in pipelines removed. Existing pipelines using `conditions` will need to be updated.

@tekton-robot tekton-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jun 6, 2022
@tekton-robot tekton-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jun 6, 2022
@abayer
Copy link
Contributor Author

abayer commented Jun 6, 2022

/assign @lbernick
/assign @afrittoli
/assign @vdemeester
/assign @jerop

@abayer
Copy link
Contributor Author

abayer commented Jun 6, 2022

I need to add the release note, but it's 7pm here and I wanted to open this PR right before I flee to eat. =)

@tekton-robot
Copy link
Collaborator

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

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1alpha1/pipeline_conversion.go 91.6% 91.4% -0.2
pkg/apis/pipeline/v1beta1/pipeline_types.go 96.6% 96.5% -0.1
pkg/apis/pipeline/v1beta1/pipelinerun_types.go 81.1% 86.5% 5.5
pkg/reconciler/pipelinerun/pipelinerun.go 86.4% 86.1% -0.3
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go 92.0% 93.4% 1.4
pkg/reconciler/pipelinerun/resources/pipelinerunstate.go 97.2% 97.4% 0.2
test/controller.go 24.0% 25.3% 1.3
test/v1alpha1/controller.go 37.5% 39.5% 2.0

Copy link
Member

@lbernick lbernick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

Thanks Andrew! Make sure you remove the Condition CRD from config/, and grep for it in the docs (there are a number of other places where it exists in the docs)

@abayer
Copy link
Contributor Author

abayer commented Jun 6, 2022

@lbernick Yeah, this is still a draft for a reason. =)

@tekton-robot
Copy link
Collaborator

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

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1alpha1/pipeline_conversion.go 91.6% 91.4% -0.2
pkg/apis/pipeline/v1beta1/pipeline_types.go 96.6% 96.5% -0.1
pkg/apis/pipeline/v1beta1/pipelinerun_types.go 81.1% 86.5% 5.5
pkg/reconciler/pipelinerun/pipelinerun.go 86.4% 86.1% -0.3
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go 92.0% 93.4% 1.4
pkg/reconciler/pipelinerun/resources/pipelinerunstate.go 97.2% 97.4% 0.2
test/controller.go 24.0% 25.3% 1.3
test/v1alpha1/controller.go 37.5% 39.5% 2.0

@jerop
Copy link
Member

jerop commented Jun 7, 2022

excited that we're finally removing conditions, thank you @abayer 🎉

@tekton-robot
Copy link
Collaborator

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

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1alpha1/pipeline_conversion.go 91.6% 91.4% -0.2
pkg/apis/pipeline/v1beta1/pipeline_types.go 96.6% 96.5% -0.1
pkg/apis/pipeline/v1beta1/pipelinerun_types.go 81.1% 86.5% 5.5
pkg/reconciler/pipelinerun/pipelinerun.go 86.4% 86.1% -0.3
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go 92.0% 93.4% 1.4
pkg/reconciler/pipelinerun/resources/pipelinerunstate.go 97.2% 97.4% 0.2
test/controller.go 24.0% 25.3% 1.3
test/v1alpha1/controller.go 37.5% 39.5% 2.0

@tekton-robot
Copy link
Collaborator

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

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1alpha1/pipeline_conversion.go 91.6% 91.4% -0.2
pkg/apis/pipeline/v1beta1/pipeline_types.go 96.6% 96.5% -0.1
pkg/apis/pipeline/v1beta1/pipelinerun_types.go 81.1% 86.5% 5.5
pkg/reconciler/pipelinerun/pipelinerun.go 86.4% 86.1% -0.3
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go 92.0% 93.4% 1.4
pkg/reconciler/pipelinerun/resources/pipelinerunstate.go 97.2% 97.4% 0.2
test/controller.go 24.0% 25.3% 1.3
test/v1alpha1/controller.go 37.5% 39.5% 2.0

@abayer
Copy link
Contributor Author

abayer commented Jun 7, 2022

/retest

@tekton-robot tekton-robot added release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jun 7, 2022
abayer added a commit to abayer/plumbing that referenced this pull request Jun 7, 2022
Starting with Pipeline v0.37.0, after tektoncd/pipeline#4942 merges, `conditions` will no longer be part of Pipeline. This removes references to `conditions` from the release tooling.

Signed-off-by: Andrew Bayer <[email protected]>
@abayer abayer marked this pull request as ready for review June 7, 2022 19:49
@tekton-robot tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 7, 2022
@tekton-robot
Copy link
Collaborator

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

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1alpha1/pipeline_conversion.go 91.6% 91.4% -0.2
pkg/apis/pipeline/v1beta1/pipeline_types.go 96.6% 96.5% -0.1
pkg/apis/pipeline/v1beta1/pipelinerun_types.go 81.1% 86.5% 5.5
pkg/reconciler/pipelinerun/pipelinerun.go 86.4% 86.1% -0.3
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go 92.0% 93.4% 1.4
pkg/reconciler/pipelinerun/resources/pipelinerunstate.go 97.2% 97.4% 0.2
test/controller.go 24.0% 25.3% 1.3
test/v1alpha1/controller.go 37.5% 39.5% 2.0

docs/conditions.md Show resolved Hide resolved
pkg/apis/pipeline/v1alpha1/pipeline_types.go Outdated Show resolved Hide resolved
@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 7, 2022
Closes tektoncd#3377

This was deprecated in v0.16.0, and is scheduled to be fully removed in v0.37.0, releasing late in June.

Signed-off-by: Andrew Bayer <[email protected]>
Copy link
Member

@jerop jerop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you @abayer 🎉

@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jerop, lbernick

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:

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

@jerop
Copy link
Member

jerop commented Jun 7, 2022

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jun 7, 2022
@jerop
Copy link
Member

jerop commented Jun 7, 2022

@abayer we need to update the deprecations table (remove conditions) - https://github.com/abayer/tektoncd-pipeline/blob/remove-conditions/docs/deprecations.md

@abayer
Copy link
Contributor Author

abayer commented Jun 7, 2022

/retest

@tekton-robot
Copy link
Collaborator

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

File Old Coverage New Coverage Delta
pkg/apis/pipeline/v1alpha1/pipeline_conversion.go 91.6% 91.4% -0.2
pkg/apis/pipeline/v1alpha1/pipeline_types.go 72.7% 83.3% 10.6
pkg/apis/pipeline/v1alpha1/pipeline_validation.go 97.0% 96.9% -0.1
pkg/apis/pipeline/v1beta1/pipeline_types.go 96.6% 96.5% -0.1
pkg/apis/pipeline/v1beta1/pipelinerun_types.go 81.1% 86.5% 5.5
pkg/reconciler/pipelinerun/pipelinerun.go 86.4% 86.1% -0.3
pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go 92.0% 93.4% 1.4
pkg/reconciler/pipelinerun/resources/pipelinerunstate.go 97.2% 97.4% 0.2
test/controller.go 24.0% 25.3% 1.3
test/v1alpha1/controller.go 37.5% 39.5% 2.0

@@ -50,7 +50,7 @@ var (
)

type updateStatusTaskRunsData struct {
withConditions map[string]*v1beta1.PipelineRunTaskRunStatus
noTaskRuns map[string]*v1beta1.PipelineRunTaskRunStatus
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

withConditions to noTaskRuns? 🤔 What is this intended to test?

@@ -78,7 +67,6 @@ type ResolvedPipelineRunTask struct {
PipelineTask *v1beta1.PipelineTask
ResolvedTaskResources *resources.ResolvedTaskResources
// ConditionChecks ~~TaskRuns but for evaling conditions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be deleted too? 🤔

@pritidesai
Copy link
Member

    stream.go:238: Invalid log format for pod tekton-pipelines-controller-868b496cf4-bpqz7: {"level":"info","ts":"2022-06-07T21:56:12.144Z","logger":"tekton-pipelines-controller","caller":"pipelinerun/pipelinerun.go:860","msg":"Creating a new Run object pipeline-run-custom-task-timeout-njpnomco-custom-task-ref","commit":"03322f3","knative.dev/controller":"github.com.tektoncd.pipeline.pkg.reconciler.pipelinerun.Reconciler","knative.dev/kind":"tekton.dev.PipelineRun","knative.dev/traceid":"22af9bdc-516d-48bf-90b9-22ec3aedb35f","knative.dev/key":"arendelle-vx9qh/pipeline-run-custom-task-timeout-njpnomco"}

🤔

@abayer
Copy link
Contributor Author

abayer commented Jun 7, 2022

/retest

@abayer
Copy link
Contributor Author

abayer commented Jun 7, 2022

That’s nothing to worry about - it’s just #4777.

@tekton-robot tekton-robot merged commit 9d60d0a into tektoncd:main Jun 7, 2022
jerop added a commit to jerop/pipeline that referenced this pull request Jun 8, 2022
`Conditions` were removed in tektoncd#4942.
In this change, we remove the deprecation and removal notice for `Conditions`
from the deprecations table. We also remaining references to `Conditions`.

Related issue: tektoncd#3377.
tekton-robot pushed a commit that referenced this pull request Jun 8, 2022
`Conditions` were removed in #4942.
In this change, we remove the deprecation and removal notice for `Conditions`
from the deprecations table. We also remaining references to `Conditions`.

Related issue: #3377.
jerop added a commit to jerop/pipeline that referenced this pull request Jun 23, 2022
In tektoncd#4942, we removed
`Conditions`. However, there was some logic that was left over.
In this change, we clean up the remaining logic for `Conditions`.
jerop added a commit to jerop/pipeline that referenced this pull request Jun 23, 2022
In tektoncd#4942, we removed
`Conditions`. However, there was some logic that was left over.
In this change, we clean up the remaining logic for `Conditions`.

Issue: tektoncd#3377
jerop added a commit to jerop/pipeline that referenced this pull request Jun 23, 2022
In tektoncd#4942, we removed
`Conditions`. However, there was some logic that was left over.
In this change, we clean up the remaining logic for `Conditions`.

Issue: tektoncd#3377
@jerop jerop mentioned this pull request Jun 23, 2022
3 tasks
tekton-robot pushed a commit that referenced this pull request Jun 23, 2022
In #4942, we removed
`Conditions`. However, there was some logic that was left over.
In this change, we clean up the remaining logic for `Conditions`.

Issue: #3377
tekton-robot pushed a commit to tektoncd/plumbing that referenced this pull request Jun 24, 2022
Starting with Pipeline v0.37.0, after tektoncd/pipeline#4942 merges, `conditions` will no longer be part of Pipeline. This removes references to `conditions` from the release tooling.

Signed-off-by: Andrew Bayer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm Indicates that a PR is ready to be merged. release-note-action-required Denotes a PR that introduces potentially breaking changes that require user action. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecate Conditions CRD
7 participants