TEP-0085: Per-Namespace Controller Configuration [Proposal] #607
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tekton-robot
added
the
size/M
|
thank you @leiyanggz 😸 /assign cc @sbwsg @vdemeester @pritidesai @dibyom |
|
thank you @leiyanggz 👍 |
|
Please add yourself as author and set the status to |
| @@ -79,6 +80,42 @@ As a contributor, I need to test my behavioral changes to ensure that they work | |||
| - Operator can allow for configuration to be defined on per-namespace basis | |||
| - User can specify and use a customized configuration for a given namespace | |||
|
|
|||
| ## Proposal | |||
|
|
|||
| To enable the `tekton-pipelines-controller`'s feature flags configuration per namespace, we propose adding a new `configmap` with the same name `feature-flags` in each of the candidate namespaces, which contains the properties to override for all the `Pipelineruns` created in this specific namespace. | |||
There was a problem hiding this comment.
please add a link to the supported properties in the proposal.
Also, please update the documentation in the implementation PR to include this new configmap once the proposal is merged.
There was a problem hiding this comment.
Since the customization is spreading across the entire namespace, I am guessing all the resources under that namespace will be impacted, including pipelineRun, taskRun, and run.
There was a problem hiding this comment.
I would like for us to use a prefix here, like tekton-feature-flags (similar, tekton-defaults, …). Mainly to make sure we don't run the risk of conflictings with something else in the cluster/namespace that would have a different schema and mean something different.
| scope-when-expressions-to-task: "true" | ||
| ``` | ||
|
|
||
| The candidate namespaces are included in a new environment variable `CONFIG_FEATURE_FLAGS_CUSTOMIZATION_NAMESPACES`. |
There was a problem hiding this comment.
Please add one more sentence explaining this env. variable can be set in the pipeline-controller deployment resource. Thanks for adding example below 👍
What happens when a namespace is listed in the deployment resource but does not exist in the system? Please add a little more explanation on such scenarios.
There was a problem hiding this comment.
just to add to @pritidesai comment it would be great to know what happen when this env variable is modified, describing what happen to existing resources and if only apply to newly created resources.
| scope-when-expressions-to-task: "true" | ||
| ``` | ||
|
|
||
| The candidate namespaces are included in a new environment variable `CONFIG_FEATURE_FLAGS_CUSTOMIZATION_NAMESPACES`. |
There was a problem hiding this comment.
just to add to @pritidesai comment it would be great to know what happen when this env variable is modified, describing what happen to existing resources and if only apply to newly created resources.
| - name: tekton-pipelines-controller | ||
| image: gcr.io/tekton-releases/github.com/tektoncd/pipeline/cmd/controller | ||
| env: | ||
| - name: CONFIG_FEATURE_FLAGS_CUSTOMIZATION_NAMESPACES |
There was a problem hiding this comment.
is it a conscious decision to use a env variable instead of an entry in the tekton-pipelines feature-flags configmap?
There was a problem hiding this comment.
One of the nice benefits of making this a field in the feature-flags configmap would be that adding new namespaces wouldn't require a controller restart.
There was a problem hiding this comment.
- Is this environment variable required or optional ? a.k.a. what happens when it's not present ?
- Do we see value in supporting regular expression here ?
Asking those question as I think one of the main use-case here might be to enable this for all namespace but a certain set of them for example. It wouldn't scale if each time a new namespace is created in the cluster, the cluster-admin need to updated the pipeline deployment spec.
There was a problem hiding this comment.
this raises an extra question if we support regexp/globing, do we want to support negation ? i.e: "tekton-.*" but not "tekton-pipelines"
in the same spriit of negation, maybe it makes sense to say, matches it for every namespaces on the cluster, but not for the admin namespaces like tekton-* openshift-* kube-* or whatever system namespaces we have.
|
/kind tep |
tekton-robot
added
the
kind/tep
|
@leiyanggz - please join us in one of the API WG on Mondays 12pm ET to discuss the review comments in this TEP and move it forward, happy to schedule an ad-hoc meeting if the time doesn't work for you, please let us know :) |
|
/test pull-community-teps-lint |
|
Issues go stale after 90d of inactivity. /lifecycle stale Send feedback to tektoncd/plumbing. |
tekton-robot
added
the
lifecycle/stale
|
Stale issues rot after 30d of inactivity. /lifecycle rotten Send feedback to tektoncd/plumbing. |
tekton-robot
added
lifecycle/rotten
|
Rotten issues close after 30d of inactivity. /close Send feedback to tektoncd/plumbing. |
|
@tekton-robot: Closed this PR. In response to this:
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 will carry this one 😇 |
In TEP-0085 we proposed the support for overriding
tekton-pipelines-controller's configuration on a per-namespace basis.In this pull request, we include the proposal to configure the
feature-flagsconfiguration per namespace which is implemented in Tekton Pipelines Pull Request #4499In summary, we propose enabling the controller's feature flags configuration per namespace via a new
feature-flagsConfigMap in each of the candidate namespaces to override the properties defined in thefeature-flagsConfigMap in the default system namespace, and the candidate namespaces are included in a new environment variable in thetekton-pipelines-controller.