Support br-federation-manager managing resources in specific-namespace tidb clusters #5410

michaelmdeng · 2023-11-21T22:04:54Z

What problem does this PR solve?

Our infra setup deploys a given TiDB cluster across multiple k8s clusters, so we use federation to manage br resources across these k8s clusters. Additionally, we deploy multiple of these TiDB clusters in a single k8s cluster across different namespaces -- for ease of deployment/upgrade and least-access, we want to deploy the br-federation-manager control plane independently for each of these TiDB clusters to manage volume backups/restores in separate namespaces.

What is changed and how does it work?

This PR applies two sets of changes that enable br-federation-manager (bfm) to run in a namespace-scoped way.

Support clusterScoped in helm charts. When clusterScoped=false, charts generate manifests for Roles and RoleBindings in the specified namespace rather than ClusterRoles and ClusterRoleBindings.
Support clusterScoped in the bfm executable. When -cluster-scoped=false, the manager will listen to changes in br resources only within the specified namespace rather than cluster-wide.

I modeled these changes after how clusterScoped is used in the controller-manager helm charts and executable.

These changes only affect how the control-plane access resources in its namespace and does not affect how bfm manages data-plane resources in other k8s clusters using provided kubeconfig. Managing data-plane resources continues to be done through .spec.clusters.tcNamespace in the federation CRDs.

Code changes

Has Go code change
Has CI related scripts change

Tests

Unit test
E2E test
Manual test

Manual testing to set up bfm in a namespace-scoped way on a control plane cluster and validate it attempts to reconcile namespaced br resources and does not attempt to reconcile br resources in other namespaces.

Set up a Kubernetes cluster for testing.
Apply CRDs

cd $TIDB_OPERATOR_REPO
kubectl create -f manifests/crd.yaml
kubectl apply -f manifests/federation-crd.yaml

Set up two namespaces, one for the bfm control-plane and one other (representing other namespaces for other tidb clusters)

apiVersion: v1
kind: Namespace
metadata:
  name: cluster1
---
apiVersion: v1
kind: Namespace
metadata:
  name: cluster2

Set up bfm kubeconfig following directions here. This can be mocked as we won't be testing managing data-plane resources
Build image and install namespace-scoped bfm

cd $TIDB_OPERATOR_REPO
make build
cd images/br-federation-manager
docker build -t {bfm-image} .

cd $TIDB_OPERATOR_REPO
helm install --namespace cluster1 br-federation-manager charts/br-federation --set image={bfm-image} --set clusterScoped=false

Create an empty backup in cluster1 namespace control plane.

apiVersion: federation.pingcap.com/v1alpha1
kind: VolumeBackup
metadata:
  name: basic-volumebackup
spec: {}

Validate that deployment attempts to reconcile this backup.

...
2023-11-21T16:32:34.313084051Z I1121 16:32:34.312972       1 fed_volume_backup_controller.go:176] VolumeBackup object cluster1/cluster1-volumebackup enqueue
2023-11-21T16:32:34.410123093Z I1121 16:32:34.410044       1 backup_manager.go:53] sync VolumeBackup cluster1/cluster1-volumebackup
2023-11-21T16:32:34.410132551Z I1121 16:32:34.410078       1 backup_manager.go:120] VolumeBackup cluster1/cluster1-volumebackup set status running
2023-11-21T16:32:34.410134801Z I1121 16:32:34.410090       1 fed_volume_backup_controller.go:126] Finished syncing VolumeBackup "cluster1/cluster1-volumebackup" (78.25µs)
...

Repeat validation for VolumeRestore and VolumeBackupSchedule
Create br resources in other namespace.

apiVersion: federation.pingcap.com/v1alpha1
kind: VolumeBackup
metadata:
  name: cluster2-volumebackup
  namespace: cluster2
spec: {}
---
apiVersion: federation.pingcap.com/v1alpha1
kind: VolumeRestore
metadata:
  name: cluster2-volumerestore
  namespace: cluster2
spec: {}

Validate that bfm does not register those changes and does not attempt to reconcile those resources.

No code

Side effects

Breaking backward compatibility
Other side effects:

Related changes

Need to cherry-pick to the release branch
Need to update the documentation

Release Notes

Please refer to Release Notes Language Style Guide before writing the release note.

sre-bot · 2023-11-21T22:05:02Z

All committers have signed the CLA.

ti-chi-bot · 2023-11-21T22:05:04Z

Welcome @michaelmdeng! It looks like this is your first PR to pingcap/tidb-operator 🎉

codecov-commenter · 2023-11-23T02:46:59Z

Codecov Report

Merging #5410 (63f115c) into master (8ebc21f) will increase coverage by 5.75%.
The diff coverage is 9.09%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5410      +/-   ##
==========================================
+ Coverage   61.43%   67.19%   +5.75%     
==========================================
  Files         228      239      +11     
  Lines       29093    32875    +3782     
==========================================
+ Hits        17873    22090    +4217     
+ Misses       9463     8988     -475     
- Partials     1757     1797      +40

Flag	Coverage Δ
e2e	`46.12% <0.00%> (?)`
unittest	`61.40% <9.09%> (-0.04%)`	⬇️

BornChanger · 2023-11-23T13:59:42Z

/cc @WangLe1321 @csuzhangxc

WangLe1321

LGTM

BornChanger · 2023-11-27T06:23:18Z

/retest

BornChanger · 2023-11-27T08:17:45Z

/run-pull-e2e-kind-br

BornChanger · 2023-11-27T14:13:37Z

/test-pull-e2e-kind-serial

BornChanger · 2023-11-27T14:17:22Z

/retest

BornChanger · 2023-11-28T05:47:26Z

/retest

BornChanger · 2023-11-28T13:42:44Z

/retest

michaelmdeng · 2023-11-28T19:14:46Z

@BornChanger just pushed a fix to the failing tests, can I get some help rerunning?

BornChanger · 2023-11-29T03:50:29Z

/retest

BornChanger · 2023-11-29T04:09:39Z

@michaelmdeng please sign the CLA as required.

BornChanger · 2023-11-29T04:09:51Z

BornChanger · 2023-11-29T09:38:01Z

/retest

`clusterScoped` controls whether the manager should control resources cluster-wide or only scoped to the specific namespace

michaelmdeng · 2023-11-29T17:54:44Z

@michaelmdeng please sign the CLA as required.

woops, used the wrong email in the commit, should be fixed now

BornChanger · 2023-11-30T00:17:46Z

/run-pull-e2e-kind-across-kubernetes

WangLe1321 · 2023-12-01T03:53:40Z

/run-pull-e2e-kind-br

WangLe1321 · 2023-12-01T03:58:40Z

/run-pull-e2e-kind

WangLe1321 · 2023-12-01T03:58:51Z

/run-pull-e2e-kind-across-kubernetes

WangLe1321 · 2023-12-01T06:52:30Z

/run-pull-e2e-kind

WangLe1321 · 2023-12-01T06:52:41Z

/run-pull-e2e-kind-basic

WangLe1321 · 2023-12-01T06:52:50Z

/run-pull-e2e-kind-serial

ti-chi-bot · 2023-12-01T06:53:58Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BornChanger, WangLe1321

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~~ [BornChanger,WangLe1321]

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

ti-chi-bot · 2023-12-01T06:54:01Z

[LGTM Timeline notifier]

Timeline:

2023-11-24 06:19:25.661358538 +0000 UTC m=+557994.326584733: ☑️ agreed by WangLe1321.
2023-11-24 06:19:40.224198575 +0000 UTC m=+558008.889424771: ✖️🔁 reset by ti-chi-bot[bot].
2023-11-27 03:19:16.254858274 +0000 UTC m=+806384.920084470: ☑️ agreed by BornChanger.
2023-11-27 14:16:08.670247675 +0000 UTC m=+845797.335473871: ✖️🔁 reset by BornChanger.
2023-12-01 06:54:00.245699502 +0000 UTC m=+1164868.910925682: ☑️ agreed by WangLe1321.

WangLe1321 · 2023-12-01T07:59:56Z

/merge

ti-chi-bot · 2023-12-01T07:59:59Z

@WangLe1321: We have migrated to builtin LGTM and approve plugins for reviewing.

Please use /approve when you want approve this pull request.

The changes announcement: LGTM plugin changes

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 ti-community-infra/tichi repository.

WangLe1321 · 2023-12-01T08:37:28Z

/cherry-pick release-1.5

ti-chi-bot · 2023-12-01T08:38:06Z

@WangLe1321: new pull request created to branch release-1.5: #5434.

In response to this:

/cherry-pick release-1.5

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 ti-community-infra/tichi repository.

…e tidb clusters (#5410) (#5434) Co-authored-by: Michael Deng <[email protected]>

ti-chi-bot bot requested review from howardlau1999 and shonge November 21, 2023 22:04

ti-chi-bot bot added contribution first-time-contributor labels Nov 21, 2023

ti-chi-bot bot added the size/M label Nov 21, 2023

ti-chi-bot bot requested review from csuzhangxc and WangLe1321 November 23, 2023 13:59

BornChanger added the area/ebs-br label Nov 23, 2023

WangLe1321 approved these changes Nov 24, 2023

View reviewed changes

ti-chi-bot bot added lgtm approved and removed lgtm labels Nov 24, 2023

BornChanger approved these changes Nov 27, 2023

View reviewed changes

ti-chi-bot bot added the lgtm label Nov 27, 2023

ti-chi-bot bot removed the lgtm label Nov 27, 2023

michaelmdeng force-pushed the mdeng/upstream/namespaced-bfm branch from eb8f36c to ac4e15e Compare November 29, 2023 17:49

ti-chi-bot bot added size/XXL and removed size/M labels Nov 29, 2023

michaelmdeng added 2 commits November 29, 2023 09:51

feat: Add clusterScoped flag to br-federation-manager

e64477f

`clusterScoped` controls whether the manager should control resources cluster-wide or only scoped to the specific namespace

fix: remove extraneous namespace in rolebinding

af6fc1e

michaelmdeng force-pushed the mdeng/upstream/namespaced-bfm branch from ac4e15e to af6fc1e Compare November 29, 2023 17:51

ti-chi-bot bot added size/M and removed size/XXL labels Nov 29, 2023

ti-chi-bot bot added 2 commits November 30, 2023 09:47

Merge branch 'master' into mdeng/upstream/namespaced-bfm

608c08b

Merge branch 'master' into mdeng/upstream/namespaced-bfm

63f115c

WangLe1321 approved these changes Dec 1, 2023

View reviewed changes

ti-chi-bot bot added the lgtm label Dec 1, 2023

ti-chi-bot bot merged commit 6c87ec1 into pingcap:master Dec 1, 2023
11 checks passed

ti-chi-bot mentioned this pull request Dec 1, 2023

Support br-federation-manager managing resources in specific-namespace tidb clusters (#5410) #5434

Merged

10 tasks

csuzhangxc pushed a commit that referenced this pull request Dec 1, 2023

Support br-federation-manager managing resources in specific-namespac…

cddb804

…e tidb clusters (#5410) (#5434) Co-authored-by: Michael Deng <[email protected]>

BornChanger mentioned this pull request Dec 1, 2023

EBS BR support to multiple k8s cluster TiDB #5003

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support br-federation-manager managing resources in specific-namespace tidb clusters #5410

Support br-federation-manager managing resources in specific-namespace tidb clusters #5410

michaelmdeng commented Nov 21, 2023 •

edited

Loading

sre-bot commented Nov 21, 2023 •

edited

Loading

ti-chi-bot bot commented Nov 21, 2023

codecov-commenter commented Nov 23, 2023 •

edited

Loading

BornChanger commented Nov 23, 2023

WangLe1321 left a comment

BornChanger commented Nov 27, 2023

BornChanger commented Nov 27, 2023

BornChanger commented Nov 27, 2023

BornChanger commented Nov 27, 2023

BornChanger commented Nov 28, 2023

BornChanger commented Nov 28, 2023

michaelmdeng commented Nov 28, 2023

BornChanger commented Nov 29, 2023

BornChanger commented Nov 29, 2023

BornChanger commented Nov 29, 2023

BornChanger commented Nov 29, 2023

michaelmdeng commented Nov 29, 2023

BornChanger commented Nov 30, 2023

WangLe1321 commented Dec 1, 2023

WangLe1321 commented Dec 1, 2023

WangLe1321 commented Dec 1, 2023

WangLe1321 commented Dec 1, 2023

WangLe1321 commented Dec 1, 2023

WangLe1321 commented Dec 1, 2023

ti-chi-bot bot commented Dec 1, 2023

ti-chi-bot bot commented Dec 1, 2023

WangLe1321 commented Dec 1, 2023

ti-chi-bot bot commented Dec 1, 2023

WangLe1321 commented Dec 1, 2023

ti-chi-bot commented Dec 1, 2023

Support br-federation-manager managing resources in specific-namespace tidb clusters #5410

Support br-federation-manager managing resources in specific-namespace tidb clusters #5410

Conversation

michaelmdeng commented Nov 21, 2023 • edited Loading

What problem does this PR solve?

What is changed and how does it work?

Code changes

Tests

Side effects

Related changes

Release Notes

sre-bot commented Nov 21, 2023 • edited Loading

ti-chi-bot bot commented Nov 21, 2023

codecov-commenter commented Nov 23, 2023 • edited Loading

Codecov Report

BornChanger commented Nov 23, 2023

WangLe1321 left a comment

Choose a reason for hiding this comment

BornChanger commented Nov 27, 2023

BornChanger commented Nov 27, 2023

BornChanger commented Nov 27, 2023

BornChanger commented Nov 27, 2023

BornChanger commented Nov 28, 2023

BornChanger commented Nov 28, 2023

michaelmdeng commented Nov 28, 2023

BornChanger commented Nov 29, 2023

BornChanger commented Nov 29, 2023

BornChanger commented Nov 29, 2023

BornChanger commented Nov 29, 2023

michaelmdeng commented Nov 29, 2023

BornChanger commented Nov 30, 2023

WangLe1321 commented Dec 1, 2023

WangLe1321 commented Dec 1, 2023

WangLe1321 commented Dec 1, 2023

WangLe1321 commented Dec 1, 2023

WangLe1321 commented Dec 1, 2023

WangLe1321 commented Dec 1, 2023

ti-chi-bot bot commented Dec 1, 2023

ti-chi-bot bot commented Dec 1, 2023

[LGTM Timeline notifier]

WangLe1321 commented Dec 1, 2023

ti-chi-bot bot commented Dec 1, 2023

WangLe1321 commented Dec 1, 2023

ti-chi-bot commented Dec 1, 2023

michaelmdeng commented Nov 21, 2023 •

edited

Loading

sre-bot commented Nov 21, 2023 •

edited

Loading

codecov-commenter commented Nov 23, 2023 •

edited

Loading