UnitedDeployment: update subset (#166)

* UnitedDeployment: update subset

* refactor codes

* record status in condition; correct some ut

* stablize ut

pkg/apis/apps/v1alpha1/uniteddeployment_types.go

@@ -32,6 +32,18 @@ const (
 	ManualUpdateStrategyType UpdateStrategyType = "Manual"
 )
 
+// UnitedDeploymentConditionType indicates valid conditions type of a UnitedDeployment.
+type UnitedDeploymentConditionType string
+
+const (
+	// SubsetProvisioned means all the expected subsets are provisioned and unexpected subsets are deleted.
+	SubsetProvisioned UnitedDeploymentConditionType = "SubsetProvisioned"
+	// SubsetUpdated means all the subsets are updated.
+	SubsetUpdated UnitedDeploymentConditionType = "SubsetUpdated"
+	// SubsetFailure is added in a UnitedDeployment when one of its subsets has failure during its own reconciling.
+	SubsetFailure UnitedDeploymentConditionType = "SubsetFailure"
+)
+
 // UnitedDeploymentSpec defines the desired state of UnitedDeployment
 type UnitedDeploymentSpec struct {
 	// Replicas is the totally desired number of replicas of all the owning workloads.
@@ -158,9 +170,6 @@ type UnitedDeploymentStatus struct {
 	UpdateStatus *UpdateStatus `json:"updateStatus,omitempty"`
 }
 
-// UnitedDeploymentConditionType indicates valid conditions type of a UnitedDeployment.
-type UnitedDeploymentConditionType string
-
 // UnitedDeploymentCondition describes current state of a UnitedDeployment.
 type UnitedDeploymentCondition struct {
 	// Type of in place set condition.

pkg/controller/uniteddeployment/revision.go

@@ -60,8 +60,8 @@ func (r *ReconcileUnitedDeployment) controlledHistories(ud *appsalphav1.UnitedDe
 	}
 
 	mts := make([]metav1.Object, len(histories.Items))
-	for i, pod := range histories.Items {
-		mts[i] = pod.DeepCopy()
+	for i, history := range histories.Items {
+		mts[i] = history.DeepCopy()
 	}
 	claims, err := cm.ClaimOwnedObjects(mts)
 	if err != nil {

pkg/controller/uniteddeployment/revision_test.go

@@ -67,7 +67,7 @@ func TestRevisionManage(t *testing.T) {
 							Spec: corev1.PodSpec{
 								Containers: []corev1.Container{
 									{
-										Name:  "containerA",
+										Name:  "container-a",
 										Image: "nginx:1.0",
 									},
 								},
@@ -79,7 +79,7 @@ func TestRevisionManage(t *testing.T) {
 			Topology: appsv1alpha1.Topology{
 				Subsets: []appsv1alpha1.Subset{
 					{
-						Name: "subsetA",
+						Name: "subset-a",
 						NodeSelector: corev1.NodeSelector{
 							NodeSelectorTerms: []corev1.NodeSelectorTerm{
 								{
@@ -96,7 +96,7 @@ func TestRevisionManage(t *testing.T) {
 					},
 				},
 			},
-			RevisionHistoryLimit: &one,
+			RevisionHistoryLimit: &two,
 		},
 	}
 

pkg/controller/uniteddeployment/statefulset_control.go

@@ -45,7 +45,7 @@ type StatefulSetControl struct {
 }
 
 // GetAllSubsets returns all of subsets owned by the UnitedDeployment.
-func (m *StatefulSetControl) GetAllSubsets(ud *alpha1.UnitedDeployment) (podSets []*Subset, err error) {
+func (m *StatefulSetControl) GetAllSubsets(ud *alpha1.UnitedDeployment) (subSets []*Subset, err error) {
 	selector, err := metav1.LabelSelectorAsSelector(ud.Spec.Selector)
 	if err != nil {
 		return nil, err
@@ -71,13 +71,13 @@ func (m *StatefulSetControl) GetAllSubsets(ud *alpha1.UnitedDeployment) (podSets
 	}
 
 	for _, claimedSet := range claimedSets {
-		podSet, err := m.convertToSubset(claimedSet.(*appsv1.StatefulSet))
+		subSet, err := m.convertToSubset(claimedSet.(*appsv1.StatefulSet))
 		if err != nil {
 			return nil, err
 		}
-		podSets = append(podSets, podSet)
+		subSets = append(subSets, subSet)
 	}
-	return podSets, nil
+	return subSets, nil
 }
 
 // CreateSubset creates the StatefulSet depending on the inputs.
@@ -134,19 +134,27 @@ func applyStatefulSetTemplate(ud *alpha1.UnitedDeployment, subsetName string, re
 
 	set.Spec.Selector = selectors
 	set.Spec.Replicas = &replicas
-	if set.Spec.UpdateStrategy.Type == appsv1.RollingUpdateStatefulSetStrategyType {
+	if ud.Spec.Template.StatefulSetTemplate.Spec.UpdateStrategy.Type == appsv1.OnDeleteStatefulSetStrategyType {
+		set.Spec.UpdateStrategy.Type = appsv1.OnDeleteStatefulSetStrategyType
+	} else {
 		if set.Spec.UpdateStrategy.RollingUpdate == nil {
 			set.Spec.UpdateStrategy.RollingUpdate = &appsv1.RollingUpdateStatefulSetStrategy{}
 		}
 		set.Spec.UpdateStrategy.RollingUpdate.Partition = &partition
 	}
 
-	set.Spec.Template = *ud.Spec.Template.StatefulSetTemplate.Spec.Template.DeepCopy()
+	set.Spec.Template = ud.Spec.Template.StatefulSetTemplate.Spec.Template
 	if set.Spec.Template.Labels == nil {
 		set.Spec.Template.Labels = map[string]string{}
 	}
 	set.Spec.Template.Labels[alpha1.SubSetNameLabelKey] = subsetName
 	set.Spec.Template.Labels[alpha1.ControllerRevisionHashLabelKey] = revision
+
+	set.Spec.RevisionHistoryLimit = ud.Spec.Template.StatefulSetTemplate.Spec.RevisionHistoryLimit
+	set.Spec.PodManagementPolicy = ud.Spec.Template.StatefulSetTemplate.Spec.PodManagementPolicy
+	set.Spec.ServiceName = ud.Spec.Template.StatefulSetTemplate.Spec.ServiceName
+	set.Spec.VolumeClaimTemplates = ud.Spec.Template.StatefulSetTemplate.Spec.VolumeClaimTemplates
+
 	attachNodeAffinity(&set.Spec.Template.Spec, subSetConfig)
 
 	return nil
@@ -185,11 +193,17 @@ func (m *StatefulSetControl) UpdateSubset(subset *Subset, ud *alpha1.UnitedDeplo
 }
 
 // DeleteSubset is called to delete the subset. The target StatefulSet can be found with the input subset.
-func (m *StatefulSetControl) DeleteSubset(podSet *Subset) error {
-	set := podSet.Spec.SubsetRef.Resources[0].(*appsv1.StatefulSet)
+func (m *StatefulSetControl) DeleteSubset(subSet *Subset) error {
+	set := subSet.Spec.SubsetRef.Resources[0].(*appsv1.StatefulSet)
 	return m.Delete(context.TODO(), set, client.PropagationPolicy(metav1.DeletePropagationBackground))
 }
 
+// GetSubsetFailure return the error message extracted form StatefulSet status conditions.
+func (m *StatefulSetControl) GetSubsetFailure(setSet *Subset) *string {
+	// StatefulSet has not condition
+	return nil
+}
+
 func (m *StatefulSetControl) convertToSubset(set *appsv1.StatefulSet) (*Subset, error) {
 	subSetName, err := getSubsetNameFrom(set)
 	if err != nil {

pkg/controller/uniteddeployment/subset.go

@@ -64,12 +64,14 @@ type ResourceRef struct {
 
 // ControlInterface defines the interface that UnitedDeployment uses to list, create, update, and delete Subsets.
 type ControlInterface interface {
-	// GetAllSubsets returns the subsets which are managed by the UnitedDeployment
+	// GetAllSubsets returns the subsets which are managed by the UnitedDeployment.
 	GetAllSubsets(ud *appsv1alpha1.UnitedDeployment) ([]*Subset, error)
-	// // CreateSubset creates the subset depending on the inputs.
+	// CreateSubset creates the subset depending on the inputs.
 	CreateSubset(ud *appsv1alpha1.UnitedDeployment, unit string, revision string, replicas, partition int32) error
 	// UpdateSubset updates the target subset with the input information.
 	UpdateSubset(subSet *Subset, ud *appsv1alpha1.UnitedDeployment, revision string, replicas, partition int32) error
 	// UpdateSubset is used to delete the input subset.
 	DeleteSubset(*Subset) error
+	// GetSubsetFailure extracts the subset failure message to expose on UnitedDeployment status.
+	GetSubsetFailure(*Subset) *string
 }

pkg/controller/uniteddeployment/uniteddeployment_controller.go

@@ -19,6 +19,7 @@ package uniteddeployment
 import (
 	"context"
 	"fmt"
+	"reflect"
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
@@ -129,8 +130,9 @@ func (r *ReconcileUnitedDeployment) Reconcile(request reconcile.Request) (reconc
 	if instance.DeletionTimestamp != nil {
 		return reconcile.Result{}, nil
 	}
+	oldStatus := instance.Status.DeepCopy()
 
-	currentRevision, updatedRevision, _, _, err := r.constructUnitedDeploymentRevisions(instance)
+	currentRevision, updatedRevision, _, collisionCount, err := r.constructUnitedDeploymentRevisions(instance)
 	if err != nil {
 		klog.Errorf("Fail to construct controller revision of UnitedDeployment %s/%s: %s", instance.Namespace, instance.Name, err)
 		r.recorder.Event(instance.DeepCopy(), corev1.EventTypeWarning, fmt.Sprintf("Failed%s", eventTypeRevisionProvision), err.Error())
@@ -156,13 +158,13 @@ func (r *ReconcileUnitedDeployment) Reconcile(request reconcile.Request) (reconc
 	nextPartitions := calcNextPartitions(instance, nextReplicas)
 	klog.V(4).Infof("Get UnitedDeployment %s/%s next partition %v", instance.Namespace, instance.Name, nextPartitions)
 
-	if _, err := r.manageSubsetProvision(instance, nameToSubset, nextReplicas, nextPartitions, currentRevision, updatedRevision, subsetType); err != nil {
+	newStatus, err := r.manageSubsets(instance, nameToSubset, nextReplicas, nextPartitions, currentRevision, updatedRevision, subsetType)
+	if err != nil {
 		klog.Errorf("Fail to update UnitedDeployment %s/%s: %s", instance.Namespace, instance.Name, err)
 		r.recorder.Event(instance.DeepCopy(), corev1.EventTypeWarning, fmt.Sprintf("Failed%s", eventTypeSubsetsUpdate), err.Error())
-		return reconcile.Result{}, nil
 	}
 
-	return reconcile.Result{}, nil
+	return r.updateStatus(instance, newStatus, oldStatus, nameToSubset, nextReplicas, nextPartitions, currentRevision, updatedRevision, collisionCount, control)
 }
 
 func (r *ReconcileUnitedDeployment) getNameToSubset(instance *appsv1alpha1.UnitedDeployment, control ControlInterface) (*map[string]*Subset, error) {
@@ -257,3 +259,124 @@ func (r *ReconcileUnitedDeployment) classifySubsetBySubsetName(ud *appsv1alpha1.
 	}
 	return mapping
 }
+
+func (r *ReconcileUnitedDeployment) updateStatus(instance *appsv1alpha1.UnitedDeployment, newStatus, oldStatus *appsv1alpha1.UnitedDeploymentStatus, nameToSubset *map[string]*Subset, nextReplicas, nextPartition *map[string]int32, currentRevision, updatedRevision *appsv1.ControllerRevision, collisionCount int32, control ControlInterface) (reconcile.Result, error) {
+	newStatus = r.calculateStatus(instance, newStatus, nameToSubset, nextReplicas, nextPartition, currentRevision, updatedRevision, collisionCount, control)
+	_, err := r.updateUnitedDeployment(instance, oldStatus, newStatus)
+	return reconcile.Result{}, err
+}
+
+func (r *ReconcileUnitedDeployment) calculateStatus(ud *appsv1alpha1.UnitedDeployment, newStatus *appsv1alpha1.UnitedDeploymentStatus, nameToSubset *map[string]*Subset, nextReplicas, nextPartition *map[string]int32, currentRevision, updatedRevision *appsv1.ControllerRevision, collisionCount int32, control ControlInterface) *appsv1alpha1.UnitedDeploymentStatus {
+	expectedRevision := currentRevision.Name
+	if updatedRevision != nil {
+		expectedRevision = updatedRevision.Name
+	}
+
+	newStatus.Replicas = 0
+	newStatus.ReadyReplicas = 0
+	newStatus.UpdatedReplicas = 0
+	newStatus.UpdatedReadyReplicas = 0
+
+	// sync from status
+	for _, subset := range *nameToSubset {
+		subsetReplicas, subsetReadyReplicas, subsetUpdatedReplicas, subsetUpdatedReadyReplicas := replicasStatusFn(subset, expectedRevision)
+		newStatus.Replicas += subsetReplicas
+		newStatus.ReadyReplicas += subsetReadyReplicas
+		newStatus.UpdatedReplicas += subsetUpdatedReplicas
+		newStatus.UpdatedReadyReplicas += subsetUpdatedReadyReplicas
+	}
+
+	newStatus.SubsetReplicas = *nextReplicas
+
+	if newStatus.CurrentRevision == "" {
+		// init with current revision
+		newStatus.CurrentRevision = currentRevision.Name
+	}
+
+	if newStatus.UpdateStatus == nil {
+		newStatus.UpdateStatus = &appsv1alpha1.UpdateStatus{}
+	}
+
+	newStatus.UpdateStatus.UpdatedRevision = expectedRevision
+	newStatus.UpdateStatus.CurrentPartitions = *nextPartition
+
+	if newStatus.UpdateStatus.UpdatedRevision != newStatus.CurrentRevision && newStatus.UpdatedReadyReplicas >= newStatus.Replicas {
+		newStatus.CurrentRevision = newStatus.UpdateStatus.UpdatedRevision
+	}
+
+	var subsetFailure *string
+	for _, subset := range *nameToSubset {
+		failureMessage := control.GetSubsetFailure(subset)
+		if failureMessage != nil {
+			subsetFailure = failureMessage
+			break
+		}
+	}
+
+	if subsetFailure == nil {
+		RemoveUnitedDeploymentCondition(newStatus, appsv1alpha1.SubsetFailure)
+	} else {
+		SetUnitedDeploymentCondition(newStatus, NewUnitedDeploymentCondition(appsv1alpha1.SubsetFailure, corev1.ConditionTrue, "Error", *subsetFailure))
+	}
+
+	return newStatus
+}
+
+var replicasStatusFn = replicasStatus
+
+func replicasStatus(subset *Subset, updatedRevision string) (replicas, readyReplicas, updatedReplicas, updatedReadyReplicas int32) {
+	replicas = subset.Status.Replicas
+	readyReplicas = subset.Status.ReadyReplicas
+
+	replicaStatus, exist := subset.Status.RevisionReplicas[updatedRevision]
+	if exist {
+		updatedReplicas = replicaStatus.Replicas
+		updatedReadyReplicas = replicaStatus.ReadyReplicas
+	}
+	return
+}
+
+func (r *ReconcileUnitedDeployment) updateUnitedDeployment(ud *appsv1alpha1.UnitedDeployment, oldStatus, newStatus *appsv1alpha1.UnitedDeploymentStatus) (*appsv1alpha1.UnitedDeployment, error) {
+	if oldStatus.Replicas == newStatus.Replicas &&
+		oldStatus.ReadyReplicas == newStatus.ReadyReplicas &&
+		oldStatus.UpdatedReplicas == newStatus.UpdatedReplicas &&
+		oldStatus.UpdatedReadyReplicas == newStatus.UpdatedReadyReplicas &&
+		oldStatus.CurrentRevision == newStatus.CurrentRevision &&
+		oldStatus.CollisionCount == newStatus.CollisionCount &&
+		ud.Generation == newStatus.ObservedGeneration &&
+		reflect.DeepEqual(oldStatus.SubsetReplicas, newStatus.SubsetReplicas) &&
+		reflect.DeepEqual(oldStatus.UpdateStatus, newStatus.UpdateStatus) &&
+		reflect.DeepEqual(oldStatus.Conditions, newStatus.Conditions) {
+		return ud, nil
+	}
+
+	newStatus.ObservedGeneration = ud.Generation
+
+	var getErr, updateErr error
+	for i, obj := 0, ud; ; i++ {
+		klog.V(4).Infof(fmt.Sprintf("The %d th time updating status for %v: %s/%s, ", i, obj.Kind, obj.Namespace, obj.Name) +
+			fmt.Sprintf("replicas %d->%d (need %d), ", obj.Status.Replicas, newStatus.Replicas, obj.Spec.Replicas) +
+			fmt.Sprintf("readyReplicas %d->%d (need %d), ", obj.Status.ReadyReplicas, newStatus.ReadyReplicas, obj.Spec.Replicas) +
+			fmt.Sprintf("updatedReplicas %d->%d, ", obj.Status.UpdatedReplicas, newStatus.UpdatedReplicas) +
+			fmt.Sprintf("updatedReadyReplicas %d->%d, ", obj.Status.UpdatedReadyReplicas, newStatus.UpdatedReadyReplicas) +
+			fmt.Sprintf("sequence No: %v->%v", obj.Status.ObservedGeneration, newStatus.ObservedGeneration))
+
+		obj.Status = *newStatus
+
+		updateErr = r.Client.Status().Update(context.TODO(), obj)
+		if updateErr == nil {
+			return obj, nil
+		}
+		if i >= updateRetries {
+			break
+		}
+		tmpObj := &appsv1alpha1.UnitedDeployment{}
+		if getErr = r.Client.Get(context.TODO(), client.ObjectKey{Namespace: obj.Namespace, Name: obj.Name}, tmpObj); getErr != nil {
+			return nil, getErr
+		}
+		obj = tmpObj
+	}
+
+	klog.Errorf("fail to update UnitedDeployment %s/%s status: %s", ud.Namespace, ud.Name, updateErr)
+	return nil, updateErr
+}

pkg/controller/uniteddeployment/uniteddeployment_controller_test.go

@@ -61,7 +61,7 @@ func TestReconcile(t *testing.T) {
 							Spec: corev1.PodSpec{
 								Containers: []corev1.Container{
 									{
-										Name:  "containerA",
+										Name:  "container-a",
 										Image: "nginx:1.0",
 									},
 								},
@@ -73,7 +73,7 @@ func TestReconcile(t *testing.T) {
 			Topology: appsv1alpha1.Topology{
 				Subsets: []appsv1alpha1.Subset{
 					{
-						Name: "subsetA",
+						Name: "subset-a",
 						NodeSelector: corev1.NodeSelector{
 							NodeSelectorTerms: []corev1.NodeSelectorTerm{
 								{

pkg/controller/uniteddeployment/uniteddeployment_controller_utils.go

@@ -1,5 +1,6 @@
 /*
 Copyright 2019 The Kruise Authors.
+Copyright 2016 The Kubernetes Authors.
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -112,3 +113,56 @@ func getRevision(objMeta metav1.Object) string {
 	}
 	return objMeta.GetLabels()[appsv1alpha1.ControllerRevisionHashLabelKey]
 }
+
+// NewUnitedDeploymentCondition creates a new UnitedDeployment condition.
+func NewUnitedDeploymentCondition(condType appsv1alpha1.UnitedDeploymentConditionType, status corev1.ConditionStatus, reason, message string) *appsv1alpha1.UnitedDeploymentCondition {
+	return &appsv1alpha1.UnitedDeploymentCondition{
+		Type:               condType,
+		Status:             status,
+		LastTransitionTime: metav1.Now(),
+		Reason:             reason,
+		Message:            message,
+	}
+}
+
+// GetUnitedDeploymentCondition returns the condition with the provided type.
+func GetUnitedDeploymentCondition(status appsv1alpha1.UnitedDeploymentStatus, condType appsv1alpha1.UnitedDeploymentConditionType) *appsv1alpha1.UnitedDeploymentCondition {
+	for i := range status.Conditions {
+		c := status.Conditions[i]
+		if c.Type == condType {
+			return &c
+		}
+	}
+	return nil
+}
+
+// SetUnitedDeploymentCondition updates the UnitedDeployment to include the provided condition. If the condition that
+// we are about to add already exists and has the same status, reason and message then we are not going to update.
+func SetUnitedDeploymentCondition(status *appsv1alpha1.UnitedDeploymentStatus, condition *appsv1alpha1.UnitedDeploymentCondition) {
+	currentCond := GetUnitedDeploymentCondition(*status, condition.Type)
+	if currentCond != nil && currentCond.Status == condition.Status && currentCond.Reason == condition.Reason && currentCond.Message == condition.Message {
+		return
+	}
+
+	if currentCond != nil && currentCond.Status == condition.Status {
+		condition.LastTransitionTime = currentCond.LastTransitionTime
+	}
+	newConditions := filterOutCondition(status.Conditions, condition.Type)
+	status.Conditions = append(newConditions, *condition)
+}
+
+// RemoveUnitedDeploymentCondition removes the UnitedDeployment condition with the provided type.
+func RemoveUnitedDeploymentCondition(status *appsv1alpha1.UnitedDeploymentStatus, condType appsv1alpha1.UnitedDeploymentConditionType) {
+	status.Conditions = filterOutCondition(status.Conditions, condType)
+}
+
+func filterOutCondition(conditions []appsv1alpha1.UnitedDeploymentCondition, condType appsv1alpha1.UnitedDeploymentConditionType) []appsv1alpha1.UnitedDeploymentCondition {
+	var newConditions []appsv1alpha1.UnitedDeploymentCondition
+	for _, c := range conditions {
+		if c.Type == condType {
+			continue
+		}
+		newConditions = append(newConditions, c)
+	}
+	return newConditions
+}

pkg/controller/uniteddeployment/uniteddeployment_update.go

@@ -30,7 +30,58 @@ import (
 	"github.com/openkruise/kruise/pkg/util"
 )
 
-func (r *ReconcileUnitedDeployment) manageSubsetProvision(ud *appsv1alpha1.UnitedDeployment, nameToSubset *map[string]*Subset, nextReplicas, nextPartitions *map[string]int32, currentRevision, updatedRevision *appsv1.ControllerRevision, subsetType subSetType) (sets.String, error) {
+func (r *ReconcileUnitedDeployment) manageSubsets(ud *appsv1alpha1.UnitedDeployment, nameToSubset *map[string]*Subset, nextReplicas, nextPartitions *map[string]int32, currentRevision, updatedRevision *appsv1.ControllerRevision, subsetType subSetType) (newStatus *appsv1alpha1.UnitedDeploymentStatus, updateErr error) {
+	newStatus = ud.Status.DeepCopy()
+	exists, provisioned, err := r.manageSubsetProvision(ud, nameToSubset, nextReplicas, nextPartitions, currentRevision, updatedRevision, subsetType)
+	if err != nil {
+		SetUnitedDeploymentCondition(newStatus, NewUnitedDeploymentCondition(appsv1alpha1.SubsetProvisioned, corev1.ConditionFalse, "Error", err.Error()))
+		return newStatus, fmt.Errorf("fail to manage Subset provision: %s", err)
+	}
+
+	if provisioned {
+		SetUnitedDeploymentCondition(newStatus, NewUnitedDeploymentCondition(appsv1alpha1.SubsetProvisioned, corev1.ConditionTrue, "", ""))
+	}
+
+	expectedRevision := currentRevision
+	if updatedRevision != nil {
+		expectedRevision = updatedRevision
+	}
+
+	var needUpdate []string
+	for _, name := range exists.List() {
+		subset := (*nameToSubset)[name]
+		if subset.Labels[appsv1alpha1.ControllerRevisionHashLabelKey] != expectedRevision.Name ||
+			subset.Spec.Replicas != (*nextReplicas)[name] ||
+			subset.Spec.UpdateStrategy.Partition != (*nextPartitions)[name] {
+			needUpdate = append(needUpdate, name)
+		}
+	}
+
+	if len(needUpdate) > 0 {
+		_, updateErr = util.SlowStartBatch(len(needUpdate), slowStartInitialBatchSize, func(index int) error {
+			cell := needUpdate[index]
+			subset := (*nameToSubset)[cell]
+			replicas := (*nextReplicas)[cell]
+			partition := (*nextPartitions)[cell]
+
+			klog.V(0).Infof("UnitedDeployment %s/%s needs to update Subset (%s) %s/%s with revision %s, replicas %d, partition %d", ud.Namespace, ud.Name, subsetType, subset.Namespace, subset.Name, expectedRevision.Name, replicas, partition)
+			updateSubsetErr := r.subSetControls[subsetType].UpdateSubset(subset, ud, expectedRevision.Name, replicas, partition)
+			if updateSubsetErr != nil {
+				r.recorder.Event(ud.DeepCopy(), corev1.EventTypeWarning, fmt.Sprintf("Failed%s", eventTypeSubsetsUpdate), fmt.Sprintf("Error updating PodSet (%s) %s when updating: %s", subsetType, subset.Name, updateSubsetErr))
+			}
+			return updateSubsetErr
+		})
+	}
+
+	if updateErr == nil {
+		SetUnitedDeploymentCondition(newStatus, NewUnitedDeploymentCondition(appsv1alpha1.SubsetUpdated, corev1.ConditionTrue, "", ""))
+	} else {
+		SetUnitedDeploymentCondition(newStatus, NewUnitedDeploymentCondition(appsv1alpha1.SubsetUpdated, corev1.ConditionFalse, "Error", updateErr.Error()))
+	}
+	return
+}
+
+func (r *ReconcileUnitedDeployment) manageSubsetProvision(ud *appsv1alpha1.UnitedDeployment, nameToSubset *map[string]*Subset, nextReplicas, nextPartitions *map[string]int32, currentRevision, updatedRevision *appsv1.ControllerRevision, subsetType subSetType) (sets.String, bool, error) {
 	expectedSubsets := sets.String{}
 	gotSubsets := sets.String{}
 
@@ -118,6 +169,7 @@ func (r *ReconcileUnitedDeployment) manageSubsetProvision(ud *appsv1alpha1.Unite
 	}
 
 	// clean the other kind of subsets
+	cleaned := false
 	for t, control := range r.subSetControls {
 		if t == subsetType {
 			continue
@@ -130,6 +182,7 @@ func (r *ReconcileUnitedDeployment) manageSubsetProvision(ud *appsv1alpha1.Unite
 		}
 
 		for _, subset := range subsets {
+			cleaned = true
 			if err := control.DeleteSubset(subset); err != nil {
 				errs = append(errs, fmt.Errorf("fail to delete Subset %s of other type %s for UnitedDeployment %s/%s: %s", subset.Name, t, ud.Namespace, ud.Name, err))
 				continue
@@ -137,5 +190,5 @@ func (r *ReconcileUnitedDeployment) manageSubsetProvision(ud *appsv1alpha1.Unite
 		}
 	}
 
-	return expectedSubsets.Intersection(gotSubsets), utilerrors.NewAggregate(errs)
+	return expectedSubsets.Intersection(gotSubsets), len(creates) > 0 || len(deletes) > 0 || cleaned, utilerrors.NewAggregate(errs)
 }