Skip to content

Commit 7b6cb0c

Browse files
authored
fix: progressive sync for applicationSet needs to evaluate more application status fields (#126)
Signed-off-by: Mike Ng <[email protected]>
1 parent f485c02 commit 7b6cb0c

8 files changed

+175
-43
lines changed

deploy/crds/apps.open-cluster-management.io_multiclusterapplicationsetreports.yaml

+6
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@ spec:
7272
type: string
7373
syncStatus:
7474
type: string
75+
operationStateStartedAt:
76+
type: string
77+
operationStatePhase:
78+
type: string
79+
syncRevision:
80+
type: string
7581
type: object
7682
type: array
7783
resources:

e2e/run_e2e.sh

+23-4
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ kubectl apply -f deploy/crds/
1212
kubectl apply -f hack/test/crds/0000_00_authentication.open-cluster-management.io_managedserviceaccounts.yaml
1313
kubectl apply -f deploy/controller/
1414

15-
sleep 120
15+
kubectl -n open-cluster-management rollout status deployment multicloud-integrations-gitops --timeout=120s
16+
kubectl -n open-cluster-management rollout status deployment multicloud-integrations --timeout=120s
1617

1718
echo "TEST Propgation controller startup (expecting error)"
1819
POD_NAME=$(kubectl -n open-cluster-management get deploy multicloud-integrations -o yaml | grep ReplicaSet | grep successful | cut -d'"' -f2)
@@ -28,7 +29,11 @@ fi
2829
echo "SETUP install Argo CD to Managed cluster"
2930
kubectl config use-context kind-cluster1
3031
kubectl create namespace argocd
31-
kubectl apply -n argocd --force -f hack/test/e2e/argo-cd-install.yaml
32+
kubectl apply -n argocd --force -f hack/test/e2e/argo-cd-install.yaml
33+
kubectl -n argocd scale deployment/argocd-applicationset-controller --replicas 0
34+
kubectl -n argocd scale deployment/argocd-server --replicas 0
35+
kubectl -n argocd scale deployment/argocd-dex-server --replicas 0
36+
kubectl -n argocd scale deployment/argocd-notifications-controller --replicas 0
3237

3338
echo "SETUP install Argo CD to Hub cluster"
3439
kubectl config use-context kind-hub
@@ -44,8 +49,7 @@ kubectl -n argocd scale statefulset/argocd-application-controller --replicas 0
4449
# enable progressive sync
4550
kubectl -n argocd patch configmap argocd-cmd-params-cm --type merge -p '{"data":{"applicationsetcontroller.enable.progressive.syncs":"true"}}'
4651
kubectl -n argocd rollout restart deployment argocd-applicationset-controller
47-
48-
sleep 60s
52+
kubectl -n argocd rollout status deployment argocd-applicationset-controller --timeout=60s
4953

5054
echo "TEST Propgation controller startup"
5155
if kubectl -n open-cluster-management logs $POD_NAME argocd-pull-integration-controller-manager | grep "Starting Controller" | grep "Application"; then
@@ -161,3 +165,18 @@ else
161165
echo "Propagation FAILED: guestbook-ui deploy not created"
162166
exit 1
163167
fi
168+
kubectl config use-context kind-hub
169+
if [[ -n $(kubectl -n argocd get app cluster1-guestbook-app -o jsonpath='{.status.operationState.phase}') ]]; then
170+
echo "Propagation: hub cluster application cluster1-guestbook-app phase is not empty"
171+
else
172+
echo "Propagation FAILED: hub cluster application cluster1-guestbook-app phase is empty"
173+
kubectl -n argocd get app cluster1-guestbook-app -o yaml
174+
exit 1
175+
fi
176+
if [[ -n $(kubectl -n argocd get app cluster1-guestbook-app -o jsonpath='{.status.sync.revision}') ]]; then
177+
echo "Propagation: hub cluster application cluster1-guestbook-app revision not empty"
178+
else
179+
echo "Propagation FAILED: hub cluster application cluster1-guestbook-app revision is empty"
180+
kubectl -n argocd get app cluster1-guestbook-app -o yaml
181+
exit 1
182+
fi

hack/crds/apps.open-cluster-management.io_multiclusterapplicationsetreports.yaml

+39-14
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ apiVersion: apiextensions.k8s.io/v1
33
kind: CustomResourceDefinition
44
metadata:
55
annotations:
6-
controller-gen.kubebuilder.io/version: v0.10.0
6+
controller-gen.kubebuilder.io/version: v0.6.0
77
creationTimestamp: null
88
name: multiclusterapplicationsetreports.apps.open-cluster-management.io
99
spec:
@@ -21,8 +21,12 @@ spec:
2121
- name: v1alpha1
2222
schema:
2323
openAPIV3Schema:
24-
description: MulticlusterApplicationSetReport is the Schema for the MulticlusterApplicationSetReport
25-
API.
24+
description: MulticlusterApplicationSetReport provides a report of the status
25+
of an application from all the managed clusters where the application is
26+
deployed on. It provides a summary of the number of clusters in the various
27+
states. If an error or warning occurred when installing the application
28+
on a managed cluster, the conditions, including the waring and error message,
29+
is captured in the report.
2630
properties:
2731
apiVersion:
2832
description: 'APIVersion defines the versioned schema of this representation
@@ -38,36 +42,47 @@ spec:
3842
type: object
3943
statuses:
4044
description: AppConditions defines all the error/warning conditions in
41-
all clusters per application
45+
all clusters where a particular application is deployed.
4246
properties:
4347
clusterConditions:
4448
items:
4549
description: ClusterCondition defines all the error/warning conditions
46-
in one cluster per application
50+
in one cluster for an application.
4751
properties:
52+
app:
53+
type: string
4854
cluster:
4955
type: string
5056
conditions:
5157
items:
5258
description: Condition defines a type of error/warning
5359
properties:
5460
message:
61+
description: Message is the warning/error message associated
62+
with this condition.
5563
type: string
5664
type:
65+
description: Type identifies if the condition is a warning
66+
or an error.
5767
type: string
5868
type: object
5969
type: array
6070
healthStatus:
6171
type: string
6272
syncStatus:
6373
type: string
64-
app:
74+
operationStateStartedAt:
75+
type: string
76+
operationStatePhase:
77+
type: string
78+
syncRevision:
6579
type: string
6680
type: object
6781
type: array
6882
resources:
6983
items:
70-
description: ResourceRef defines a kind of resource
84+
description: ResourceRef identifies the resource that is deployed
85+
by the application.
7186
properties:
7287
apiVersion:
7388
type: string
@@ -80,27 +95,31 @@ spec:
8095
type: object
8196
type: array
8297
summary:
83-
description: Summary provides a summary of results
98+
description: 'ReportSummary provides a summary by providing a count
99+
of the total number of clusters where the application is deployed.
100+
It also provides a count of how many clusters where an application
101+
are in the following states: synced, notSynced, healthy, notHealthy,
102+
and inProgress.'
84103
properties:
85104
clusters:
86105
description: Clusters provides the count of all managed clusters
87-
the application is deployed to
106+
the application is deployed.
88107
type: string
89108
healthy:
90-
description: Healthy provides the count of healthy applications
109+
description: Healthy provides the count of healthy applications.
91110
type: string
92111
inProgress:
93112
description: InProgress provides the count of applications that
94-
are in the process of being deployed
113+
are in the process of being deployed.
95114
type: string
96115
notHealthy:
97-
description: NotHealthy provides the count of non-healthy applications
116+
description: NotHealthy provides the count of non-healthy applications.
98117
type: string
99118
notSynced:
100-
description: NotSynced provides the count of the out of sync applications
119+
description: NotSynced provides the count of the out of sync applications.
101120
type: string
102121
synced:
103-
description: Synced provides the count of synced applications
122+
description: Synced provides the count of synced applications.
104123
type: string
105124
type: object
106125
type: object
@@ -109,3 +128,9 @@ spec:
109128
type: object
110129
served: true
111130
storage: true
131+
status:
132+
acceptedNames:
133+
kind: ""
134+
plural: ""
135+
conditions: []
136+
storedVersions: []

pkg/apis/appsetreport/v1alpha1/appsetreport_types.go

+8-5
Original file line numberDiff line numberDiff line change
@@ -59,11 +59,14 @@ type Condition struct {
5959

6060
// ClusterCondition defines all the error/warning conditions in one cluster for an application.
6161
type ClusterCondition struct {
62-
Cluster string `json:"cluster,omitempty"`
63-
SyncStatus string `json:"syncStatus,omitempty"`
64-
HealthStatus string `json:"healthStatus,omitempty"`
65-
App string `json:"app,omitempty"`
66-
Conditions []Condition `json:"conditions,omitempty"`
62+
Cluster string `json:"cluster,omitempty"`
63+
SyncStatus string `json:"syncStatus,omitempty"`
64+
HealthStatus string `json:"healthStatus,omitempty"`
65+
OperationStateStartedAt string `json:"operationStateStartedAt,omitempty"`
66+
OperationStatePhase string `json:"operationStatePhase,omitempty"`
67+
SyncRevision string `json:"syncRevision,omitempty"`
68+
App string `json:"app,omitempty"`
69+
Conditions []Condition `json:"conditions,omitempty"`
6770
}
6871

6972
// AppConditions defines all the error/warning conditions in all clusters where a particular application is deployed.

pkg/controller/multiclusterstatusaggregation/multiclusterStatusAggregation_controller.go

+27-9
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,12 @@ type Cluster struct {
6262

6363
// Value for the appSetClusterStatusMap
6464
type OverallStatus struct {
65-
HealthStatus string
66-
SyncStatus string
67-
App string
65+
HealthStatus string
66+
SyncStatus string
67+
OperationStateStartedAt string
68+
OperationStatePhase string
69+
SyncRevision string
70+
App string
6871
}
6972

7073
// AppSetClusterResourceSorter sorts appsetreport resources by name
@@ -187,6 +190,9 @@ func (r *ReconcilePullModelAggregation) generateAggregation() error {
187190

188191
for _, manifestWork := range appSetClusterList.Items {
189192
healthStatus, syncStatus := "Unknown", "Unknown"
193+
operationStatePhase := ""
194+
operationStateStartedAt := ""
195+
syncRevision := ""
190196

191197
appsetNs, appsetName := ParseNamespacedName(manifestWork.Annotations[propagation.AnnotationKeyAppSet])
192198

@@ -203,13 +209,22 @@ func (r *ReconcilePullModelAggregation) generateAggregation() error {
203209
healthStatus = *statuses.Value.String
204210
} else if statuses.Name == "syncStatus" {
205211
syncStatus = *statuses.Value.String
212+
} else if statuses.Name == "operationStatePhase" {
213+
operationStatePhase = *statuses.Value.String
214+
} else if statuses.Name == "operationStateStartedAt" {
215+
operationStateStartedAt = *statuses.Value.String
216+
} else if statuses.Name == "syncRevision" {
217+
syncRevision = *statuses.Value.String
206218
}
207219
}
208220
}
209221

210222
appSetClusterStatusMap[appSetKey][clusterKey] = OverallStatus{
211-
HealthStatus: healthStatus,
212-
SyncStatus: syncStatus,
223+
HealthStatus: healthStatus,
224+
SyncStatus: syncStatus,
225+
OperationStateStartedAt: operationStateStartedAt,
226+
OperationStatePhase: operationStatePhase,
227+
SyncRevision: syncRevision,
213228
App: appsetNs + "/" + manifestWork.Annotations[propagation.AnnotationKeyHubApplicationName] +
214229
"/" + manifestWork.Namespace + "/" + manifestWork.Name,
215230
}
@@ -346,10 +361,13 @@ func (r *ReconcilePullModelAggregation) generateSummary(appSetClusterStatusMap m
346361
klog.V(1).Info("Cluster: ", cluster)
347362
// generate the cluster condition list per this appset
348363
appSetClusterConditionsMap[cluster.clusterName] = appsetreportV1alpha1.ClusterCondition{
349-
Cluster: cluster.clusterName,
350-
SyncStatus: appSetClusterStatusMap[appset][cluster].SyncStatus,
351-
HealthStatus: appSetClusterStatusMap[appset][cluster].HealthStatus,
352-
App: appSetClusterStatusMap[appset][cluster].App,
364+
Cluster: cluster.clusterName,
365+
SyncStatus: appSetClusterStatusMap[appset][cluster].SyncStatus,
366+
HealthStatus: appSetClusterStatusMap[appset][cluster].HealthStatus,
367+
OperationStateStartedAt: appSetClusterStatusMap[appset][cluster].OperationStateStartedAt,
368+
OperationStatePhase: appSetClusterStatusMap[appset][cluster].OperationStatePhase,
369+
SyncRevision: appSetClusterStatusMap[appset][cluster].SyncRevision,
370+
App: appSetClusterStatusMap[appset][cluster].App,
353371
}
354372

355373
// Calculate the summary while we're here.

propagation-controller/application/application_status_controller.go

+54-5
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,14 @@ import (
2222
"encoding/json"
2323
"fmt"
2424
"strings"
25+
"time"
2526

2627
"k8s.io/apimachinery/pkg/api/errors"
2728
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
2829
"k8s.io/apimachinery/pkg/runtime"
2930
"k8s.io/apimachinery/pkg/runtime/schema"
3031
"k8s.io/apimachinery/pkg/types"
32+
"k8s.io/client-go/util/retry"
3133
ctrl "sigs.k8s.io/controller-runtime"
3234
"sigs.k8s.io/controller-runtime/pkg/client"
3335
"sigs.k8s.io/controller-runtime/pkg/log"
@@ -116,16 +118,62 @@ func (r *ApplicationStatusReconciler) Reconcile(ctx context.Context, req ctrl.Re
116118
return ctrl.Result{}, err
117119
}
118120

121+
if cc.HealthStatus != "" {
122+
if cc.HealthStatus == "Healthy" {
123+
// If the health status is Healthy then simulate Progressing first then set to Healthy for ApplicationSet controller
124+
if err := unstructured.SetNestedField(application.Object, "Progressing", "status", "health", "status"); err != nil {
125+
log.Error(err, "unable to set healthStatus in Application status")
126+
return ctrl.Result{}, err
127+
}
128+
129+
log.Info("updating Application health status to Progressing")
130+
131+
if err := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
132+
return r.Client.Update(ctx, application)
133+
}); err != nil {
134+
log.Error(err, "unable to update Application")
135+
return ctrl.Result{}, err
136+
}
137+
}
138+
139+
if err := unstructured.SetNestedField(newStatus, cc.HealthStatus, "health", "status"); err != nil {
140+
log.Error(err, "unable to set health")
141+
return ctrl.Result{}, err
142+
}
143+
}
144+
119145
if cc.SyncStatus != "" {
120146
if err := unstructured.SetNestedField(newStatus, cc.SyncStatus, "sync", "status"); err != nil {
121147
log.Error(err, "unable to set sync")
122148
return ctrl.Result{}, err
123149
}
124150
}
125151

126-
if cc.HealthStatus != "" {
127-
if err := unstructured.SetNestedField(newStatus, cc.HealthStatus, "health", "status"); err != nil {
128-
log.Error(err, "unable to set health")
152+
if cc.OperationStatePhase != "" {
153+
if err := unstructured.SetNestedField(newStatus, cc.OperationStatePhase, "operationState", "phase"); err != nil {
154+
log.Error(err, "unable to set OperationStatePhase")
155+
return ctrl.Result{}, err
156+
}
157+
158+
if cc.OperationStateStartedAt == "" {
159+
cc.OperationStateStartedAt = time.Now().UTC().Format(time.RFC3339)
160+
}
161+
162+
if err = unstructured.SetNestedField(newStatus, cc.OperationStateStartedAt, "operationState", "startedAt"); err != nil {
163+
log.Error(err, "unable to set OperationStateStartedAt")
164+
return ctrl.Result{}, err
165+
}
166+
167+
if err = unstructured.SetNestedField(newStatus, map[string]interface{}{}, "operationState", "operation"); err != nil {
168+
// Application required field, set it to empty object
169+
log.Error(err, "unable to set operation")
170+
return ctrl.Result{}, err
171+
}
172+
}
173+
174+
if cc.SyncRevision != "" {
175+
if err := unstructured.SetNestedField(newStatus, cc.SyncRevision, "sync", "revision"); err != nil {
176+
log.Error(err, "unable to set SyncRevision")
129177
return ctrl.Result{}, err
130178
}
131179
}
@@ -172,8 +220,9 @@ func (r *ApplicationStatusReconciler) Reconcile(ctx context.Context, req ctrl.Re
172220
return ctrl.Result{}, err
173221
}
174222

175-
err = r.Client.Update(ctx, application)
176-
if err != nil {
223+
if err := retry.RetryOnConflict(retry.DefaultBackoff, func() error {
224+
return r.Client.Update(ctx, application)
225+
}); err != nil {
177226
log.Error(err, "unable to update Application")
178227
return ctrl.Result{}, err
179228
}

0 commit comments

Comments
 (0)