Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support promote dependent resources automatically #4135

Merged
merged 1 commit into from
Nov 23, 2023

Conversation

zhy76
Copy link
Member

@zhy76 zhy76 commented Oct 16, 2023

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:
Part of #4036

Special notes for your reviewer:
WIP
Does this PR introduce a user-facing change?:

`karmadactl`: Introduced `--dependencies` flag to `promote` command, which will be used to promote dependencies.

@karmada-bot karmada-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 16, 2023
@karmada-bot karmada-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 16, 2023
@codecov-commenter
Copy link

codecov-commenter commented Oct 16, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1b2c6ed) 52.78% compared to head (682adff) 52.78%.
Report is 34 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4135      +/-   ##
==========================================
- Coverage   52.78%   52.78%   -0.01%     
==========================================
  Files         239      240       +1     
  Lines       23584    23624      +40     
==========================================
+ Hits        12448    12469      +21     
- Misses      10460    10475      +15     
- Partials      676      680       +4     
Flag Coverage Δ
unittests 52.78% <ø> (-0.01%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@zhy76 zhy76 force-pushed the promote branch 3 times, most recently from c25c09d to 5781685 Compare October 16, 2023 17:19
@jwcesign
Copy link
Member

/assign

@zhy76
Copy link
Member Author

zhy76 commented Oct 23, 2023

Effect verification

Native k8s resources automatically promote dependence

test file:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: myapp
  labels:
    app: myapp
spec:
  replicas: 1
  selector:
    matchLabels:
      app: myapp
  template:
    metadata:
      labels:
        app: myapp
    spec:
      containers:
        - image: nginx
          name: nginx
          volumeMounts:
            - name: configmap
              mountPath: "/configmap"
      volumes:
        - name: configmap
          configMap:
            name: my-config
---
apiVersion: v1
kind: ConfigMap
metadata:
  name: my-config
data:
  nginx.properties: |
    proxy-connect-timeout: "10s"
    proxy-read-timeout: "10s"
    client-max-body-size: "2m"
  1. Deploy Deployment and Configmap in the member1 cluster
    image
    image

  2. Use '-d=true' to promote deploy to the control plane
    karmadactl promote deployment myapp -C member1 --kubeconfig=/root/.kube/karmada.config -n default -d

  3. Result
    At first, deploy is not deployed on the control plane. The preceding command successfully promotes deploy and its dependent resource configmap.
    image
    image
    image
    rb and work are good
    image
    If a dependent resource exists on the control plane, the promotion of the dependent resource is skipped.
    image

  4. After deploy for the control plane is deleted, deploy for the member cluster is also deleted.
    image

Lua configuration mode resources promote dependent resources

  1. Install openkruise crd first
# Firstly add openkruise charts repository if you haven't do this.
$ helm repo add openkruise https://openkruise.github.io/charts/

# [Optional]
$ helm repo update

# Install the latest version.
$ helm install kruise openkruise/kruise --set featureGates="TemplateNoDefaults=true" --set  manager.image.repository=openkruise-registry.cn-shanghai.cr.aliyuncs.com/openkruise/kruise-manager

# Those that have been installed need to be upgraded
$ helm upgrade kruise openkruise/kruise --set featureGates="TemplateNoDefaults=true" --set  manager.image.repository=openkruise-registry.cn-shanghai.cr.aliyuncs.com/openkruise/kruise-manager
  1. create cloneset cr
    # kubectl apply -f desired-cloneset-nginx.yaml
apiVersion: apps.kruise.io/v1alpha1
kind: CloneSet
metadata:
  labels:
    app: sample
  name: sample
  namespace: test-cloneset
spec:
  replicas: 4
  selector:
    matchLabels:
      app: sample
  template:
    metadata:
      labels:
        app: sample
    spec:
      volumes:
        - name: configmap
          configMap:
            name: my-sample-config    
      containers:
        - name: nginx
          image: nginx:alpine
---
apiVersion: v1
kind: ConfigMap
metadata:
  name: my-sample-config
  namespace: test-cloneset
data:
  nginx.properties: |
    proxy-connect-timeout: "10s"
    proxy-read-timeout: "10s"
    client-max-body-size: "2m"

image
image

  1. The first step is to promote cloneset's crd to the control plane.
    ./karmadactl promote crd clonesets.apps.kruise.io -C member2 --kubeconfig=/root/.kube/karmada.config -d=false
    image

  2. Promote namespace to the control plane.
    ./karmadactl promote namespace test-cloneset -C member2 --kubeconfig=/root/.kube/karmada.config -d=false
    image

  3. Promote cr with '-d=true' and its dependence on the control plane.
    ./karmadactl promote cloneset sample -C member2 --kubeconfig=/root/.kube/karmada.config -n test-cloneset -d
    image

  4. Result
    You can see that cr and its dependent cm have been promoted to the control plane.
    image
    image
    resourcebinding
    image
    clusterresourcebinding
    image

@carlory
Copy link
Member

carlory commented Oct 24, 2023

when the PR is ready to review, please drop the WIP prefix of the title and I will back. Thank you.

@zhy76 zhy76 changed the title (WIP) feat: support promote dependent resources automatically feat: support promote dependent resources automatically Oct 24, 2023
@karmada-bot karmada-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 24, 2023
pkg/karmadactl/promote/promote.go Outdated Show resolved Hide resolved
pkg/karmadactl/promote/promote.go Outdated Show resolved Hide resolved
@zhy76 zhy76 force-pushed the promote branch 2 times, most recently from 8f9df5e to 66f72d0 Compare November 15, 2023 15:04
@zhy76
Copy link
Member Author

zhy76 commented Nov 15, 2023

test file:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: myapp
  labels:
    app: myapp
spec:
  replicas: 1
  selector:
    matchLabels:
      app: myapp
  template:
    metadata:
      labels:
        app: myapp
    spec:
      containers:
        - image: nginx
          name: nginx
          volumeMounts:
            - name: configmap-1
              mountPath: "/configmap-1"
            - name: configmap-2
              mountPath: "/configmap-2"
      volumes:
        - name: configmap-1
          configMap:
            name: my-config-1
        - name: configmap-2
          configMap:
            name: my-config-2
---
apiVersion: v1
kind: ConfigMap
metadata:
  name: my-config-1
data:
  nginx.properties: |
    proxy-connect-timeout: "10s"
    proxy-read-timeout: "10s"
    client-max-body-size: "2m"
---
apiVersion: v1
kind: ConfigMap
metadata:
  name: my-config-2
data:
  nginx.properties: |
    proxy-connect-timeout: "10s"
    proxy-read-timeout: "10s"
    client-max-body-size: "2m"

If promote dependency my-config-2 failed, it will revert:
image
In control-plane will only has the resource, but no its dependency:
image
In member cluster, the configmap all remained:
image

@zhy76 zhy76 force-pushed the promote branch 2 times, most recently from 16f0163 to fb4d917 Compare November 15, 2023 15:12
Signed-off-by: zhy76 <958474674@qq.com>
@jwcesign
Copy link
Member

/cc @RainbowMango
Please take a look, it's ok for me now

Copy link
Member

@RainbowMango RainbowMango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/assign

@RainbowMango
Copy link
Member

@chaunceyjiang @yizhang-zen do you have any further comments?

Copy link
Member

@chaunceyjiang chaunceyjiang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work~

/lgtm

@karmada-bot karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Nov 23, 2023
Copy link
Member

@RainbowMango RainbowMango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

@karmada-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RainbowMango

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:

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

@karmada-bot karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 23, 2023
@karmada-bot karmada-bot merged commit ec35998 into karmada-io:master Nov 23, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants