Skip to content

Conversation

@nikhil-thomas
Copy link
Member

@nikhil-thomas nikhil-thomas commented Feb 3, 2022

Improve ReadyChecks for Pipelines and Triggers

Add mechanism to detect upgrade scenario in reconcilers and update
status with relevant messages

Minor fixes to ensure TektonAddon reconciler acts more predictably

Signed-off-by: Nikhil Thomas nikthoma@redhat.com

Changes

Submitter Checklist

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

See the contribution guide for more details.

Release Notes

NONE

@tekton-robot tekton-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Feb 3, 2022
@tekton-robot tekton-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 3, 2022
@nikhil-thomas nikhil-thomas force-pushed the fix/tektonaddon-upgrade branch from 8a0557c to 12f630c Compare February 3, 2022 13:06
@tekton-robot tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 3, 2022
@tekton-robot
Copy link
Contributor

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/reconciler/common/initcontroller.go Do not exist 0.0%
pkg/reconciler/openshift/tektonconfig/extension/addon.go 56.5% 56.7% 0.3
pkg/reconciler/shared/tektonconfig/pipeline/pipeline.go 56.5% 56.7% 0.3
pkg/reconciler/shared/tektonconfig/trigger/trigger.go 72.1% 71.2% -0.9

@nikhil-thomas nikhil-thomas force-pushed the fix/tektonaddon-upgrade branch from 12f630c to 65cd60f Compare February 3, 2022 13:16
@tekton-robot
Copy link
Contributor

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/reconciler/common/initcontroller.go Do not exist 0.0%
pkg/reconciler/openshift/tektonconfig/extension/addon.go 56.5% 56.7% 0.3
pkg/reconciler/shared/tektonconfig/pipeline/pipeline.go 56.5% 56.7% 0.3
pkg/reconciler/shared/tektonconfig/trigger/trigger.go 72.1% 71.2% -0.9

@nikhil-thomas nikhil-thomas force-pushed the fix/tektonaddon-upgrade branch from 65cd60f to 9173bc6 Compare February 3, 2022 13:27
@tekton-robot
Copy link
Contributor

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/reconciler/common/initcontroller.go Do not exist 0.0%

@nikhil-thomas nikhil-thomas force-pushed the fix/tektonaddon-upgrade branch from 9173bc6 to f22c38b Compare February 3, 2022 13:37
@nikhil-thomas
Copy link
Member Author

@concaf @vdemeester
ready for review

@tekton-robot
Copy link
Contributor

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/reconciler/common/initcontroller.go Do not exist 0.0%
pkg/reconciler/openshift/tektonconfig/extension/addon.go 56.5% 56.7% 0.3
pkg/reconciler/shared/tektonconfig/pipeline/pipeline.go 56.5% 56.7% 0.3
pkg/reconciler/shared/tektonconfig/trigger/trigger.go 72.1% 71.2% -0.9

@tekton-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vdemeester

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

The pull request process is described here

Details 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

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 3, 2022
@nikhil-thomas nikhil-thomas force-pushed the fix/tektonaddon-upgrade branch from f22c38b to 43603f5 Compare February 3, 2022 13:41
@tekton-robot
Copy link
Contributor

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/reconciler/common/initcontroller.go Do not exist 0.0%
pkg/reconciler/openshift/tektonconfig/extension/addon.go 56.5% 56.7% 0.3
pkg/reconciler/shared/tektonconfig/pipeline/pipeline.go 56.5% 56.7% 0.3
pkg/reconciler/shared/tektonconfig/trigger/trigger.go 72.1% 71.2% -0.9

@nikhil-thomas nikhil-thomas force-pushed the fix/tektonaddon-upgrade branch from 43603f5 to 4ba64b1 Compare February 3, 2022 13:49
@tekton-robot
Copy link
Contributor

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/reconciler/common/initcontroller.go Do not exist 0.0%
pkg/reconciler/openshift/tektonconfig/extension/addon.go 56.5% 56.7% 0.3
pkg/reconciler/shared/tektonconfig/pipeline/pipeline.go 56.5% 56.7% 0.3
pkg/reconciler/shared/tektonconfig/trigger/trigger.go 72.1% 71.2% -0.9

@sm43
Copy link
Member

sm43 commented Feb 3, 2022

=========================================
==== TESTING IF GOLINT HAS BEEN DONE ====
=========================================
pkg/reconciler/kubernetes/tektoninstallerset/const.go:1: : import cycle not allowed in test (typecheck)
/*
==============================
==== GO LINT TESTS FAILED ====
==============================

lint check seems to be failing

LGTM !

@nikhil-thomas nikhil-thomas force-pushed the fix/tektonaddon-upgrade branch from 4ba64b1 to bf79a04 Compare February 4, 2022 06:32
@tekton-robot tekton-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Feb 4, 2022
@tekton-robot
Copy link
Contributor

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/reconciler/common/initcontroller.go Do not exist 0.0%
pkg/reconciler/openshift/tektonconfig/extension/addon.go 56.5% 56.7% 0.3
pkg/reconciler/shared/tektonconfig/pipeline/pipeline.go 56.5% 56.7% 0.3
pkg/reconciler/shared/tektonconfig/trigger/trigger.go 72.1% 71.2% -0.9

@nikhil-thomas nikhil-thomas force-pushed the fix/tektonaddon-upgrade branch from 6a8ec39 to 303f800 Compare February 4, 2022 07:07
@tekton-robot
Copy link
Contributor

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/reconciler/common/initcontroller.go Do not exist 0.0%
pkg/reconciler/openshift/tektonconfig/extension/addon.go 56.5% 56.7% 0.3
pkg/reconciler/shared/tektonconfig/pipeline/pipeline.go 56.5% 56.7% 0.3
pkg/reconciler/shared/tektonconfig/trigger/trigger.go 72.1% 71.2% -0.9

@nikhil-thomas
Copy link
Member Author

/hold cancel

@tekton-robot tekton-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 4, 2022
@nikhil-thomas nikhil-thomas force-pushed the fix/tektonaddon-upgrade branch from 303f800 to 48d4149 Compare February 4, 2022 07:24
@tekton-robot
Copy link
Contributor

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/reconciler/common/initcontroller.go Do not exist 0.0%
pkg/reconciler/openshift/tektonconfig/extension/addon.go 56.5% 56.7% 0.3
pkg/reconciler/shared/tektonconfig/pipeline/pipeline.go 56.5% 56.7% 0.3
pkg/reconciler/shared/tektonconfig/trigger/trigger.go 72.1% 71.2% -0.9

@concaf
Copy link
Contributor

concaf commented Feb 4, 2022

/test pull-tekton-operator-integration-tests

@nikhil-thomas
Copy link
Member Author

/hold

@tekton-robot tekton-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 4, 2022
@piyush-garg
Copy link
Contributor

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 4, 2022
@nikhil-thomas nikhil-thomas force-pushed the fix/tektonaddon-upgrade branch from 48d4149 to 01a222c Compare February 4, 2022 12:01
@tekton-robot tekton-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Feb 4, 2022
@tekton-robot
Copy link
Contributor

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/reconciler/common/initcontroller.go Do not exist 0.0%
pkg/reconciler/openshift/tektonconfig/extension/addon.go 56.5% 56.7% 0.3
pkg/reconciler/shared/tektonconfig/pipeline/pipeline.go 56.5% 56.7% 0.3
pkg/reconciler/shared/tektonconfig/trigger/trigger.go 72.1% 71.2% -0.9

@nikhil-thomas
Copy link
Member Author

/hold cancel

@tekton-robot tekton-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 4, 2022
Improve ReadyChecks for Pipelines and Triggers

Add mechanism to detect upgrade scenario in reconcilers and update
status with relevant messages

Minor fixes to ensure TektonAddon reconciler acts more predictably

Signed-off-by: Nikhil Thomas <nikthoma@redhat.com>
@nikhil-thomas nikhil-thomas force-pushed the fix/tektonaddon-upgrade branch from 01a222c to 16ef7bd Compare February 4, 2022 12:32
@tekton-robot tekton-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 4, 2022
@tekton-robot
Copy link
Contributor

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/reconciler/common/initcontroller.go Do not exist 0.0%
pkg/reconciler/openshift/tektonconfig/extension/addon.go 56.5% 56.7% 0.3
pkg/reconciler/shared/tektonconfig/pipeline/pipeline.go 56.5% 56.7% 0.3
pkg/reconciler/shared/tektonconfig/trigger/trigger.go 72.1% 71.2% -0.9

Copy link
Contributor

@pradeepitm12 pradeepitm12 left a comment

Choose a reason for hiding this comment

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

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 4, 2022
@tekton-robot tekton-robot merged commit e8f205b into tektoncd:main Feb 4, 2022
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. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants