Skip to content

Commit

Permalink
fix: acr controller crashes when it is newly created app (#343)
Browse files Browse the repository at this point in the history
* feat: acr controller crashes when here no history

* feat: acr controller crashes when here no history

* feat: acr controller crashes when here no history

* feat: acr controller crashes when here no history

* feat: acr controller crashes when here no history
  • Loading branch information
pasha-codefresh authored Oct 16, 2024
1 parent 96507c8 commit 2161a75
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 8 deletions.
15 changes: 11 additions & 4 deletions acr_controller/service/acr_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func (c *acrService) ChangeRevision(ctx context.Context, a *application.Applicat

func (c *acrService) calculateRevision(ctx context.Context, a *application.Application) (*string, error) {
currentRevision, previousRevision := c.getRevisions(ctx, a)
log.Infof("Calculate revision for application %s, current revision %s, previous revision %s", a.Name, currentRevision, previousRevision)
log.Infof("Calculate revision for application '%s', current revision '%s', previous revision '%s'", a.Name, currentRevision, previousRevision)
changeRevisionResult, err := c.applicationServiceClient.GetChangeRevision(ctx, &appclient.ChangeRevisionRequest{
AppName: pointer.String(a.GetName()),
Namespace: pointer.String(a.GetNamespace()),
Expand Down Expand Up @@ -171,10 +171,17 @@ func (c *acrService) patchOperationSyncResultWithChangeRevision(ctx context.Cont
return err
}

func getCurrentRevisionFromOperation(a *application.Application) string {
if a.Operation != nil && a.Operation.Sync != nil {
return a.Operation.Sync.Revision
}
return ""
}

func (c *acrService) getRevisions(ctx context.Context, a *application.Application) (string, string) {
if a.Status.History == nil || len(a.Status.History) == 0 {
// it is first sync operation, and we dont need detect change revision in such case
return "", ""
// it is first sync operation, and we have only current revision
return getCurrentRevisionFromOperation(a), ""
}

// in case if sync is already done, we need to use revision from sync result and previous revision from history
Expand All @@ -184,7 +191,7 @@ func (c *acrService) getRevisions(ctx context.Context, a *application.Applicatio
}

// in case if sync is in progress, we need to use revision from operation and revision from latest history record
currentRevision := a.Operation.Sync.Revision
currentRevision := getCurrentRevisionFromOperation(a)
previousRevision := a.Status.History[len(a.Status.History)-1].Revision
return currentRevision, previousRevision
}
40 changes: 40 additions & 0 deletions acr_controller/service/acr_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,39 @@ spec:
server: https://cluster-api.example.com
`

const fakeAppWithOperation = `
apiVersion: argoproj.io/v1alpha1
kind: Application
metadata:
annotations:
argocd.argoproj.io/manifest-generate-paths: .
finalizers:
- resources-finalizer.argocd.argoproj.io
labels:
app.kubernetes.io/instance: guestbook
name: guestbook
namespace: codefresh
operation:
initiatedBy:
automated: true
retry:
limit: 5
sync:
prune: true
revision: c732f4d2ef24c7eeb900e9211ff98f90bb646505
syncOptions:
- CreateNamespace=true
spec:
destination:
namespace: guestbook
server: https://kubernetes.default.svc
project: default
source:
path: apps/guestbook
repoURL: https://github.com/pasha-codefresh/precisely-gitsource.git
targetRevision: HEAD
`

const syncedAppWithHistory = `
apiVersion: argoproj.io/v1alpha1
kind: Application
Expand Down Expand Up @@ -135,6 +168,13 @@ func Test_getRevisions(r *testing.T) {
assert.Equal(t, "", previous)
})

r.Run("history list is empty, but operation happens right now", func(t *testing.T) {
acrService := newTestACRService(&mocks.ApplicationClient{})
current, previous := acrService.getRevisions(context.TODO(), createTestApp(fakeAppWithOperation))
assert.Equal(t, "c732f4d2ef24c7eeb900e9211ff98f90bb646505", current)
assert.Equal(t, "", previous)
})

r.Run("application is synced", func(t *testing.T) {
acrService := newTestACRService(&mocks.ApplicationClient{})
app := createTestApp(syncedAppWithHistory)
Expand Down
4 changes: 0 additions & 4 deletions changelog/CHANGELOG.md

This file was deleted.

5 changes: 5 additions & 0 deletions util/git/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -907,6 +907,11 @@ func (m *nativeGitClient) Diff(targetRevision string) ([]string, error) {
}

func (m *nativeGitClient) ListRevisions(revision string, targetRevision string) ([]string, error) {
// it happens when app just created and there is no revision yet
if revision == "" {
return []string{targetRevision}, nil
}

if !IsCommitSHA(revision) || !IsCommitSHA(targetRevision) {
return nil, fmt.Errorf("invalid revision provided, must be SHA")
}
Expand Down

0 comments on commit 2161a75

Please sign in to comment.