Skip to content

Adds config map for operator version #563

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.

Sign up for GitHub

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

Jump to bottom
Merged
tekton-robot merged 1 commit into from
Jan 21, 2022
Merged

Adds config map for operator version #563

tekton-robot merged 1 commit into from
Jan 21, 2022

Conversation

@PuneetPunamiya
Copy link
Member

@PuneetPunamiya PuneetPunamiya commented Dec 15, 2021 •
edited
Loading

  • This patch adds a config map which has the operator verison.
    The config map is created in the tektonConfig pre-reconciler

  • First it checks in the tektonConfig reconciler if the targetNs
    exists, if not it creates the targetNs and then in the pipeline
    reconciler if the tektonConfig instance exists then just add the
    labels from the release yamls if the targetNs is tekton-pipelines

Fixes: #539

Signed-off-by: Puneet Punamiya ppunamiy@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

- Adds a config map which has the operator verison
- This config map can be used by `tkn cli` which would give
  the operator version in the output of `tkn version` command

@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. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Dec 15, 2021
@tekton-robot tekton-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 15, 2021
@pradeepitm12
Copy link
Contributor

pradeepitm12 commented Dec 15, 2021

@PuneetPunamiya Thanks for the pr. Can you please add a commit message explaining what this pr is doing!
This not only helps in the review but also to know what things got in by just looking at the commit message.

@PuneetPunamiya
Copy link
Member Author

PuneetPunamiya commented Dec 15, 2021

@PuneetPunamiya Thanks for the pr. Can you please add a commit message explaining what this pr is doing! This not only helps in the review but also to know what things got in by just looking at the commit message.

Yeah sure, since the implementation was not complete yet and I have been working on it hence did just created a wip pr, once the implementation is complete will add a proper commit message which describe what the patch does

@PuneetPunamiya PuneetPunamiya force-pushed the add-cm-operator branch 2 times, most recently from e3a5af9 to 85c9478 Compare December 22, 2021 06:53
@tekton-robot 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 Dec 22, 2021
@PuneetPunamiya PuneetPunamiya force-pushed the add-cm-operator branch 2 times, most recently from 4697ebb to 4daed96 Compare December 22, 2021 07:37
@PuneetPunamiya PuneetPunamiya changed the title [WIP] Adds config map for operator version Adds config map for operator version Dec 22, 2021
@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 Dec 22, 2021
sm43
sm43 reviewed Jan 4, 2022
View reviewed changes
sm43
sm43 reviewed Jan 4, 2022
View reviewed changes
pkg/reconciler/kubernetes/tektonconfig/extension.go Outdated
return []mf.Transformer{}
}
func (oe kubernetesExtension) PreReconcile(context.Context, v1alpha1.TektonComponent) error {
func (oe kubernetesExtension) PreReconcile(ctx context.Context, tc v1alpha1.TektonComponent) error {
Copy link
Member

@sm43 sm43 Jan 4, 2022

Choose a reason for hiding this comment

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

I see configmap for the openshift and k8s
but this code is in k8s extension?
do we not need to add in openshift part?

nikhil-thomas
nikhil-thomas reviewed Jan 5, 2022
View reviewed changes
nikhil-thomas
nikhil-thomas reviewed Jan 5, 2022
View reviewed changes
nikhil-thomas
nikhil-thomas reviewed Jan 5, 2022
View reviewed changes
nikhil-thomas
nikhil-thomas reviewed Jan 5, 2022
View reviewed changes
nikhil-thomas
nikhil-thomas reviewed Jan 5, 2022
View reviewed changes
@PuneetPunamiya PuneetPunamiya force-pushed the add-cm-operator branch 3 times, most recently from 6285408 to 5a47d67 Compare January 11, 2022 10:46
sm43
sm43 reviewed Jan 13, 2022
View reviewed changes
pkg/reconciler/kubernetes/tektonpipeline/tektonpipeline.go Outdated
// Pass the object through defaulting
tp.SetDefaults(ctx)

if err := r.ensureTektonConfigExists(ctx, tp); err != nil {
Copy link
Member

@sm43 sm43 Jan 13, 2022

Choose a reason for hiding this comment

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

what are we trying to do here using this func?

nikhil-thomas
nikhil-thomas reviewed Jan 17, 2022
View reviewed changes
nikhil-thomas
nikhil-thomas reviewed Jan 17, 2022
View reviewed changes
nikhil-thomas
nikhil-thomas reviewed Jan 17, 2022
View reviewed changes
@PuneetPunamiya PuneetPunamiya force-pushed the add-cm-operator branch 2 times, most recently from 7a1397e to 7dec068 Compare January 17, 2022 05:34
@nikhil-thomas
Copy link
Member

nikhil-thomas commented Jan 18, 2022

@PuneetPunamiya we need one modification in one of the Tasks in Operator release pipeline.

A sed command below the already existing ones here:

operator/tekton/build-publish-images-manifests.yaml

Line 122 in 2217ee6

# Rewrite "devel" to params.versionTag

nikhil-thomas
nikhil-thomas reviewed Jan 19, 2022
View reviewed changes
@nikhil-thomas
Copy link
Member

nikhil-thomas commented Jan 19, 2022

@PuneetPunamiya overall this patch looks good.

Could you make 🧑‍💻 3 more changes

  1. Adds config map for operator version #563 (comment)
  2. Adds config map for operator version #563 (comment)
  3. Adds config map for operator version #563 (comment)

nikhil-thomas
nikhil-thomas reviewed Jan 21, 2022
View reviewed changes
@nikhil-thomas
Copy link
Member

nikhil-thomas commented Jan 21, 2022

/approve

@tekton-robot tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 21, 2022
@nikhil-thomas
Copy link
Member

nikhil-thomas commented Jan 21, 2022

✅ lgtm

@nikhil-thomas
Copy link
Member

nikhil-thomas commented Jan 21, 2022 •
edited
Loading

@PuneetPunamiya could you add a release note in the PR description

format

    ```release-note
    release note line(s)
    ```

savitaashture
savitaashture reviewed Jan 21, 2022
View reviewed changes
savitaashture
savitaashture reviewed Jan 21, 2022
View reviewed changes
@PuneetPunamiya
Adds config map for operator version
ae2b0ab 
  - This patch adds a config map which has the operator verison.
    The config map is created in the tektonConfig reconciler before
    the pre-reconciler is called

  - First it checks in the tektonConfig reconciler if the targetNs
    exists, if not it creates the targetNs and then in the pipeline
    reconciler if the tektonConfig instance exists then just add the
    labels from the release yamls if the targetNs is `tekton-pipelines`

Signed-off-by: Puneet Punamiya <ppunamiy@redhat.com>
@tekton-robot
Copy link
Contributor

tekton-robot commented Jan 21, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nikhil-thomas, 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:
  • OWNERS [nikhil-thomas,vdemeester]

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

savitaashture
savitaashture reviewed Jan 21, 2022
View reviewed changes
Copy link
Contributor

@savitaashture savitaashture 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 👍 🎉

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 21, 2022
@tekton-robot tekton-robot merged commit a87f7e4 into tektoncd:main Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@savitaashture savitaashture savitaashture left review comments

@vdemeester vdemeester vdemeester approved these changes

@piyush-garg piyush-garg Awaiting requested review from piyush-garg

@pradeepitm12 pradeepitm12 Awaiting requested review from pradeepitm12

+2 more reviewers

@nikhil-thomas nikhil-thomas nikhil-thomas left review comments

@sm43 sm43 sm43 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@savitaashture savitaashture

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/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add an operator-info config map in "targetNamespace"

7 participants

@PuneetPunamiya @pradeepitm12 @nikhil-thomas @tekton-robot @vdemeester @savitaashture @sm43