bugfixs of dependencies distributor #1499

mrlihanbo · 2022-03-17T07:07:00Z

What type of PR is this?
/kind bug

What this PR does / why we need it:
Bugfixs of dependencies distributor :

Enqueue the resource binding that has been scheduled to member clusters.
Update Resource info when update attached bindings.
Add some logs.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

`karmada-controller-manager`: Fixed ResourceBinding maybe prevents en-queue in case of schedule failure.

…r clusters Signed-off-by: lihanbo <lihanbo2@huawei.com>

Signed-off-by: lihanbo <lihanbo2@huawei.com>

RainbowMango · 2022-03-17T07:19:58Z

How to reproduce it?
Please add a release note.

mrlihanbo · 2022-03-17T07:48:22Z

To reproduce bug: enqueue the resource binding that has been scheduled to member clusters

create a deployment which references configMap:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: nginx
  labels:
    app: nginx
spec:
  replicas: 1
  selector:
    matchLabels:
      app: nginx
  template:
    metadata:
      labels:
        app: nginx
    spec:
      containers:
      - image: nginx
        name: nginx
        volumeMounts:
        - name: configmap
          mountPath: "/configmap"
      volumes:
      - name: configmap
        configMap:
          name: game-config

create configMap:

apiVersion: v1
kind: ConfigMap
metadata:
  name: game-config
data:
  game.properties: |
    enemies=aliens
    lives=3
    enemies.cheat=true
    enemies.cheat.level=noGoodRotten
    secret.code.passphrase=UUDDLRLRBABAS
    secret.code.allowed=true
    secret.code.lives=30

create a propagation policy to propagate deployment to member1:

apiVersion: policy.karmada.io/v1alpha1
kind: PropagationPolicy
metadata:
  name: nginx-propagation
spec:
  propagateDeps: true
  resourceSelectors:
    - apiVersion: apps/v1
      kind: Deployment
      name: nginx
  placement:
    clusterAffinity:
      clusterNames:
        - member1
    replicaScheduling:
      replicaSchedulingType: Duplicated

update the propagation policy to propagate deployment to member4(which is not exist):

apiVersion: policy.karmada.io/v1alpha1
kind: PropagationPolicy
metadata:
  name: nginx-propagation
spec:
  propagateDeps: true
  resourceSelectors:
    - apiVersion: apps/v1
      kind: Deployment
      name: nginx
  placement:
    clusterAffinity:
      clusterNames:
        - member4
    replicaScheduling:
      replicaSchedulingType: Duplicated

then we will found out the resource binding will reserve the last successful schedule result and the Scheduled condition is false:

apiVersion: work.karmada.io/v1alpha2
kind: ResourceBinding
metadata:
  annotations:
    lhb: lhb
  creationTimestamp: "2022-03-14T05:49:11Z"
  finalizers:
  - karmada.io/binding-controller
  generation: 29
  labels:
    propagationpolicy.karmada.io/name: nginx-propagation
    propagationpolicy.karmada.io/namespace: default
  name: nginx-deployment
  namespace: default
  ownerReferences:
  - apiVersion: apps/v1
    blockOwnerDeletion: true
    controller: true
    kind: Deployment
    name: nginx
    uid: 71d5023f-0d51-4e2a-9f31-9ddf553bc536
  resourceVersion: "768505"
  uid: 6a605353-5683-4205-b58d-d6ce8b6e75d8
spec:
  clusters:
  - name: member1
    replicas: 2
  replicas: 2
  resource:
    apiVersion: apps/v1
    kind: Deployment
    name: nginx
    namespace: default
    resourceVersion: "768434"
    uid: 71d5023f-0d51-4e2a-9f31-9ddf553bc536
status:
  aggregatedStatus:
  - applied: true
    clusterName: member1
    status:
      availableReplicas: 2
      conditions:
      - lastTransitionTime: "2022-03-17T07:31:36Z"
        lastUpdateTime: "2022-03-17T07:31:36Z"
        message: Deployment has minimum availability.
        reason: MinimumReplicasAvailable
        status: "True"
        type: Available
      - lastTransitionTime: "2022-03-17T07:31:30Z"
        lastUpdateTime: "2022-03-17T07:31:36Z"
        message: ReplicaSet "nginx-6799fc88d8" has successfully progressed.
        reason: NewReplicaSetAvailable
        status: "True"
        type: Progressing
      observedGeneration: 1
      readyReplicas: 2
      replicas: 2
      updatedReplicas: 2
  conditions:
  - lastTransitionTime: "2022-03-17T07:31:30Z"
    message: All works have been successfully applied
    reason: FullyAppliedSuccess
    status: "True"
    type: FullyApplied
  - lastTransitionTime: "2022-03-17T07:31:54Z"
    message: no clusters fit
    reason: BindingFailedScheduling
    status: "False"
    type: Scheduled

when we update deployment, the resource binding will be dropped in dependencies distributor as its Scheduled condtion is false.

mrlihanbo · 2022-03-17T07:48:51Z

To reproduce bug: update Resource info when update attached bindings.

create a deployment which references configMap:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: nginx
  labels:
    app: nginx
spec:
  replicas: 1
  selector:
    matchLabels:
      app: nginx
  template:
    metadata:
      labels:
        app: nginx
    spec:
      containers:
      - image: nginx
        name: nginx
        volumeMounts:
        - name: configmap
          mountPath: "/configmap"
      volumes:
      - name: configmap
        configMap:
          name: game-config

create configMap:

apiVersion: v1
kind: ConfigMap
metadata:
  name: game-config
data:
  game.properties: |
    enemies=aliens
    lives=3
    enemies.cheat=true
    enemies.cheat.level=noGoodRotten
    secret.code.passphrase=UUDDLRLRBABAS
    secret.code.allowed=true
    secret.code.lives=30

create a propagation policy to propagate deployment to member1:

apiVersion: policy.karmada.io/v1alpha1
kind: PropagationPolicy
metadata:
  name: nginx-propagation
spec:
  propagateDeps: true
  resourceSelectors:
    - apiVersion: apps/v1
      kind: Deployment
      name: nginx
  placement:
    clusterAffinity:
      clusterNames:
        - member1
    replicaScheduling:
      replicaSchedulingType: Duplicated

update the configmap to add data:

apiVersion: v1
kind: ConfigMap
metadata:
  name: game-config
data:
  game.properties: |
    enemies=aliens
    lives=3
    enemies.cheat=true
    enemies.cheat.level=noGoodRotten
    secret.code.passphrase=UUDDLRLRBABAS
    secret.code.allowed=true
    secret.code.lives=30    
  ui.properties: |
    color.good=purple
    color.bad=yellow
    allow.textmode=true
    how.nice.to.look=fairlyNice

you will found that the configmap in member1 is not updated as the reference binding is not updated.

XiShanYongYe-Chang · 2022-03-17T09:36:58Z

pkg/dependenciesdistributor/dependencies_distributor.go

-	// prevent newBindingObject from the queue if it's not in Scheduled condition
-	if !helper.IsBindingScheduled(&newBindingObject.Status) {
+	// prevent newBindingObject from the queue if it's not scheduled yet.
+	if len(oldBindingObject.Spec.Clusters) == 0 && len(newBindingObject.Spec.Clusters) == 0 {


~~When len(oldBindingObject.Spec.Clusters) == 0 and len(newBindingObject.Spec.Clusters) != 0, does it also need to be added to the queue?~~

When len(oldBindingObject.Spec.Clusters) == 0 and len(newBindingObject.Spec.Clusters) != 0, does it also need to be added to the queue?

When len(oldBindingObject.Spec.Clusters) == 0 and len(newBindingObject.Spec.Clusters) != 0, it means the resource will be scheduled to some clusters, and it need to be enqueued so that dependencies distributor will propagate its dependencies to the same clusters.
When len(oldBindingObject.Spec.Clusters) != 0 and len(newBindingObject.Spec.Clusters) == 0, it means the resource has been scheduled to some clusters and now be deleted, and it need to be enqueued so that dependencies distributor will remove its dependencies from the clusters.

XiShanYongYe-Chang

/lgtm

RainbowMango

/approve

karmada-bot · 2022-03-18T09:21:45Z

[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:

~~OWNERS~~ [RainbowMango]

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

RainbowMango · 2022-03-18T09:23:16Z

Hi @mrlihanbo , please check-pick this patch to release-1.1 branch.

Here is the script you can leverage: https://github.com/karmada-io/karmada/blob/master/hack/cherry_pick_pull.sh

…9-upstream-release-1.1 Automated cherry pick of #1499: bugfix: enqueue the resource binding that has been

mrlihanbo added 2 commits March 17, 2022 14:51

bugfix: enqueue the resource binding that has been scheduled to membe…

8843749

…r clusters Signed-off-by: lihanbo <lihanbo2@huawei.com>

bugfix: update Resource info when update attached binding

f66e391

Signed-off-by: lihanbo <lihanbo2@huawei.com>

karmada-bot added release-note-none Denotes a PR that doesn't merit a release note. kind/bug Categorizes issue or PR as related to a bug. labels Mar 17, 2022

karmada-bot requested review from Garrybest and XiShanYongYe-Chang March 17, 2022 07:07

karmada-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Mar 17, 2022

karmada-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Mar 17, 2022

XiShanYongYe-Chang reviewed Mar 17, 2022

View reviewed changes

XiShanYongYe-Chang reviewed Mar 18, 2022

View reviewed changes

karmada-bot assigned XiShanYongYe-Chang Mar 18, 2022

karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 18, 2022

RainbowMango approved these changes Mar 18, 2022

View reviewed changes

karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 18, 2022

karmada-bot merged commit d8bcedc into karmada-io:master Mar 18, 2022

mrlihanbo mentioned this pull request Mar 20, 2022

Automated cherry pick of #1499: bugfix: enqueue the resource binding that has been #1507

Merged

karmada-bot added a commit that referenced this pull request Mar 21, 2022

Merge pull request #1507 from mrlihanbo/automated-cherry-pick-of-#149…

0a87c83

…9-upstream-release-1.1 Automated cherry pick of #1499: bugfix: enqueue the resource binding that has been

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bugfixs of dependencies distributor #1499

bugfixs of dependencies distributor #1499

mrlihanbo commented Mar 17, 2022 •

edited by RainbowMango

RainbowMango commented Mar 17, 2022

mrlihanbo commented Mar 17, 2022 •

edited

mrlihanbo commented Mar 17, 2022

XiShanYongYe-Chang Mar 17, 2022 •

edited

mrlihanbo Mar 17, 2022

XiShanYongYe-Chang left a comment

RainbowMango left a comment

karmada-bot commented Mar 18, 2022

RainbowMango commented Mar 18, 2022

bugfixs of dependencies distributor #1499

bugfixs of dependencies distributor #1499

Conversation

mrlihanbo commented Mar 17, 2022 • edited by RainbowMango

RainbowMango commented Mar 17, 2022

mrlihanbo commented Mar 17, 2022 • edited

mrlihanbo commented Mar 17, 2022

XiShanYongYe-Chang Mar 17, 2022 • edited

Choose a reason for hiding this comment

mrlihanbo Mar 17, 2022

Choose a reason for hiding this comment

XiShanYongYe-Chang left a comment

Choose a reason for hiding this comment

RainbowMango left a comment

Choose a reason for hiding this comment

karmada-bot commented Mar 18, 2022

RainbowMango commented Mar 18, 2022

mrlihanbo commented Mar 17, 2022 •

edited by RainbowMango

mrlihanbo commented Mar 17, 2022 •

edited

XiShanYongYe-Chang Mar 17, 2022 •

edited