Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: acr controller crashes when it is newly created app #343

Merged
merged 5 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 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 @@ -173,7 +173,10 @@ func (c *acrService) patchOperationSyncResultWithChangeRevision(ctx context.Cont

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
// it is first sync operation, and we have only current revision
if a.Operation != nil && a.Operation.Sync != nil {
pasha-codefresh marked this conversation as resolved.
Show resolved Hide resolved
return a.Operation.Sync.Revision, ""
}
return "", ""
}

Expand Down
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
Loading