Different configmap for Leader Election per deployment by khrm · Pull Request #7014 · tektoncd/pipeline

khrm · 2023-08-02T07:37:18Z

Change configmap for leader election. This is so that we can apply changes per deployment. Also changed the resolvers leader election name so that we can avoid conflict with the controllers config if we install in the same namespace.

Fixes #6979

Changes

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

Has Docs if any changes are user facing, including updates to minimum requirements e.g. Kubernetes version bumps
Has Tests included if any functionality added or changed
Follows the commit message standard
Meets the Tekton contributor standards (including functionality, content, code)
Has a kind label. You can add one by adding a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
Release notes block below has been updated with any user facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings). See some examples of good release notes.
Release notes contains the string "action required" if the change requires additional action from users switching to the new release

Release Notes

Change configmap for LeaderElection of webhook to config-leader-election-webhook, event controller to config-leader-election-event and resolvers to config-leader-election-resolvers

/kind bug

config/resolvers/resolvers-deployment.yaml

khrm · 2023-08-02T07:40:48Z

Can we include this in v0.50.x?

vdemeester

One comment, otherwise LGTM 👼🏼
cc @tektoncd/core-maintainers

config/resolvers/resolvers-deployment.yaml

config/config-leader-election-event.yaml

piyush-garg · 2023-08-02T08:13:58Z

we should change the name for pipelines-controller too, not use the default atleast

khrm

/retest

khrm · 2023-08-14T06:18:43Z

I just remember I need to update roles. That should fix the test failures.

khrm · 2023-08-14T07:00:48Z

I have changed the config name for pipelines-controller also.

khrm · 2023-08-14T12:10:23Z

@vdemeester @lbernick @dibyom Can we get this merged?

vdemeester

/cc @afrittoli @JeromeJu @jerop

khrm · 2023-08-14T13:02:47Z

Should we include this fix in 0.50.2? Without this fix, setting HA for pipelines controller is changing triggers, webhook, events and resolvers (if in same namespace) config also.

tekton-robot · 2023-08-14T14:30:45Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JeromeJu, 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~~ [vdemeester]

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

Yongxuanzhang

nit: I think this doc link needs to be updated?

pipeline/docs/enabling-ha.md

Line 41 in fff25b1

    
           Leader election can be configured in [config-leader-election.yaml](./../config/config-leader-election.yaml). The ConfigMap defines the following parameters:

jerop · 2023-09-20T17:00:32Z

@khrm if you have a moment to update the docs, can review this in time for v0.52 release (added it to the milestone) 🙏🏾

khrm · 2023-09-20T17:47:26Z

@jerop It's quite late. I will update it tomorrow morning.

jerop · 2023-09-20T18:14:42Z

no rush, moved it to the next milestone -- @pritidesai is cutting the release today

Change configmap for leader election. This is so that we can apply changes per deployment. Also changed resolvers leader election name so that we can avoid conflict with controllers config if we install in same namespace.

vdemeester · 2023-10-02T11:13:22Z

/lgtm

vdemeester · 2023-10-02T11:13:52Z

/cherry-pick release-v0.50.x

tekton-robot · 2023-10-02T11:13:53Z

@vdemeester: once the present PR merges, I will cherry-pick it on top of release-v0.50.x in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-v0.50.x

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.

tekton-robot · 2023-10-02T11:15:10Z

@vdemeester: #7014 failed to apply on top of branch "release-v0.50.x":

Applying: Different config map for Leader Election per deployment
Using index info to reconstruct a base tree...
M	config/200-role.yaml
Falling back to patching base and 3-way merge...
Auto-merging config/200-role.yaml
CONFLICT (content): Merge conflict in config/200-role.yaml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 Different config map for Leader Election per deployment
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherry-pick release-v0.50.x

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.

This will update the name of the configmap responsible for controller to contain config of leader election Based on the PR on pipeline upstream tektoncd/pipeline#7014

tekton-robot added kind/bug Categorizes issue or PR as related to a bug. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Aug 2, 2023

tekton-robot requested review from vdemeester and wlynch August 2, 2023 07:37

tekton-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 2, 2023

khrm commented Aug 2, 2023

View reviewed changes

config/resolvers/resolvers-deployment.yaml Show resolved Hide resolved

vdemeester reviewed Aug 2, 2023

View reviewed changes

config/resolvers/resolvers-deployment.yaml Show resolved Hide resolved

config/config-leader-election-event.yaml Outdated Show resolved Hide resolved

khrm force-pushed the b/leaderElectionConfig branch 2 times, most recently from 2001eaa to e045c1c Compare August 2, 2023 08:57

khrm commented Aug 14, 2023

View reviewed changes

khrm force-pushed the b/leaderElectionConfig branch 4 times, most recently from 2e94e09 to d5f6ee9 Compare August 14, 2023 07:00

vdemeester approved these changes Aug 14, 2023

View reviewed changes

tekton-robot requested review from JeromeJu, afrittoli and jerop August 14, 2023 12:49

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 14, 2023

JeromeJu approved these changes Aug 14, 2023

View reviewed changes

Yongxuanzhang reviewed Aug 15, 2023

View reviewed changes

wlynch removed their request for review August 30, 2023 17:41

jerop added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 20, 2023

jerop removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 20, 2023

jerop added this to the Pipelines v0.52 milestone Sep 20, 2023

jerop modified the milestones: Pipelines v0.52, Pipelines v0.53 Sep 20, 2023

Different config map for Leader Election per deployment

89e82fd

Change configmap for leader election. This is so that we can apply changes per deployment. Also changed resolvers leader election name so that we can avoid conflict with controllers config if we install in same namespace.

khrm force-pushed the b/leaderElectionConfig branch from d5f6ee9 to 89e82fd Compare September 29, 2023 10:58

tekton-robot assigned vdemeester Oct 2, 2023

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 2, 2023

tekton-robot merged commit 1e0cea2 into tektoncd:main Oct 2, 2023

piyush-garg mentioned this pull request Nov 28, 2023

Fix config leader election configmap name tektoncd/operator#1881

Merged

4 tasks

Conversation

khrm commented Aug 2, 2023

Changes

Submitter Checklist

Release Notes

Uh oh!

Uh oh!

khrm commented Aug 2, 2023

Uh oh!

vdemeester left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

piyush-garg commented Aug 2, 2023

Uh oh!

khrm left a comment

Choose a reason for hiding this comment

Uh oh!

khrm commented Aug 14, 2023

Uh oh!

khrm commented Aug 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

khrm commented Aug 14, 2023

Uh oh!

vdemeester left a comment

Choose a reason for hiding this comment

Uh oh!

khrm commented Aug 14, 2023

Uh oh!

tekton-robot commented Aug 14, 2023

Uh oh!

Yongxuanzhang left a comment

Choose a reason for hiding this comment

Uh oh!

jerop commented Sep 20, 2023

Uh oh!

khrm commented Sep 20, 2023

Uh oh!

jerop commented Sep 20, 2023

Uh oh!

vdemeester commented Oct 2, 2023

Uh oh!

vdemeester commented Oct 2, 2023

Uh oh!

tekton-robot commented Oct 2, 2023

Uh oh!

tekton-robot commented Oct 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

khrm commented Aug 14, 2023 •

edited

Loading