Skip to content

Commit 5af7ecc

Browse files
chore(event-reporter): refactoring, variables grouping, code splitting (#351)
* event-reporter: created dedicated func resolveApplicationVersions * event-reporter: added new types for variables grouping * event-reporter: removed redundant code comment * event-reporter(refactoring): params grouping, added new type ArgoTrackingMetadata to pass data between methods * event-reporter(refactoring): params grouping, added new type ReportedEntityParentApp to pass data between methods * event-reporter(refactoring): params grouping, added new type ReportedResource to pass data between methods, created new methods to group logic inside getResourceEventPayload * event-reporter(refactoring): fixed nil pointer issues after refactoring * event-reporter(refactoring): fixed lint issue * event-reporter(refactoring): fixed lint issue * event-reporter: changes after pr review
1 parent 66dc000 commit 5af7ecc

File tree

6 files changed

+299
-166
lines changed

6 files changed

+299
-166
lines changed

event_reporter/controller/controller.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,10 @@ func (c *eventReporterController) Run(ctx context.Context) {
7676
}
7777
trackingMethod := argoutil.GetTrackingMethod(c.settingsMgr)
7878

79-
err = c.applicationEventReporter.StreamApplicationEvents(ctx, &a, eventProcessingStartedAt, ignoreResourceCache, appInstanceLabelKey, trackingMethod)
79+
err = c.applicationEventReporter.StreamApplicationEvents(ctx, &a, eventProcessingStartedAt, ignoreResourceCache, &reporter.ArgoTrackingMetadata{
80+
AppInstanceLabelKey: &appInstanceLabelKey,
81+
TrackingMethod: &trackingMethod,
82+
})
8083
if err != nil {
8184
return err
8285
}

event_reporter/reporter/application_event_reporter.go

Lines changed: 55 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,7 @@ type ApplicationEventReporter interface {
4848
a *appv1.Application,
4949
eventProcessingStartedAt string,
5050
ignoreResourceCache bool,
51-
appInstanceLabelKey string,
52-
trackingMethod appv1.TrackingMethod,
51+
argoTrackingMetadata *ArgoTrackingMetadata,
5352
) error
5453
ShouldSendApplicationEvent(ae *appv1.ApplicationWatchEvent) (shouldSend bool, syncStatusChanged bool)
5554
}
@@ -113,8 +112,7 @@ func (s *applicationEventReporter) StreamApplicationEvents(
113112
a *appv1.Application,
114113
eventProcessingStartedAt string,
115114
ignoreResourceCache bool,
116-
appInstanceLabelKey string,
117-
trackingMethod appv1.TrackingMethod,
115+
argoTrackingMetadata *ArgoTrackingMetadata,
118116
) error {
119117
metricTimer := metricsUtils.NewMetricTimer()
120118

@@ -141,18 +139,11 @@ func (s *applicationEventReporter) StreamApplicationEvents(
141139

142140
desiredManifests, manifestGenErr := s.getDesiredManifests(ctx, a, nil, logCtx)
143141

144-
syncRevision := utils.GetOperationStateRevision(a)
145-
var applicationVersions *apiclient.ApplicationVersions
146-
if syncRevision != nil {
147-
syncManifests, _ := s.getDesiredManifests(ctx, a, syncRevision, logCtx)
148-
applicationVersions = syncManifests.GetApplicationVersions()
149-
} else {
150-
applicationVersions = nil
151-
}
142+
applicationVersions := s.resolveApplicationVersions(ctx, a, logCtx)
152143

153144
logCtx.Info("getting parent application name")
154145

155-
parentAppIdentity := utils.GetParentAppIdentity(a, appInstanceLabelKey, trackingMethod)
146+
parentAppIdentity := utils.GetParentAppIdentity(a, *argoTrackingMetadata.AppInstanceLabelKey, *argoTrackingMetadata.TrackingMethod)
156147

157148
if utils.IsChildApp(parentAppIdentity) {
158149
logCtx.Info("processing as child application")
@@ -165,27 +156,29 @@ func (s *applicationEventReporter) StreamApplicationEvents(
165156
}
166157

167158
rs := utils.GetAppAsResource(a)
159+
utils.SetHealthStatusIfMissing(rs)
168160

169161
parentDesiredManifests, manifestGenErr := s.getDesiredManifests(ctx, parentApplicationEntity, nil, logCtx)
170162

171-
// helm app hasnt revision
172-
// TODO: add check if it helm application
173163
parentAppSyncRevisionsMetadata, err := s.getApplicationRevisionsMetadata(ctx, logCtx, parentApplicationEntity)
174164
if err != nil {
175165
logCtx.WithError(err).Warn("failed to get parent application's revision metadata, resuming")
176166
}
177167

178-
utils.SetHealthStatusIfMissing(rs)
179-
err = s.processResource(ctx, *rs, parentApplicationEntity, logCtx, eventProcessingStartedAt, parentDesiredManifests, appTree, manifestGenErr, a, parentAppSyncRevisionsMetadata, appInstanceLabelKey, trackingMethod, applicationVersions)
168+
err = s.processResource(ctx, *rs, logCtx, eventProcessingStartedAt, parentDesiredManifests, manifestGenErr, a, applicationVersions, &ReportedEntityParentApp{
169+
app: parentApplicationEntity,
170+
appTree: appTree,
171+
revisionsMetadata: parentAppSyncRevisionsMetadata,
172+
}, argoTrackingMetadata)
180173
if err != nil {
181174
s.metricsServer.IncErroredEventsCounter(metrics.MetricChildAppEventType, metrics.MetricEventUnknownErrorType, a.Name)
182175
return err
183176
}
184177
s.metricsServer.ObserveEventProcessingDurationHistogramDuration(a.Name, metrics.MetricChildAppEventType, metricTimer.Duration())
185178
} else {
186-
logCtx.Info("processing as root application")
187179
// will get here only for root applications (not managed as a resource by another application)
188-
appEvent, err := s.getApplicationEventPayload(ctx, a, appTree, eventProcessingStartedAt, appInstanceLabelKey, trackingMethod, applicationVersions)
180+
logCtx.Info("processing as root application")
181+
appEvent, err := s.getApplicationEventPayload(ctx, a, appTree, eventProcessingStartedAt, applicationVersions, argoTrackingMetadata)
189182
if err != nil {
190183
s.metricsServer.IncErroredEventsCounter(metrics.MetricParentAppEventType, metrics.MetricEventGetPayloadErrorType, a.Name)
191184
return fmt.Errorf("failed to get application event: %w", err)
@@ -216,7 +209,11 @@ func (s *applicationEventReporter) StreamApplicationEvents(
216209
s.metricsServer.IncCachedIgnoredEventsCounter(metrics.MetricResourceEventType, a.Name)
217210
continue
218211
}
219-
err := s.processResource(ctx, rs, a, logCtx, eventProcessingStartedAt, desiredManifests, appTree, manifestGenErr, nil, revisionsMetadata, appInstanceLabelKey, trackingMethod, nil)
212+
err := s.processResource(ctx, rs, logCtx, eventProcessingStartedAt, desiredManifests, manifestGenErr, nil, nil, &ReportedEntityParentApp{
213+
app: a,
214+
appTree: appTree,
215+
revisionsMetadata: revisionsMetadata,
216+
}, argoTrackingMetadata)
220217
if err != nil {
221218
s.metricsServer.IncErroredEventsCounter(metrics.MetricResourceEventType, metrics.MetricEventUnknownErrorType, a.Name)
222219
return err
@@ -225,6 +222,16 @@ func (s *applicationEventReporter) StreamApplicationEvents(
225222
return nil
226223
}
227224

225+
func (s *applicationEventReporter) resolveApplicationVersions(ctx context.Context, a *appv1.Application, logCtx *log.Entry) *apiclient.ApplicationVersions {
226+
syncRevision := utils.GetOperationStateRevision(a)
227+
if syncRevision == nil {
228+
return nil
229+
}
230+
231+
syncManifests, _ := s.getDesiredManifests(ctx, a, syncRevision, logCtx)
232+
return syncManifests.GetApplicationVersions()
233+
}
234+
228235
func (s *applicationEventReporter) getAppForResourceReporting(
229236
rs appv1.ResourceStatus,
230237
ctx context.Context,
@@ -252,17 +259,14 @@ func (s *applicationEventReporter) getAppForResourceReporting(
252259
func (s *applicationEventReporter) processResource(
253260
ctx context.Context,
254261
rs appv1.ResourceStatus,
255-
parentApplication *appv1.Application,
256262
logCtx *log.Entry,
257263
appEventProcessingStartedAt string,
258264
desiredManifests *apiclient.ManifestResponse,
259-
appTree *appv1.ApplicationTree,
260265
manifestGenErr bool,
261-
originalApplication *appv1.Application,
262-
revisionsMetadata *utils.AppSyncRevisionsMetadata,
263-
appInstanceLabelKey string,
264-
trackingMethod appv1.TrackingMethod,
265-
applicationVersions *apiclient.ApplicationVersions,
266+
originalApplication *appv1.Application, // passed onlu if resource is app
267+
applicationVersions *apiclient.ApplicationVersions, // passed onlu if resource is app
268+
reportedEntityParentApp *ReportedEntityParentApp,
269+
argoTrackingMetadata *ArgoTrackingMetadata,
266270
) error {
267271
metricsEventType := metrics.MetricResourceEventType
268272
if utils.IsApp(rs) {
@@ -277,25 +281,44 @@ func (s *applicationEventReporter) processResource(
277281
// get resource desired state
278282
desiredState := getResourceDesiredState(&rs, desiredManifests, logCtx)
279283

280-
actualState, err := s.getResourceActualState(ctx, logCtx, metricsEventType, rs, parentApplication, originalApplication)
284+
actualState, err := s.getResourceActualState(ctx, logCtx, metricsEventType, rs, reportedEntityParentApp.app, originalApplication)
281285
if err != nil {
282286
return err
283287
}
284288
if actualState == nil {
285289
return nil
286290
}
287291

288-
parentApplicationToReport, revisionMetadataToReport := s.getAppForResourceReporting(rs, ctx, logCtx, parentApplication, revisionsMetadata)
292+
parentApplicationToReport, revisionMetadataToReport := s.getAppForResourceReporting(rs, ctx, logCtx, reportedEntityParentApp.app, reportedEntityParentApp.revisionsMetadata)
289293

290294
var originalAppRevisionMetadata *utils.AppSyncRevisionsMetadata = nil
291295

292296
if originalApplication != nil {
293297
originalAppRevisionMetadata, _ = s.getApplicationRevisionsMetadata(ctx, logCtx, originalApplication)
294298
}
295299

296-
ev, err := getResourceEventPayload(parentApplicationToReport, &rs, actualState, desiredState, appTree, manifestGenErr, appEventProcessingStartedAt, originalApplication, revisionMetadataToReport, originalAppRevisionMetadata, appInstanceLabelKey, trackingMethod, applicationVersions)
300+
ev, err := getResourceEventPayload(
301+
appEventProcessingStartedAt,
302+
&ReportedResource{
303+
rs: &rs,
304+
actualState: actualState,
305+
desiredState: desiredState,
306+
manifestGenErr: manifestGenErr,
307+
rsAsAppInfo: &ReportedResourceAsApp{
308+
app: originalApplication,
309+
revisionsMetadata: originalAppRevisionMetadata,
310+
applicationVersions: applicationVersions,
311+
},
312+
},
313+
&ReportedEntityParentApp{
314+
app: parentApplicationToReport,
315+
appTree: reportedEntityParentApp.appTree,
316+
revisionsMetadata: revisionMetadataToReport,
317+
},
318+
argoTrackingMetadata,
319+
)
297320
if err != nil {
298-
s.metricsServer.IncErroredEventsCounter(metricsEventType, metrics.MetricEventGetPayloadErrorType, parentApplication.Name)
321+
s.metricsServer.IncErroredEventsCounter(metricsEventType, metrics.MetricEventGetPayloadErrorType, reportedEntityParentApp.app.Name)
299322
logCtx.WithError(err).Warn("failed to get event payload, resuming")
300323
return nil
301324
}
@@ -307,7 +330,7 @@ func (s *applicationEventReporter) processResource(
307330
appName = appRes.Name
308331
} else {
309332
utils.LogWithResourceStatus(logCtx, rs).Info("streaming resource event")
310-
appName = parentApplication.Name
333+
appName = reportedEntityParentApp.app.Name
311334
}
312335

313336
if err := s.codefreshClient.SendEvent(ctx, appName, ev); err != nil {

event_reporter/reporter/application_event_reporter_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,10 @@ import (
3838
"github.com/stretchr/testify/assert"
3939
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
4040

41-
"github.com/argoproj/argo-cd/v2/common"
4241
"github.com/argoproj/argo-cd/v2/pkg/apiclient/application"
4342
"github.com/argoproj/argo-cd/v2/pkg/apiclient/events"
4443
"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
4544
"github.com/argoproj/argo-cd/v2/pkg/codefresh"
46-
"github.com/argoproj/argo-cd/v2/util/argo"
4745
)
4846

4947
const (
@@ -226,7 +224,7 @@ func TestStreamApplicationEvent(t *testing.T) {
226224
assert.Equal(t, *app, actualApp)
227225
return nil
228226
}
229-
_ = eventReporter.StreamApplicationEvents(context.Background(), app, "", false, common.LabelKeyAppInstance, argo.TrackingMethodLabel)
227+
_ = eventReporter.StreamApplicationEvents(context.Background(), app, "", false, getMockedArgoTrackingMetadata())
230228
})
231229
}
232230

0 commit comments

Comments
 (0)