kubevela icon indicating copy to clipboard operation
kubevela copied to clipboard

Feat: enhanced horizontal scaling of app controller

Open fourierr opened this issue 2 years ago • 8 comments

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

fourierr avatar Aug 13 '22 10:08 fourierr

Codecov Report

Merging #4618 (2b18dc4) into master (cea9ef5) will decrease coverage by 20.35%. The diff coverage is n/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.

codecov[bot] avatar Aug 13 '22 10:08 codecov[bot]

  1. how this PR be tested?
  2. please also add some test cases
  3. 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

fourierr avatar Aug 13 '22 13:08 fourierr

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?

fourierr avatar Aug 14 '22 08:08 fourierr

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.

wonderflow avatar Aug 14 '22 13:08 wonderflow

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.

Somefive avatar Aug 15 '22 02:08 Somefive

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 don't think we should re-sue app.oam.dev/controller-version-require in this case, reasons:

  1. Users can't understand the annotation, this is mainly for inner apiserver to use.
  2. 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.

wonderflow avatar Aug 15 '22 05:08 wonderflow

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 don't think we should re-sue app.oam.dev/controller-version-require in this case, reasons:

  1. Users can't understand the annotation, this is mainly for inner apiserver to use.
  2. 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.

Somefive avatar Aug 15 '22 08:08 Somefive

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.

fourierr avatar Aug 21 '22 00:08 fourierr

Duplicated with #4705

Somefive avatar Oct 08 '22 02:10 Somefive