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

Add Helm chart for installing Tekton Operator #627

Merged
merged 1 commit into from
May 16, 2022

Conversation

jacksgt
Copy link
Contributor

@jacksgt jacksgt commented Feb 18, 2022

Hello Tekton operators!

Changes

As discussed in #543 , the tekton-operator could really use a Helm chart for installation.
Helm is widely used in the Kubernetes ecosystem and allows for easy configuration of the installed components.
At the same time it gives much more flexibility when compared to installing from OperatorHub.

We are using an internal Helm chart for deploying the Operator, so I thought I'd share the chart here.

I have not added extensive documentation for it yet, but will do before merging this PR.

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

Add Helm chart for installing Tekton Operator

@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 18, 2022
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Feb 18, 2022

CLA Signed

The committers are authorized under a signed CLA.

@tekton-robot tekton-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 18, 2022
@tekton-robot
Copy link
Contributor

Hi @jacksgt. Thanks for your PR.

I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@dirien
Copy link

dirien commented Feb 18, 2022

Cool finally we have a helm chart! Thanks @jacksgt!

@sm43
Copy link
Member

sm43 commented Feb 23, 2022

Hi @jacksgt
welcome to the tekton community 👋🏽 🙂
Could you please sign the Easy CLA? that would be required for getting this pr in😄

chart/crds/openshift.yaml Outdated Show resolved Hide resolved
@sm43
Copy link
Member

sm43 commented Feb 23, 2022

I have not added extensive documentation for it yet, but will do before merging this PR

yep documentation would be useful to understand and test it out 👍🏽 🙂

@jacksgt
Copy link
Contributor Author

jacksgt commented Feb 25, 2022

welcome to the tekton community 👋🏽 slightly_smiling_face

Hi, thanks!

Could you please sign the Easy CLA? that would be required for getting this pr in

Sure, done!

yep documentation would be useful to understand and test it out 👍🏽 slightly_smiling_face

I have added a README. Let me know if the instructions are clear for you.

chart/templates/service.yaml Outdated Show resolved Hide resolved
@sm43
Copy link
Member

sm43 commented Mar 2, 2022

another reason it is failing on openshift is #643

chart/README.md Show resolved Hide resolved
@jacksgt
Copy link
Contributor Author

jacksgt commented Mar 2, 2022

Thanks for taking a look and giving the chart a try Shivam!
I addressed the comments above.

another reason it is failing on openshift is #643

I have also change the default target namespace based on the flavor: unless explicitly overridden, it now uses openshift-pipelines for Openshift and tekton-pipelines for Kubernetes.

@sm43
Copy link
Member

sm43 commented Mar 4, 2022

@jacksgt

name: tekton-config-defaults
configmap is not getting created ? on both k8s and openshift

@jacksgt
Copy link
Contributor Author

jacksgt commented Mar 14, 2022

@jacksgt

name: tekton-config-defaults

configmap is not getting created ? on both k8s and openshift

Those settings are directly injected as environment variables into the Tekton operator: https://github.com/tektoncd/operator/pull/627/files#diff-2c6c478bf17d4896de57d0def131d88f926a02e3a0159954b1e1457a19b78bebR54

@nikhil-thomas
Copy link
Member

@jacksgt 💯 👍 for this pr.
I'm testing this right now. I shall come back with my questions and comments in a couple of days. 🧑‍💻

please excuse the delay. we got a bit busy trying to handle the new additions like TektonHub and TektonChains in operator. 🕺

@sm43
Copy link
Member

sm43 commented Apr 14, 2022

@jacksgt sorry for the delay in reviews.
I tried but its failed on k8s and openshift.
can you update it to latest operator version and update the chart
I will try it and then we can merge.
We will also need to think how to manage the chart with new release?
some kind of automation ..?
how to add this the chart in operator release?

@jacksgt
Copy link
Contributor Author

jacksgt commented Apr 28, 2022

Hi @sm43 , sorry for the late reply, I finally got around to addressing your feedback.

I tried but its failed on k8s and openshift.

There was a minor issue in the YAML names. I fixed it and verified that it deploys on k8s and openshift.

can you update it to latest operator version and update the chart

Yes, done. Also rebased on latest main.

I will try it and then we can merge.

Please give it another go :-)

We will also need to think how to manage the chart with new release?
some kind of automation ..?

Yes, I can address this in a follow-up MR. I guess you have some kind of script for creating a release, we just need to add a few commands there (for updating the appVersion and chart version).

Copy link
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

/ok-to-test

@tekton-robot tekton-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 29, 2022
@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

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 Apr 29, 2022
@nikhil-thomas
Copy link
Member

/retest

as #769 has merged, ci should pass

@sm43
Copy link
Member

sm43 commented May 4, 2022

/retest

@jacksgt
Copy link
Contributor Author

jacksgt commented May 13, 2022

Hello folks, the tests passed, is there anything else left to do for this PR? :-)

@vdemeester
Copy link
Member

@jacksgt can you squash your commits ?
@sm43 can you take a last look ?

Once it is squash, I am more than ok to get this merge, and iterate over it if there is some bugs, etc.

@sm43
Copy link
Member

sm43 commented May 13, 2022

Thanks! I just tried, there are couple of things to fix

  • in 0.56.0 there are 2 new CRDs added, hub and chains
  • the operator deployment are created with the tag v0.56.0 but it should be 0.56.0

once those are fix, the chart works fine
good to merge !

@jacksgt
Copy link
Contributor Author

jacksgt commented May 13, 2022

Thanks! I just tried, there are couple of things to fix

Thanks for taking the time to check again!

* in 0.56.0 there are 2 new CRDs added, [hub](https://github.com/tektoncd/operator/blob/main/config/base/300-operator_v1alpha1_hub_crd.yaml) and [chains](https://github.com/tektoncd/operator/blob/main/config/base/300-operator_v1alpha1_chain_crd.yaml)

Thanks for the hint! I synced the CRDs with the YAMLs from release 0.57

* the operator deployment are created with the tag `v0.56.0` but it should be `0.56.0`

This seems to be an inconsistency with the 0.56 release.
I bumped the version to v0.57.0 (and also rebased on main), so now it should be fine.

I'll make sure to submit a follow-up MR that will automatically sync the CRDs for each new release and bump the chart version.

@sm43
Copy link
Member

sm43 commented May 14, 2022

for 0.57.0 you will need to include 9819f1d this change too ! then we are good to merge 🙃

Signed-off-by: Jack Henschel <jackdev@mailbox.org>
@jacksgt
Copy link
Contributor Author

jacksgt commented May 16, 2022

for 0.57.0 you will need to include 9819f1d this change too ! then we are good to merge upside_down_face

Ohhh thanks for the hint, very kind of you!
I missed that change.

I updated the chart accordingly.

@sm43
Copy link
Member

sm43 commented May 16, 2022

/test pull-tekton-operator-go-coverage

@vdemeester
Copy link
Member

/retest
/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label May 16, 2022
@tekton-robot tekton-robot merged commit 8dad037 into tektoncd:main May 16, 2022
@tekton-robot
Copy link
Contributor

@jacksgt: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
pull-tekton-operator-go-coverage 8ba04c9 link /test pull-tekton-operator-go-coverage

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

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. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note Denotes a PR that will be considered when it comes time to generate release notes. 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.

6 participants