Skip to content

Commit

Permalink
Merge branch 'release-2.11' of github.com:codefresh-io/argo-cd into r…
Browse files Browse the repository at this point in the history
…elease-2.11
  • Loading branch information
pasha-codefresh committed Jul 30, 2024
2 parents f09e338 + c787eab commit a31bf96
Show file tree
Hide file tree
Showing 14 changed files with 1,019 additions and 723 deletions.
7 changes: 7 additions & 0 deletions assets/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -9302,6 +9302,13 @@
"comparedTo": {
"$ref": "#/definitions/v1alpha1ComparedTo"
},
"manifestsChanged": {
"type": "object",
"title": "ManifestsChanged indicates whether the manifests have changed base on argocd.argoproj.io/manifest-generate-paths annotation",
"additionalProperties": {
"type": "boolean"
}
},
"revision": {
"type": "string",
"title": "Revision contains information about the revision the comparison has been performed to"
Expand Down
8 changes: 6 additions & 2 deletions controller/appcontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"math"
"math/rand"
"net/http"
"os"
"reflect"
"runtime/debug"
"sort"
Expand Down Expand Up @@ -1959,8 +1960,11 @@ func alreadyAttemptedSync(app *appv1.Application, commitSHA string, commitSHAsMS
return false, ""
}
} else {
if app.Status.OperationState.SyncResult.Revision != commitSHA {
return false, ""
manifestsChangedMap := app.Status.Sync.ManifestsChanged
if os.Getenv("PERSIST_CHANGE_REVISIONS") != "1" || manifestsChangedMap[commitSHA] {
if app.Status.OperationState.SyncResult.Revision != commitSHA {
return false, ""
}
}
}

Expand Down
52 changes: 52 additions & 0 deletions controller/appcontroller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"encoding/json"
"errors"
"os"
"testing"
"time"

Expand Down Expand Up @@ -1929,3 +1930,54 @@ func TestAddControllerNamespace(t *testing.T) {
assert.Equal(t, test.FakeArgoCDNamespace, updatedApp.Status.ControllerNamespace)
})
}

func TestAlreadyAttemptSync(t *testing.T) {
app := newFakeApp()
t.Run("same manifest with sync result, with disabled flag", func(t *testing.T) {

manifestChangedMap := make(map[string]bool)
manifestChangedMap["sha"] = false

app.Status.Sync.ManifestsChanged = manifestChangedMap

attempted, _ := alreadyAttemptedSync(app, "sha", []string{}, false)
assert.False(t, attempted)
})

t.Run("same manifest with sync result, with enabled flag", func(t *testing.T) {

_ = os.Setenv("PERSIST_CHANGE_REVISIONS", "1")

manifestChangedMap := make(map[string]bool)
manifestChangedMap["sha"] = false

app.Status.Sync.ManifestsChanged = manifestChangedMap

attempted, _ := alreadyAttemptedSync(app, "sha", []string{}, false)
assert.True(t, attempted)
})

t.Run("different manifest with sync result, with disabled flag", func(t *testing.T) {

manifestChangedMap := make(map[string]bool)
manifestChangedMap["sha"] = true

app.Status.Sync.ManifestsChanged = manifestChangedMap

attempted, _ := alreadyAttemptedSync(app, "sha", []string{}, false)
assert.False(t, attempted)
})

t.Run("different manifest with sync result, with enabled flag", func(t *testing.T) {

_ = os.Setenv("PERSIST_CHANGE_REVISIONS", "1")

manifestChangedMap := make(map[string]bool)
manifestChangedMap["sha"] = true

app.Status.Sync.ManifestsChanged = manifestChangedMap

attempted, _ := alreadyAttemptedSync(app, "sha", []string{}, false)
assert.False(t, attempted)
})
}
2 changes: 1 addition & 1 deletion controller/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func (ctrl *ApplicationController) executePostDeleteHooks(app *v1alpha1.Applicat
revisions = append(revisions, src.TargetRevision)
}

targets, _, err := ctrl.appStateManager.GetRepoObjs(app, app.Spec.GetSources(), appLabelKey, revisions, false, false, false, proj)
targets, _, _, err := ctrl.appStateManager.GetRepoObjs(app, app.Spec.GetSources(), appLabelKey, revisions, false, false, false, proj)
if err != nil {
return false, err
}
Expand Down
60 changes: 36 additions & 24 deletions controller/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"errors"
"fmt"
"os"
"reflect"
"strings"
goSync "sync"
Expand Down Expand Up @@ -73,7 +74,7 @@ type managedResource struct {
type AppStateManager interface {
CompareAppState(app *v1alpha1.Application, project *v1alpha1.AppProject, revisions []string, sources []v1alpha1.ApplicationSource, noCache bool, noRevisionCache bool, localObjects []string, hasMultipleSources bool) (*comparisonResult, error)
SyncAppState(app *v1alpha1.Application, state *v1alpha1.OperationState)
GetRepoObjs(app *v1alpha1.Application, sources []v1alpha1.ApplicationSource, appLabelKey string, revisions []string, noCache, noRevisionCache, verifySignature bool, proj *v1alpha1.AppProject) ([]*unstructured.Unstructured, []*apiclient.ManifestResponse, error)
GetRepoObjs(app *v1alpha1.Application, sources []v1alpha1.ApplicationSource, appLabelKey string, revisions []string, noCache, noRevisionCache, verifySignature bool, proj *v1alpha1.AppProject) ([]*unstructured.Unstructured, []*apiclient.ManifestResponse, map[string]bool, error)
}

// comparisonResult holds the state of an application after the reconciliation
Expand Down Expand Up @@ -126,51 +127,51 @@ type appStateManager struct {
// task to the repo-server. It returns the list of generated manifests as unstructured
// objects. It also returns the full response from all calls to the repo server as the
// second argument.
func (m *appStateManager) GetRepoObjs(app *v1alpha1.Application, sources []v1alpha1.ApplicationSource, appLabelKey string, revisions []string, noCache, noRevisionCache, verifySignature bool, proj *v1alpha1.AppProject) ([]*unstructured.Unstructured, []*apiclient.ManifestResponse, error) {
func (m *appStateManager) GetRepoObjs(app *v1alpha1.Application, sources []v1alpha1.ApplicationSource, appLabelKey string, revisions []string, noCache, noRevisionCache, verifySignature bool, proj *v1alpha1.AppProject) ([]*unstructured.Unstructured, []*apiclient.ManifestResponse, map[string]bool, error) {
ts := stats.NewTimingStats()
helmRepos, err := m.db.ListHelmRepositories(context.Background())
if err != nil {
return nil, nil, fmt.Errorf("failed to list Helm repositories: %w", err)
return nil, nil, nil, fmt.Errorf("failed to list Helm repositories: %w", err)
}
permittedHelmRepos, err := argo.GetPermittedRepos(proj, helmRepos)
if err != nil {
return nil, nil, fmt.Errorf("failed to get permitted Helm repositories for project %q: %w", proj.Name, err)
return nil, nil, nil, fmt.Errorf("failed to get permitted Helm repositories for project %q: %w", proj.Name, err)
}

ts.AddCheckpoint("repo_ms")
helmRepositoryCredentials, err := m.db.GetAllHelmRepositoryCredentials(context.Background())
if err != nil {
return nil, nil, fmt.Errorf("failed to get Helm credentials: %w", err)
return nil, nil, nil, fmt.Errorf("failed to get Helm credentials: %w", err)
}
permittedHelmCredentials, err := argo.GetPermittedReposCredentials(proj, helmRepositoryCredentials)
if err != nil {
return nil, nil, fmt.Errorf("failed to get permitted Helm credentials for project %q: %w", proj.Name, err)
return nil, nil, nil, fmt.Errorf("failed to get permitted Helm credentials for project %q: %w", proj.Name, err)
}

enabledSourceTypes, err := m.settingsMgr.GetEnabledSourceTypes()
if err != nil {
return nil, nil, fmt.Errorf("failed to get enabled source types: %w", err)
return nil, nil, nil, fmt.Errorf("failed to get enabled source types: %w", err)
}
ts.AddCheckpoint("plugins_ms")

kustomizeSettings, err := m.settingsMgr.GetKustomizeSettings()
if err != nil {
return nil, nil, fmt.Errorf("failed to get Kustomize settings: %w", err)
return nil, nil, nil, fmt.Errorf("failed to get Kustomize settings: %w", err)
}

helmOptions, err := m.settingsMgr.GetHelmSettings()
if err != nil {
return nil, nil, fmt.Errorf("failed to get Helm settings: %w", err)
return nil, nil, nil, fmt.Errorf("failed to get Helm settings: %w", err)
}

ts.AddCheckpoint("build_options_ms")
serverVersion, apiResources, err := m.liveStateCache.GetVersionsInfo(app.Spec.Destination.Server)
if err != nil {
return nil, nil, fmt.Errorf("failed to get cluster version for cluster %q: %w", app.Spec.Destination.Server, err)
return nil, nil, nil, fmt.Errorf("failed to get cluster version for cluster %q: %w", app.Spec.Destination.Server, err)
}
conn, repoClient, err := m.repoClientset.NewRepoServerClient()
if err != nil {
return nil, nil, fmt.Errorf("failed to connect to repo server: %w", err)
return nil, nil, nil, fmt.Errorf("failed to connect to repo server: %w", err)
}
defer io.Close(conn)

Expand All @@ -180,21 +181,23 @@ func (m *appStateManager) GetRepoObjs(app *v1alpha1.Application, sources []v1alp
// Store the map of all sources having ref field into a map for applications with sources field
refSources, err := argo.GetRefSources(context.Background(), app.Spec, m.db)
if err != nil {
return nil, nil, fmt.Errorf("failed to get ref sources: %v", err)
return nil, nil, nil, fmt.Errorf("failed to get ref sources: %v", err)
}

manifestsChanges := make(map[string]bool)

for i, source := range sources {
if len(revisions) < len(sources) || revisions[i] == "" {
revisions[i] = source.TargetRevision
}
ts.AddCheckpoint("helm_ms")
repo, err := m.db.GetRepository(context.Background(), source.RepoURL)
if err != nil {
return nil, nil, fmt.Errorf("failed to get repo %q: %w", source.RepoURL, err)
return nil, nil, nil, fmt.Errorf("failed to get repo %q: %w", source.RepoURL, err)
}
kustomizeOptions, err := kustomizeSettings.GetOptions(source, m.settingsMgr.GetKustomizeSetNamespaceEnabled())
if err != nil {
return nil, nil, fmt.Errorf("failed to get Kustomize options for source %d of %d: %w", i+1, len(sources), err)
return nil, nil, nil, fmt.Errorf("failed to get Kustomize options for source %d of %d: %w", i+1, len(sources), err)
}

syncedRevision := app.Status.Sync.Revision
Expand All @@ -209,7 +212,7 @@ func (m *appStateManager) GetRepoObjs(app *v1alpha1.Application, sources []v1alp
val, ok := app.Annotations[v1alpha1.AnnotationKeyManifestGeneratePaths]
if !source.IsHelm() && syncedRevision != "" && ok && val != "" {
// Validate the manifest-generate-path annotation to avoid generating manifests if it has not changed.
_, err = repoClient.UpdateRevisionForPaths(context.Background(), &apiclient.UpdateRevisionForPathsRequest{
updateRevisionResponse, err := repoClient.UpdateRevisionForPaths(context.Background(), &apiclient.UpdateRevisionForPathsRequest{
Repo: repo,
Revision: revisions[i],
SyncedRevision: syncedRevision,
Expand All @@ -224,8 +227,13 @@ func (m *appStateManager) GetRepoObjs(app *v1alpha1.Application, sources []v1alp
RefSources: refSources,
HasMultipleSources: app.Spec.HasMultipleSources(),
})

// not need to change, if we already found at least one cached revision that has no changes
if updateRevisionResponse != nil && os.Getenv("PERSIST_CHANGE_REVISIONS") == "1" {
manifestsChanges[syncedRevision] = updateRevisionResponse.Changes
}
if err != nil {
return nil, nil, fmt.Errorf("failed to compare revisions for source %d of %d: %w", i+1, len(sources), err)
return nil, nil, nil, fmt.Errorf("failed to compare revisions for source %d of %d: %w", i+1, len(sources), err)
}
}

Expand Down Expand Up @@ -256,13 +264,13 @@ func (m *appStateManager) GetRepoObjs(app *v1alpha1.Application, sources []v1alp
ApplicationMetadata: &app.ObjectMeta,
})
if err != nil {
return nil, nil, fmt.Errorf("failed to generate manifest for source %d of %d: %w", i+1, len(sources), err)
return nil, nil, nil, fmt.Errorf("failed to generate manifest for source %d of %d: %w", i+1, len(sources), err)
}

targetObj, err := unmarshalManifests(manifestInfo.GetCompiledManifests())

if err != nil {
return nil, nil, fmt.Errorf("failed to unmarshal manifests for source %d of %d: %w", i+1, len(sources), err)
return nil, nil, nil, fmt.Errorf("failed to unmarshal manifests for source %d of %d: %w", i+1, len(sources), err)
}
targetObjs = append(targetObjs, targetObj...)

Expand All @@ -276,7 +284,7 @@ func (m *appStateManager) GetRepoObjs(app *v1alpha1.Application, sources []v1alp
}
logCtx = logCtx.WithField("time_ms", time.Since(ts.StartTime).Milliseconds())
logCtx.Info("GetRepoObjs stats")
return targetObjs, manifestInfos, nil
return targetObjs, manifestInfos, manifestsChanges, nil
}

func unmarshalManifests(manifests []string) ([]*unstructured.Unstructured, error) {
Expand Down Expand Up @@ -444,6 +452,8 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
var manifestInfos []*apiclient.ManifestResponse
targetNsExists := false

var manifestChanged map[string]bool

if len(localManifests) == 0 {
// If the length of revisions is not same as the length of sources,
// we take the revisions from the sources directly for all the sources.
Expand All @@ -454,7 +464,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
}
}

targetObjs, manifestInfos, err = m.GetRepoObjs(app, sources, appLabelKey, revisions, noCache, noRevisionCache, verifySignature, project)
targetObjs, manifestInfos, manifestChanged, err = m.GetRepoObjs(app, sources, appLabelKey, revisions, noCache, noRevisionCache, verifySignature, project)
if err != nil {
targetObjs = make([]*unstructured.Unstructured, 0)
msg := fmt.Sprintf("Failed to load target state: %s", err.Error())
Expand Down Expand Up @@ -828,8 +838,9 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
Sources: sources,
IgnoreDifferences: app.Spec.IgnoreDifferences,
},
Status: syncCode,
Revisions: manifestRevisions,
Status: syncCode,
Revisions: manifestRevisions,
ManifestsChanged: manifestChanged,
}
} else {
syncStatus = v1alpha1.SyncStatus{
Expand All @@ -838,8 +849,9 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
Source: app.Spec.GetSource(),
IgnoreDifferences: app.Spec.IgnoreDifferences,
},
Status: syncCode,
Revision: revision,
Status: syncCode,
Revision: revision,
ManifestsChanged: manifestChanged,
}
}

Expand Down
7 changes: 7 additions & 0 deletions manifests/core-install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5036,6 +5036,13 @@ spec:
required:
- destination
type: object
manifestsChanged:
additionalProperties:
type: boolean
description: ManifestsChanged indicates whether the manifests
have changed base on argocd.argoproj.io/manifest-generate-paths
annotation
type: object
revision:
description: Revision contains information about the revision
the comparison has been performed to
Expand Down
7 changes: 7 additions & 0 deletions manifests/crds/application-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5035,6 +5035,13 @@ spec:
required:
- destination
type: object
manifestsChanged:
additionalProperties:
type: boolean
description: ManifestsChanged indicates whether the manifests
have changed base on argocd.argoproj.io/manifest-generate-paths
annotation
type: object
revision:
description: Revision contains information about the revision
the comparison has been performed to
Expand Down
7 changes: 7 additions & 0 deletions manifests/ha/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5036,6 +5036,13 @@ spec:
required:
- destination
type: object
manifestsChanged:
additionalProperties:
type: boolean
description: ManifestsChanged indicates whether the manifests
have changed base on argocd.argoproj.io/manifest-generate-paths
annotation
type: object
revision:
description: Revision contains information about the revision
the comparison has been performed to
Expand Down
7 changes: 7 additions & 0 deletions manifests/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5036,6 +5036,13 @@ spec:
required:
- destination
type: object
manifestsChanged:
additionalProperties:
type: boolean
description: ManifestsChanged indicates whether the manifests
have changed base on argocd.argoproj.io/manifest-generate-paths
annotation
type: object
revision:
description: Revision contains information about the revision
the comparison has been performed to
Expand Down
Loading

0 comments on commit a31bf96

Please sign in to comment.