Updates tektonPipeline to make it independent of tektonConfig #600

PuneetPunamiya · 2022-01-28T10:56:38Z

Changes

With the addition of operator version config map operators-info
getting created in tektonConfig, the targetNamespace was getting
created in tektonConfig reconciler, but if tektonConfig is not installed
and tektonPipeline is supposed to installed alone then it won't
be installed as it would miss the targetNamespace.
So this patch checks for the targetNamespace in tektonPipeline
reconciler, if it is not present then it creates the targetNamespace
and also the operator version config map i.e. makes tektonPipeline
independent of tektonConfig

Signed-off-by: Puneet Punamiya [email protected]

Submitter Checklist

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

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

tekton-robot · 2022-01-28T10:59:56Z

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/transformers.go	73.8%	71.4%	-2.4

savitaashture · 2022-02-01T11:25:43Z

@nikhil-thomas I have verified this PR resolve TargetNamespace issue
Its good to merge
Thank you @PuneetPunamiya 👍

pkg/reconciler/kubernetes/tektonpipeline/tektonpipeline.go

pkg/reconciler/common/transformers.go

pkg/reconciler/kubernetes/tektonpipeline/tektonpipeline.go

nikhil-thomas · 2022-02-01T23:22:41Z

@nikhil-thomas I have verified this PR resolve TargetNamespace issue Its good to merge Thank you @PuneetPunamiya 👍

Thanks @savitaashture 👍

@PuneetPunamiya Functionally this patch looks great. I have a few comments regarding code organization. We shall merge this once those are addressed. 🧑‍💻

tekton-robot · 2022-02-02T05:16:59Z

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

tekton-robot · 2022-02-02T05:43:27Z

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

tekton-robot · 2022-02-02T05:43:49Z

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

pkg/reconciler/common/targetnamespace.go

savitaashture · 2022-02-03T07:49:25Z

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

Can you add UT for this

PuneetPunamiya · 2022-02-07T11:58:42Z

/hold

tekton-robot · 2022-02-07T12:00:57Z

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

vinamra28 · 2022-02-09T04:49:23Z

pkg/reconciler/shared/tektonconfig/tektonconfig.go

@@ -219,6 +194,10 @@ func (r *Reconciler) ensureTargetNamespaceExists(ctx context.Context, tc *v1alph
 			}
 		}
 	} else {
+		labels := map[string]string{"operator.tekton.dev/targetNamespace": "true"}


I can see that the following label already exists at line https://github.com/tektoncd/operator/pull/600/files#diff-de56e5fd636d3b9b5ab99eb6197436045c5bc27187a3f59649bae7d66a8e7202R19, so do we still need to pass this label from here?

So from the tektonConfig I have removed but in the tektonPipelines we need it

why is it not needed in tektonConfig. I was thinking we will need it in both TektonConfig and TektonPipeline.

It is needed in tektonConfig as well, but the reason I've remove this variable because, in the createTargetNamespace function itself I'm adding that label while creating the namespace. For ref: please see here

pkg/reconciler/common/targetnamespace.go

tekton-robot · 2022-02-09T10:51:52Z

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/targetnamespace.go	Do not exist	64.7%

tekton-robot · 2022-02-21T05:13:54Z

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/targetnamespace.go	Do not exist	66.7%

tekton-robot · 2022-02-21T06:00:56Z

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/targetnamespace.go	Do not exist	64.7%

PuneetPunamiya · 2022-02-21T07:21:36Z

/test pull-tekton-operator-integration-tests

tekton-robot · 2022-02-21T07:51:50Z

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/targetnamespace.go	Do not exist	66.7%

pkg/reconciler/common/targetnamespace.go

tekton-robot · 2022-02-22T06:44:51Z

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/targetnamespace.go	Do not exist	66.7%

PuneetPunamiya · 2022-02-22T08:11:18Z

/hold cancel

pkg/reconciler/common/targetnamespace.go

tekton-robot · 2022-02-23T04:53:22Z

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/targetnamespace.go	Do not exist	66.7%

tekton-robot · 2022-02-23T05:39:55Z

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/targetnamespace.go	Do not exist	66.7%

PuneetPunamiya · 2022-02-23T06:23:55Z

/test pull-tekton-operator-integration-tests

pkg/reconciler/kubernetes/tektonpipeline/tektonpipeline.go

pkg/reconciler/shared/tektonconfig/tektonconfig.go

tekton-robot · 2022-02-23T08:31:51Z

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/targetnamespace.go	Do not exist	66.7%

tekton-robot · 2022-02-24T11:05:49Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: savitaashture, vdemeester

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~~ [savitaashture,vdemeester]

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

pkg/reconciler/common/targetnamespace.go

- With the addition of operator version config map getting created in tektonConfig, the targetNamespace was getting created in tektonConfig reconciler, but if tektonConfig is not installed and tektonPipeline is supposed to installed alone then it won't be installed as it would miss the targetNamespace. So this patch checks for the targetNamespace in tektonPipeline reconciler, if it is not present then it creates the targetNamespace and also the operator version config map i.e. makes tektonPipeline independent of tektonConfig Signed-off-by: Puneet Punamiya <[email protected]>

tekton-robot · 2022-02-25T03:38:22Z

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/targetnamespace.go	Do not exist	66.7%

tekton-robot requested review from concaf and pradeepitm12 January 28, 2022 10:56

tekton-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 28, 2022

savitaashture approved these changes Feb 1, 2022

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 1, 2022

nikhil-thomas reviewed Feb 1, 2022

View reviewed changes

pkg/reconciler/kubernetes/tektonpipeline/tektonpipeline.go Outdated Show resolved Hide resolved

pkg/reconciler/common/transformers.go Outdated Show resolved Hide resolved

pkg/reconciler/kubernetes/tektonpipeline/tektonpipeline.go Outdated Show resolved Hide resolved

PuneetPunamiya force-pushed the update-tp branch from ce3c09d to c80fb40 Compare February 2, 2022 05:13

PuneetPunamiya force-pushed the update-tp branch 2 times, most recently from 9aade37 to 38bd9a2 Compare February 2, 2022 05:40

savitaashture reviewed Feb 3, 2022

View reviewed changes

pkg/reconciler/common/targetnamespace.go Show resolved Hide resolved

tekton-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 5, 2022

PuneetPunamiya force-pushed the update-tp branch from 38bd9a2 to f982a72 Compare February 7, 2022 11:58

tekton-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 7, 2022

tekton-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 7, 2022

vinamra28 reviewed Feb 9, 2022

View reviewed changes

pkg/reconciler/common/targetnamespace.go Show resolved Hide resolved

PuneetPunamiya force-pushed the update-tp branch from f982a72 to 8106861 Compare February 9, 2022 10:49

tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Feb 9, 2022

PuneetPunamiya force-pushed the update-tp branch from 4b36022 to cd2b281 Compare February 21, 2022 05:58

PuneetPunamiya force-pushed the update-tp branch from cd2b281 to 2e5673d Compare February 21, 2022 07:49

savitaashture reviewed Feb 22, 2022

View reviewed changes

pkg/reconciler/common/targetnamespace.go Show resolved Hide resolved

PuneetPunamiya force-pushed the update-tp branch from 2e5673d to d0ab42a Compare February 22, 2022 06:42

vinamra28 mentioned this pull request Feb 22, 2022

Ship Tekton Hub via Tekton Operator #467

Merged

3 tasks

tekton-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 22, 2022

sm43 reviewed Feb 23, 2022

View reviewed changes

pkg/reconciler/common/targetnamespace.go Outdated Show resolved Hide resolved

PuneetPunamiya force-pushed the update-tp branch from d0ab42a to 0051cdd Compare February 23, 2022 04:51

PuneetPunamiya force-pushed the update-tp branch from 0051cdd to 617022b Compare February 23, 2022 05:37

piyush-garg reviewed Feb 23, 2022

View reviewed changes

pkg/reconciler/kubernetes/tektonpipeline/tektonpipeline.go Outdated Show resolved Hide resolved

piyush-garg reviewed Feb 23, 2022

View reviewed changes

pkg/reconciler/shared/tektonconfig/tektonconfig.go Outdated Show resolved Hide resolved

PuneetPunamiya force-pushed the update-tp branch from 617022b to cf46a04 Compare February 23, 2022 08:29

vdemeester approved these changes Feb 24, 2022

View reviewed changes

sm43 reviewed Feb 25, 2022

View reviewed changes

pkg/reconciler/common/targetnamespace.go Outdated Show resolved Hide resolved

PuneetPunamiya force-pushed the update-tp branch from cf46a04 to 529511c Compare February 25, 2022 03:36

sm43 added the lgtm Indicates that a PR is ready to be merged. label Feb 25, 2022

tekton-robot merged commit 277d2cd into tektoncd:main Feb 25, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updates tektonPipeline to make it independent of tektonConfig #600

Updates tektonPipeline to make it independent of tektonConfig #600

PuneetPunamiya commented Jan 28, 2022 •

edited

Loading

tekton-robot commented Jan 28, 2022

savitaashture commented Feb 1, 2022 •

edited

Loading

nikhil-thomas commented Feb 1, 2022

tekton-robot commented Feb 2, 2022

tekton-robot commented Feb 2, 2022

tekton-robot commented Feb 2, 2022

savitaashture commented Feb 3, 2022

PuneetPunamiya commented Feb 7, 2022

tekton-robot commented Feb 7, 2022

vinamra28 Feb 9, 2022

PuneetPunamiya Feb 9, 2022

nikhil-thomas Feb 21, 2022

PuneetPunamiya Feb 21, 2022 •

edited

Loading

tekton-robot commented Feb 9, 2022

tekton-robot commented Feb 21, 2022

tekton-robot commented Feb 21, 2022

PuneetPunamiya commented Feb 21, 2022

tekton-robot commented Feb 21, 2022

tekton-robot commented Feb 22, 2022

PuneetPunamiya commented Feb 22, 2022

tekton-robot commented Feb 23, 2022

tekton-robot commented Feb 23, 2022

PuneetPunamiya commented Feb 23, 2022

tekton-robot commented Feb 23, 2022

tekton-robot commented Feb 24, 2022

tekton-robot commented Feb 25, 2022

Updates tektonPipeline to make it independent of tektonConfig #600

Updates tektonPipeline to make it independent of tektonConfig #600

Conversation

PuneetPunamiya commented Jan 28, 2022 • edited Loading

Changes

Submitter Checklist

Release Notes

tekton-robot commented Jan 28, 2022

savitaashture commented Feb 1, 2022 • edited Loading

nikhil-thomas commented Feb 1, 2022

tekton-robot commented Feb 2, 2022

tekton-robot commented Feb 2, 2022

tekton-robot commented Feb 2, 2022

savitaashture commented Feb 3, 2022

PuneetPunamiya commented Feb 7, 2022

tekton-robot commented Feb 7, 2022

vinamra28 Feb 9, 2022

Choose a reason for hiding this comment

PuneetPunamiya Feb 9, 2022

Choose a reason for hiding this comment

nikhil-thomas Feb 21, 2022

Choose a reason for hiding this comment

PuneetPunamiya Feb 21, 2022 • edited Loading

Choose a reason for hiding this comment

tekton-robot commented Feb 9, 2022

tekton-robot commented Feb 21, 2022

tekton-robot commented Feb 21, 2022

PuneetPunamiya commented Feb 21, 2022

tekton-robot commented Feb 21, 2022

tekton-robot commented Feb 22, 2022

PuneetPunamiya commented Feb 22, 2022

tekton-robot commented Feb 23, 2022

tekton-robot commented Feb 23, 2022

PuneetPunamiya commented Feb 23, 2022

tekton-robot commented Feb 23, 2022

tekton-robot commented Feb 24, 2022

tekton-robot commented Feb 25, 2022

PuneetPunamiya commented Jan 28, 2022 •

edited

Loading

savitaashture commented Feb 1, 2022 •

edited

Loading

PuneetPunamiya Feb 21, 2022 •

edited

Loading