Feat: add support for envbinding with namespace selector #2432

Somefive · 2021-10-09T09:00:19Z

Description of your changes

Previous EnvBinding (cluster-gateway mode) only support ClusterSelector (only cluster name is supported). Other legacy modes (like single-cluster & ocm) breaks as workflow is reworked. In this PR, we support NamespaceSelector (only namespace name currently) in EnvBinding. Now we can simultaneously set ClusterSelector and NamespaceSelector in one environment or use them separately. For example, we can deploy application in prod namespace in sub worker cluster. Example:

apiVersion: core.oam.dev/v1beta1
kind: Application
metadata:
  name: example-app
  namespace: default
spec:
  components:
    - name: hello-world-server
      type: webservice
      properties:
        image: crccheck/hello-world
        port: 8000
      traits:
        - type: scaler
          properties:
            replicas: 1
    - name: data-worker
      type: worker
      properties:
        image: busybox
        cmd:
          - sleep
          - '1000000'
  policies:
    - name: example-multi-env-policy
      type: env-binding
      properties:
        envs:
          - name: test
            placement: # selecting the namespace to deploy to
              namespaceSelector:
                name: test
            selector:
              components:
                - data-worker

          - name: staging
            placement: # selecting the cluster to deploy to
              clusterSelector:
                name: cluster-worker

          - name: prod
            placement: # selecting both namespace and cluster to deploy to
              clusterSelector:
                name: cluster-worker
              namespaceSelector:
                name: prod
            patch: # overlay patch on above components
              components:
                - name: hello-world-server
                  type: webservice
                  traits:
                    - type: scaler
                      properties:
                        replicas: 3

  workflow:
    steps:
      # deploy to test env
      - name: deploy-test
        type: deploy2env
        properties:
          policy: example-multi-env-policy
          env: test

      # deploy to staging env
      - name: deploy-staging
        type: deploy2env
        properties:
          policy: example-multi-env-policy
          env: staging

      # deploy to prod env
      - name: deploy-prod
        type: deploy2env
        properties:
          policy: example-multi-env-policy
          env: prod

I have:

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.
Run make reviewable to ensure this PR is ready for review.
Added backport release-x.y labels to auto-backport this PR if necessary.

Future work from this PR:

Enable LabelSelector to select cluster/namespace. This feature might need be accompanied by VelaCLI support so that user can manage namespaces and clusters conveniently.
Enable support for multi-region deployment as described in [Feature] Multi-datacenters Deployment #2385. This can be decomposed into two parts.
a. Support hierarchical grouping for deploy locations.
b. Support workload orchestration across namespace/cluster/region.

How has this code been tested

Special notes for your reviewer

Somefive · 2021-10-09T09:09:17Z

The injection of namespace in #ApplyComponent is not very elegant. It is now closely coupled with multi-cluster and ApplyComponent. Any recommendation for the code architecture? @leejanee @hongchaodeng

codecov · 2021-10-09T09:21:42Z

Codecov Report

Merging #2432 (0011c45) into master (bcd7f3e) will increase coverage by 0.07%.
The diff coverage is 71.87%.

@@            Coverage Diff             @@
##           master    #2432      +/-   ##
==========================================
+ Coverage   61.13%   61.20%   +0.07%     
==========================================
  Files         160      160              
  Lines       16997    17014      +17     
==========================================
+ Hits        10391    10414      +23     
+ Misses       5490     5484       -6     
  Partials     1116     1116

Flag	Coverage Δ
e2e-multicluster-test	`19.88% <71.87%> (+0.05%)`	⬆️
e2e-rollout-tests	`31.27% <15.62%> (+0.01%)`	⬆️
e2etests	`37.18% <15.62%> (-0.03%)`	⬇️
unittests	`55.46% <15.62%> (+0.02%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
....dev/v1alpha1/envbinding/cluster_gateway_engine.go	`73.33% <66.66%> (+14.50%)`	⬆️
...ler/core.oam.dev/v1alpha2/application/generator.go	`79.24% <75.00%> (-0.37%)`	⬇️
pkg/workflow/providers/oam/apply.go	`77.27% <100.00%> (+1.66%)`	⬆️
...es/policydefinition/policydefinition_controller.go	`60.25% <0.00%> (-2.57%)`	⬇️
pkg/controller/utils/capability.go	`79.85% <0.00%> (-1.47%)`	⬇️
pkg/cue/definition/template.go	`59.05% <0.00%> (-0.87%)`	⬇️
pkg/cue/model/value/value.go	`84.75% <0.00%> (+0.74%)`	⬆️
...pis/core.oam.dev/v1alpha1/zz_generated.deepcopy.go	`23.84% <0.00%> (+1.77%)`	⬆️
...m.dev/v1alpha1/envbinding/envbinding_controller.go	`72.89% <0.00%> (+3.61%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bcd7f3e...0011c45. Read the comment docs.

hongchaodeng

lgtm after nit

hongchaodeng · 2021-10-11T03:35:41Z

pkg/controller/core.oam.dev/v1alpha1/envbinding/cluster_gateway_engine.go

@@ -32,6 +32,11 @@ import (
 	"github.com/oam-dev/kubevela/pkg/multicluster"
 )

+const (
+	// OverrideNamespaceLabelKey identifies the override namespace for patched Application


override namespace 是用来干什么的？解释一下？

在原始不设置 namespace 的情况下，EnvBinding 的每个 Env 下的 Application 里的 Component 可以跨 Namespace 放置资源。这里的 Override Namespace 的意思是用户可以在 Env 里面指定 Namespace 让对应的资源全都放置在 Env 指定的 Namespace 下。

hongchaodeng · 2021-10-11T03:37:55Z

pkg/controller/core.oam.dev/v1alpha1/envbinding/cluster_gateway_engine.go

 func (engine *ClusterGatewayEngine) prepare(ctx context.Context, configs []v1alpha1.EnvConfig) error {
 	engine.clusterDecisions = make(map[string]v1alpha1.ClusterDecision)
-	clusterNameToConfig := make(map[string]string)
+	locationToConfig := make(map[string]string)


locationToConfig 是做什么的？还有下面这段 forloop 代码太复杂了，需要重构下成更小的函数，用函数名来介绍下做什么的。

这里是做一个简单的重复检查，如果有两个 Env 它们指向了同一个 Cluster 的同一个 Namespace，就会报错。

在这个 forloop 里加了一些注释来标识每段代码的作用

hongchaodeng · 2021-10-11T03:39:26Z

test/e2e-multicluster-test/testdata/app/example-envbinding-app.yaml

-              clusterSelector:
-                name: local
+          - name: test
+            placement: # selecting the namespace to deploy to


这里需要解释一下 namespace 是指子集群的 namespace

FogDong

在 docs/example 里面加个例子？

github-actions · 2021-10-11T06:52:29Z

Successfully created backport PR #2440 for release-1.1.

Somefive requested review from FogDong, hongchaodeng, leejanee and wonderflow as code owners October 9, 2021 09:00

Somefive force-pushed the multicluster_ns branch 2 times, most recently from ed4f276 to 4f2442a Compare October 9, 2021 09:06

Feat: add support for envbinding with namespace selector

409fe1b

Somefive force-pushed the multicluster_ns branch from 4f2442a to 409fe1b Compare October 9, 2021 09:07

Fix: service account kubeconfig e2e-test

db75396

Somefive force-pushed the multicluster_ns branch from 9d879f3 to db75396 Compare October 11, 2021 02:40

Somefive requested a review from yangsoon October 11, 2021 03:19

hongchaodeng approved these changes Oct 11, 2021

View reviewed changes

Docs: add comments to explain functions

c9bf6b8

FogDong approved these changes Oct 11, 2021

View reviewed changes

Docs: add envbinding example

0011c45

Somefive added the backport release-1.1 label Oct 11, 2021

yangsoon approved these changes Oct 11, 2021

View reviewed changes

Somefive merged commit 472ed3a into kubevela:master Oct 11, 2021

github-actions bot mentioned this pull request Oct 11, 2021

[Backport release-1.1] Feat: add support for envbinding with namespace selector #2440

Merged

Somefive deleted the multicluster_ns branch November 24, 2021 02:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat: add support for envbinding with namespace selector #2432

Feat: add support for envbinding with namespace selector #2432

Somefive commented Oct 9, 2021 •

edited

Somefive commented Oct 9, 2021

codecov bot commented Oct 9, 2021 •

edited

hongchaodeng left a comment

hongchaodeng Oct 11, 2021

Somefive Oct 11, 2021

hongchaodeng Oct 11, 2021

Somefive Oct 11, 2021

Somefive Oct 11, 2021

hongchaodeng Oct 11, 2021

Somefive Oct 11, 2021

FogDong left a comment

github-actions bot commented Oct 11, 2021

Feat: add support for envbinding with namespace selector #2432

Feat: add support for envbinding with namespace selector #2432

Conversation

Somefive commented Oct 9, 2021 • edited

Description of your changes

How has this code been tested

Special notes for your reviewer

Somefive commented Oct 9, 2021

codecov bot commented Oct 9, 2021 • edited

Codecov Report

hongchaodeng left a comment

Choose a reason for hiding this comment

hongchaodeng Oct 11, 2021

Choose a reason for hiding this comment

Somefive Oct 11, 2021

Choose a reason for hiding this comment

hongchaodeng Oct 11, 2021

Choose a reason for hiding this comment

Somefive Oct 11, 2021

Choose a reason for hiding this comment

Somefive Oct 11, 2021

Choose a reason for hiding this comment

hongchaodeng Oct 11, 2021

Choose a reason for hiding this comment

Somefive Oct 11, 2021

Choose a reason for hiding this comment

FogDong left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 11, 2021

Somefive commented Oct 9, 2021 •

edited

codecov bot commented Oct 9, 2021 •

edited