kubevela
kubevela copied to clipboard
Feat: enhanced horizontal scaling of app controller
Signed-off-by: Xiangbo Ma [email protected]
Description of your changes
Fixes https://github.com/kubevela/kubevela/issues/4596
Add sharding for app controller, and legacy app cr will be processed by sharding-1. I have:
- [x] Read and followed KubeVela's contribution process.
- [ ] Related Docs updated properly. In a new feature or configuration option, an update to the documentation is necessary.
- [x] Run
make reviewable
to ensure this PR is ready for review. - [x] Added
backport release-x.y
labels to auto-backport this PR if necessary.
How has this code been tested
Special notes for your reviewer
Codecov Report
Merging #4618 (2b18dc4) into master (cea9ef5) will decrease coverage by
20.35%
. The diff coverage isn/a
.
@@ Coverage Diff @@
## master #4618 +/- ##
===========================================
- Coverage 60.85% 40.49% -20.36%
===========================================
Files 322 84 -238
Lines 31725 11978 -19747
===========================================
- Hits 19307 4851 -14456
+ Misses 9912 6278 -3634
+ Partials 2506 849 -1657
Flag | Coverage Δ | |
---|---|---|
apiserver-e2etests | ? |
|
apiserver-unittests | 40.49% <ø> (+0.05%) |
:arrow_up: |
core-unittests | ? |
|
e2e-multicluster-test | ? |
|
e2e-rollout-tests | ? |
|
e2etests | ? |
Flags with carried forward coverage won't be shown. Click here to find out more.
Impacted Files | Coverage Δ | |
---|---|---|
pkg/apiserver/domain/service/service.go | 0.00% <0.00%> (-92.31%) |
:arrow_down: |
pkg/apiserver/interfaces/api/velaql.go | 4.16% <0.00%> (-83.34%) |
:arrow_down: |
pkg/apiserver/interfaces/api/payload_types.go | 5.55% <0.00%> (-77.78%) |
:arrow_down: |
pkg/apiserver/domain/repository/envbinding.go | 0.00% <0.00%> (-72.10%) |
:arrow_down: |
pkg/apiserver/interfaces/api/oam_application.go | 1.53% <0.00%> (-67.70%) |
:arrow_down: |
pkg/apiserver/interfaces/api/addon.go | 1.13% <0.00%> (-67.62%) |
:arrow_down: |
pkg/apiserver/interfaces/api/webhook.go | 4.34% <0.00%> (-65.22%) |
:arrow_down: |
pkg/apiserver/server.go | 0.00% <0.00%> (-64.59%) |
:arrow_down: |
pkg/velaql/providers/query/utils.go | 14.81% <0.00%> (-62.97%) |
:arrow_down: |
pkg/apiserver/interfaces/api/application.go | 0.10% <0.00%> (-61.45%) |
:arrow_down: |
... and 299 more |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
- how this PR be tested?
- please also add some test cases
- please add docs for this feature
I plan to create three app cr,
- the first cr was default and no mark,
- the second cr was marked with
sharding-1
. - the third cr was marked with
sharding-2
. The expected result is: - the first cr was processed successful,
- the second cr has not been processed,
- the third cr has not been processed. How about this testing case? @wonderflow
At first, I wanted to set multiple shardings through helm -- set
, but in most cases, only one app controller was deployed at the beginning. Later, it was found that it could not be satisfied, and then the app controller was gradually added sharding. In this case, helm cannot implement it, so I plan to increase the number of shardings gradually through vela sharding add
. How about vela sharding add
?
The sharding controller only consumes those apps with sharding annotation. What's blocked on helm installation?
The default behavior is the default controller don't consume apps with sharding annotation.
I have some design questions for this PR. I understand that sharding could help make controller horizontally scalable. But technically, I think controller-requirement
can already satisfy the need of sharding. The only problem is the shared leader election lock cause multiple controllers cannot work concurrently.
Therefore, I think it might not be necessary to add app.oam.dev/sharding
for applications. It should be able to reuse app.oam.dev/controller-version-require
.
I have some design questions for this PR. I understand that sharding could help make controller horizontally scalable. But technically, I think
controller-requirement
can already satisfy the need of sharding. The only problem is the shared leader election lock cause multiple controllers cannot work concurrently.Therefore, I think it might not be necessary to add
app.oam.dev/sharding
for applications. It should be able to reuseapp.oam.dev/controller-version-require
.
I don't think we should re-sue app.oam.dev/controller-version-require
in this case, reasons:
- Users can't understand the annotation, this is mainly for inner apiserver to use.
- In some cases, it could be possible that we both these two Annotations, for example, a sharding key for some BU while there're two versions of controller to rollout.
I have some design questions for this PR. I understand that sharding could help make controller horizontally scalable. But technically, I think
controller-requirement
can already satisfy the need of sharding. The only problem is the shared leader election lock cause multiple controllers cannot work concurrently. Therefore, I think it might not be necessary to addapp.oam.dev/sharding
for applications. It should be able to reuseapp.oam.dev/controller-version-require
.I don't think we should re-sue
app.oam.dev/controller-version-require
in this case, reasons:
- Users can't understand the annotation, this is mainly for inner apiserver to use.
- In some cases, it could be possible that we both these two Annotations, for example, a sharding key for some BU while there're two versions of controller to rollout.
I see. Then the implementation is relatively reasonable for me.
The sharding controller only consumes those apps with sharding annotation. What's blocked on helm installation?
The default behavior is the default controller don't consume apps with sharding annotation.
I didn't think there was any blocks to helm.
Duplicated with #4705