Skip to content

Commit 66e1e38

Browse files
oleksandr-codefreshpasha-codefresh
oleksandr-codefresh
and
pasha-codefresh
authored
feat: revisions info reporting with multi-sourced apps support (#333)
* event-reporter: added utils methods to retrieve revisions metadata for application * event-reporter: report all revisions metadata instead single to support multisourced apps * event-reporter: added revision sha to reported value in anotations - "app.meta.revisions-metadata" * event-reporter: added change revisions sha to reported value in anotations - app.meta.revisions-metadata * event-reporter: updated changelog * event-reporter: changes to anotations repoting - app.meta.revisions-metadata, report only revision in case of helm chart * event-reporter: changes after pr review * event-reporter: fixed unit tests * event-reporter: fix lint issues * event-reporter: changes after pr reviev, fixing typo, added dedicated func a.Spec.IsHelmSource(idx), removed legacy code * event-reporter: refactoring of getApplicationLegacyRevisionDetails method * event-reporter / app_revision_test.go: added some tests to AddCommitsDetailsToAnnotations, AddCommitsDetailsToAppAnnotations, getRevisions, getOperationSyncRevisions * event-reporter / app_revision_test.go: added tests for GetRevisionsDetails method * event-reporter: updated app client to support sourceIndex param in non-grpc mode * event-reporter / app_revision.go: added sourceIndex param to applicationServiceClient.RevisionMetadata in order to properly support multisourced apps * event-reporter: lint fix * event-reporter: fix lint issues * event-reporter: fix lint issues * event-reporter: added back regacy logic with setting of commit details to labels as new runtimes should work on old on-prem versions * event-reporter: added condition to not send empty array for ChangeRevisions metadata --------- Co-authored-by: pashakostohrys <pavel@codefresh.io>
1 parent 288e7d3 commit 66e1e38

File tree

11 files changed

+763
-69
lines changed

11 files changed

+763
-69
lines changed

‎changelog/CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
### Features
2-
- fix: change revision controller should verify that revision already exists
2+
- feat: event-reporter: report change revisions metadata in app annotations

‎event_reporter/application/client.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,9 @@ func (c *httpApplicationClient) RevisionMetadata(ctx context.Context, in *appcli
107107
params := fmt.Sprintf("?appNamespace=%s&project=%s",
108108
*in.AppNamespace,
109109
*in.Project)
110+
if in.SourceIndex != nil {
111+
params += fmt.Sprintf("&sourceIndex=%d", *in.SourceIndex)
112+
}
110113
url := fmt.Sprintf("%s/api/v1/applications/%s/revisions/%s/metadata%s", c.baseUrl, *in.Name, *in.Revision, params)
111114
revisionMetadata := &v1alpha1.RevisionMetadata{}
112115
err := c.execute(ctx, url, revisionMetadata)

‎event_reporter/reporter/app_revision.go‎

Lines changed: 81 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,89 @@ package reporter
33
import (
44
"context"
55

6+
"github.com/argoproj/argo-cd/v2/event_reporter/utils"
67
"github.com/argoproj/argo-cd/v2/pkg/apiclient/application"
7-
appv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
8+
"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
9+
10+
log "github.com/sirupsen/logrus"
811
)
912

10-
func (s *applicationEventReporter) getApplicationRevisionDetails(ctx context.Context, a *appv1.Application, revision string) (*appv1.RevisionMetadata, error) {
13+
// treats multi-sourced apps as single source and gets first revision details
14+
func getApplicationLegacyRevisionDetails(a *v1alpha1.Application, revisionsWithMetadata *utils.AppSyncRevisionsMetadata) *v1alpha1.RevisionMetadata {
15+
if revisionsWithMetadata.SyncRevisions == nil || len(revisionsWithMetadata.SyncRevisions) == 0 {
16+
return nil
17+
}
18+
19+
sourceIdx := 0
20+
21+
if a.Spec.HasMultipleSources() {
22+
_, sourceIdx = a.Spec.GetNonRefSource()
23+
}
24+
25+
if revisionWithMetadata := revisionsWithMetadata.SyncRevisions[sourceIdx]; revisionWithMetadata != nil {
26+
return revisionWithMetadata.Metadata
27+
}
28+
29+
return nil
30+
}
31+
32+
func (s *applicationEventReporter) getRevisionsDetails(ctx context.Context, a *v1alpha1.Application, revisions []string) ([]*utils.RevisionWithMetadata, error) {
1133
project := a.Spec.GetProject()
12-
return s.applicationServiceClient.RevisionMetadata(ctx, &application.RevisionMetadataQuery{
13-
Name: &a.Name,
14-
AppNamespace: &a.Namespace,
15-
Revision: &revision,
16-
Project: &project,
17-
})
34+
rms := make([]*utils.RevisionWithMetadata, 0)
35+
36+
for idx, revision := range revisions {
37+
// report just revision for helm sources
38+
if a.Spec.SourceUnderIdxIsHelm(idx) {
39+
rms = append(rms, &utils.RevisionWithMetadata{
40+
Revision: revision,
41+
})
42+
continue
43+
}
44+
45+
sourceIndex := int32(idx)
46+
47+
rm, err := s.applicationServiceClient.RevisionMetadata(ctx, &application.RevisionMetadataQuery{
48+
Name: &a.Name,
49+
AppNamespace: &a.Namespace,
50+
Revision: &revision,
51+
Project: &project,
52+
SourceIndex: &sourceIndex,
53+
})
54+
if err != nil {
55+
return nil, err
56+
}
57+
rms = append(rms, &utils.RevisionWithMetadata{
58+
Revision: revision,
59+
Metadata: rm,
60+
})
61+
}
62+
63+
return rms, nil
64+
}
65+
66+
func (s *applicationEventReporter) getApplicationRevisionsMetadata(ctx context.Context, logCtx *log.Entry, a *v1alpha1.Application) (*utils.AppSyncRevisionsMetadata, error) {
67+
result := &utils.AppSyncRevisionsMetadata{}
68+
69+
if a.Status.Sync.Revision != "" || a.Status.Sync.Revisions != nil || (a.Status.History != nil && len(a.Status.History) > 0) {
70+
// can be the latest revision of repository
71+
operationSyncRevisionsMetadata, err := s.getRevisionsDetails(ctx, a, utils.GetOperationSyncRevisions(a))
72+
if err != nil {
73+
logCtx.WithError(err).Warnf("failed to get application(%s) sync revisions metadata, resuming", a.GetName())
74+
}
75+
76+
if err == nil && operationSyncRevisionsMetadata != nil {
77+
result.SyncRevisions = operationSyncRevisionsMetadata
78+
}
79+
// latest revision of repository where changes to app resource were actually made; empty if no changeRevision(-s) present
80+
operationChangeRevisionsMetadata, err := s.getRevisionsDetails(ctx, a, utils.GetOperationChangeRevisions(a))
81+
if err != nil {
82+
logCtx.WithError(err).Warnf("failed to get application(%s) change revisions metadata, resuming", a.GetName())
83+
}
84+
85+
if err == nil && operationChangeRevisionsMetadata != nil && len(operationChangeRevisionsMetadata) > 0 {
86+
result.ChangeRevisions = operationChangeRevisionsMetadata
87+
}
88+
}
89+
90+
return result, nil
1891
}

‎event_reporter/reporter/app_revision_test.go‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
package reporter
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
"github.com/argoproj/argo-cd/v2/event_reporter/application/mocks"
8+
"github.com/argoproj/argo-cd/v2/event_reporter/metrics"
9+
"github.com/argoproj/argo-cd/v2/event_reporter/utils"
10+
"github.com/argoproj/argo-cd/v2/pkg/apiclient/application"
11+
"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
12+
"github.com/argoproj/argo-cd/v2/server/cache"
13+
14+
"github.com/stretchr/testify/assert"
15+
"github.com/stretchr/testify/mock"
16+
)
17+
18+
func TestGetRevisionsDetails(t *testing.T) {
19+
t.Run("should return revisions for single source app", func(t *testing.T) {
20+
expectedRevision := "expected-revision"
21+
expectedResult := []*utils.RevisionWithMetadata{{
22+
Revision: expectedRevision,
23+
Metadata: &v1alpha1.RevisionMetadata{
24+
Author: "Test Author",
25+
Message: "first commit",
26+
},
27+
}}
28+
29+
app := v1alpha1.Application{
30+
Spec: v1alpha1.ApplicationSpec{
31+
Source: &v1alpha1.ApplicationSource{
32+
RepoURL: "https://my-site.com",
33+
TargetRevision: "HEAD",
34+
Path: ".",
35+
},
36+
},
37+
}
38+
39+
appServiceClient := mocks.NewApplicationClient(t)
40+
project := app.Spec.GetProject()
41+
sourceIdx1 := int32(0)
42+
43+
appServiceClient.On("RevisionMetadata", mock.Anything, &application.RevisionMetadataQuery{
44+
Name: &app.Name,
45+
AppNamespace: &app.Namespace,
46+
Revision: &expectedResult[0].Revision,
47+
Project: &project,
48+
SourceIndex: &sourceIdx1,
49+
}).Return(expectedResult[0].Metadata, nil)
50+
51+
reporter := &applicationEventReporter{
52+
&cache.Cache{},
53+
&MockCodefreshClient{},
54+
newAppLister(),
55+
appServiceClient,
56+
&metrics.MetricsServer{},
57+
}
58+
59+
result, _ := reporter.getRevisionsDetails(context.Background(), &app, []string{expectedRevision})
60+
61+
assert.Equal(t, expectedResult, result)
62+
})
63+
64+
t.Run("should return revisions for multi sourced apps", func(t *testing.T) {
65+
expectedRevision1 := "expected-revision-1"
66+
expectedRevision2 := "expected-revision-2"
67+
expectedResult := []*utils.RevisionWithMetadata{{
68+
Revision: expectedRevision1,
69+
Metadata: &v1alpha1.RevisionMetadata{
70+
Author: "Repo1 Author",
71+
Message: "first commit repo 1",
72+
},
73+
}, {
74+
Revision: expectedRevision2,
75+
Metadata: &v1alpha1.RevisionMetadata{
76+
Author: "Repo2 Author",
77+
Message: "first commit repo 2",
78+
},
79+
}}
80+
81+
app := v1alpha1.Application{
82+
Spec: v1alpha1.ApplicationSpec{
83+
Sources: []v1alpha1.ApplicationSource{{
84+
RepoURL: "https://my-site.com/repo-1",
85+
TargetRevision: "branch1",
86+
Path: ".",
87+
}, {
88+
RepoURL: "https://my-site.com/repo-2",
89+
TargetRevision: "branch2",
90+
Path: ".",
91+
}},
92+
},
93+
}
94+
95+
project := app.Spec.GetProject()
96+
97+
appServiceClient := mocks.NewApplicationClient(t)
98+
sourceIdx1 := int32(0)
99+
sourceIdx2 := int32(1)
100+
appServiceClient.On("RevisionMetadata", mock.Anything, &application.RevisionMetadataQuery{
101+
Name: &app.Name,
102+
AppNamespace: &app.Namespace,
103+
Revision: &expectedRevision1,
104+
Project: &project,
105+
SourceIndex: &sourceIdx1,
106+
}).Return(expectedResult[0].Metadata, nil)
107+
appServiceClient.On("RevisionMetadata", mock.Anything, &application.RevisionMetadataQuery{
108+
Name: &app.Name,
109+
AppNamespace: &app.Namespace,
110+
Revision: &expectedRevision2,
111+
Project: &project,
112+
SourceIndex: &sourceIdx2,
113+
}).Return(expectedResult[1].Metadata, nil)
114+
115+
reporter := &applicationEventReporter{
116+
&cache.Cache{},
117+
&MockCodefreshClient{},
118+
newAppLister(),
119+
appServiceClient,
120+
&metrics.MetricsServer{},
121+
}
122+
123+
result, _ := reporter.getRevisionsDetails(context.Background(), &app, []string{expectedRevision1, expectedRevision2})
124+
125+
assert.Equal(t, expectedResult, result)
126+
})
127+
128+
t.Run("should return only revision because of helm single source app", func(t *testing.T) {
129+
expectedRevision := "expected-revision"
130+
expectedResult := []*utils.RevisionWithMetadata{{
131+
Revision: expectedRevision,
132+
}}
133+
134+
app := v1alpha1.Application{
135+
Spec: v1alpha1.ApplicationSpec{
136+
Source: &v1alpha1.ApplicationSource{
137+
RepoURL: "https://my-site.com",
138+
TargetRevision: "HEAD",
139+
Path: ".",
140+
},
141+
},
142+
}
143+
144+
appServiceClient := mocks.NewApplicationClient(t)
145+
project := app.Spec.GetProject()
146+
sourceIdx1 := int32(0)
147+
148+
appServiceClient.On("RevisionMetadata", mock.Anything, &application.RevisionMetadataQuery{
149+
Name: &app.Name,
150+
AppNamespace: &app.Namespace,
151+
Revision: &expectedResult[0].Revision,
152+
Project: &project,
153+
SourceIndex: &sourceIdx1,
154+
}).Return(expectedResult[0].Metadata, nil)
155+
156+
reporter := &applicationEventReporter{
157+
&cache.Cache{},
158+
&MockCodefreshClient{},
159+
newAppLister(),
160+
appServiceClient,
161+
&metrics.MetricsServer{},
162+
}
163+
164+
result, _ := reporter.getRevisionsDetails(context.Background(), &app, []string{expectedRevision})
165+
166+
assert.Equal(t, expectedResult, result)
167+
})
168+
}

‎event_reporter/reporter/application_event_reporter.go‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -166,14 +166,13 @@ func (s *applicationEventReporter) StreamApplicationEvents(
166166

167167
// helm app hasnt revision
168168
// TODO: add check if it helm application
169-
parentOperationRevision := utils.GetOperationRevision(parentApplicationEntity)
170-
parentRevisionMetadata, err := s.getApplicationRevisionDetails(ctx, parentApplicationEntity, parentOperationRevision)
169+
parentAppSyncRevisionsMetadata, err := s.getApplicationRevisionsMetadata(ctx, logCtx, parentApplicationEntity)
171170
if err != nil {
172171
logCtx.WithError(err).Warn("failed to get parent application's revision metadata, resuming")
173172
}
174173

175174
utils.SetHealthStatusIfMissing(rs)
176-
err = s.processResource(ctx, *rs, parentApplicationEntity, logCtx, ts, parentDesiredManifests, appTree, manifestGenErr, a, parentRevisionMetadata, appInstanceLabelKey, trackingMethod, desiredManifests.ApplicationVersions)
175+
err = s.processResource(ctx, *rs, parentApplicationEntity, logCtx, ts, parentDesiredManifests, appTree, manifestGenErr, a, parentAppSyncRevisionsMetadata, appInstanceLabelKey, trackingMethod, desiredManifests.ApplicationVersions)
177176
if err != nil {
178177
s.metricsServer.IncErroredEventsCounter(metrics.MetricChildAppEventType, metrics.MetricEventUnknownErrorType, a.Name)
179178
return err
@@ -203,7 +202,7 @@ func (s *applicationEventReporter) StreamApplicationEvents(
203202
s.metricsServer.ObserveEventProcessingDurationHistogramDuration(a.Name, metrics.MetricParentAppEventType, reconcileDuration)
204203
}
205204

206-
revisionMetadata, _ := s.getApplicationRevisionDetails(ctx, a, utils.GetOperationRevision(a))
205+
revisionsMetadata, _ := s.getApplicationRevisionsMetadata(ctx, logCtx, a)
207206
// for each resource in the application get desired and actual state,
208207
// then stream the event
209208
for _, rs := range a.Status.Resources {
@@ -215,7 +214,7 @@ func (s *applicationEventReporter) StreamApplicationEvents(
215214
s.metricsServer.IncCachedIgnoredEventsCounter(metrics.MetricResourceEventType, a.Name)
216215
continue
217216
}
218-
err := s.processResource(ctx, rs, a, logCtx, ts, desiredManifests, appTree, manifestGenErr, nil, revisionMetadata, appInstanceLabelKey, trackingMethod, nil)
217+
err := s.processResource(ctx, rs, a, logCtx, ts, desiredManifests, appTree, manifestGenErr, nil, revisionsMetadata, appInstanceLabelKey, trackingMethod, nil)
219218
if err != nil {
220219
s.metricsServer.IncErroredEventsCounter(metrics.MetricResourceEventType, metrics.MetricEventUnknownErrorType, a.Name)
221220
return err
@@ -227,21 +226,22 @@ func (s *applicationEventReporter) StreamApplicationEvents(
227226
func (s *applicationEventReporter) getAppForResourceReporting(
228227
rs appv1.ResourceStatus,
229228
ctx context.Context,
229+
logCtx *log.Entry,
230230
a *appv1.Application,
231-
revisionMetadata *appv1.RevisionMetadata,
232-
) (*appv1.Application, *appv1.RevisionMetadata) {
231+
syncRevisionsMetadata *utils.AppSyncRevisionsMetadata,
232+
) (*appv1.Application, *utils.AppSyncRevisionsMetadata) {
233233
if rs.Kind != "Rollout" { // for rollout it's crucial to report always correct operationSyncRevision
234-
return a, revisionMetadata
234+
return a, syncRevisionsMetadata
235235
}
236236

237237
latestAppStatus, err := s.appLister.Applications(a.Namespace).Get(a.Name)
238238
if err != nil {
239-
return a, revisionMetadata
239+
return a, syncRevisionsMetadata
240240
}
241241

242-
revisionMetadataToReport, err := s.getApplicationRevisionDetails(ctx, latestAppStatus, utils.GetOperationRevision(latestAppStatus))
242+
revisionMetadataToReport, err := s.getApplicationRevisionsMetadata(ctx, logCtx, latestAppStatus)
243243
if err != nil {
244-
return a, revisionMetadata
244+
return a, syncRevisionsMetadata
245245
}
246246

247247
return latestAppStatus, revisionMetadataToReport
@@ -257,7 +257,7 @@ func (s *applicationEventReporter) processResource(
257257
appTree *appv1.ApplicationTree,
258258
manifestGenErr bool,
259259
originalApplication *appv1.Application,
260-
revisionMetadata *appv1.RevisionMetadata,
260+
revisionsMetadata *utils.AppSyncRevisionsMetadata,
261261
appInstanceLabelKey string,
262262
trackingMethod appv1.TrackingMethod,
263263
applicationVersions *apiclient.ApplicationVersions,
@@ -283,12 +283,12 @@ func (s *applicationEventReporter) processResource(
283283
return nil
284284
}
285285

286-
parentApplicationToReport, revisionMetadataToReport := s.getAppForResourceReporting(rs, ctx, parentApplication, revisionMetadata)
286+
parentApplicationToReport, revisionMetadataToReport := s.getAppForResourceReporting(rs, ctx, logCtx, parentApplication, revisionsMetadata)
287287

288-
var originalAppRevisionMetadata *appv1.RevisionMetadata = nil
288+
var originalAppRevisionMetadata *utils.AppSyncRevisionsMetadata = nil
289289

290290
if originalApplication != nil {
291-
originalAppRevisionMetadata, _ = s.getApplicationRevisionDetails(ctx, originalApplication, utils.GetOperationRevision(originalApplication))
291+
originalAppRevisionMetadata, _ = s.getApplicationRevisionsMetadata(ctx, logCtx, originalApplication)
292292
}
293293

294294
ev, err := getResourceEventPayload(parentApplicationToReport, &rs, actualState, desiredState, appTree, manifestGenErr, ts, originalApplication, revisionMetadataToReport, originalAppRevisionMetadata, appInstanceLabelKey, trackingMethod, applicationVersions)
@@ -305,7 +305,7 @@ func (s *applicationEventReporter) processResource(
305305
appName = appRes.Name
306306
} else {
307307
utils.LogWithResourceStatus(logCtx, rs).Info("streaming resource event")
308-
appName = rs.Name
308+
appName = parentApplication.Name
309309
}
310310

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

0 commit comments

Comments
 (0)