diff --git a/controller/appcontroller_test.go b/controller/appcontroller_test.go index c8c0129cac786..30eb964dcbcea 100644 --- a/controller/appcontroller_test.go +++ b/controller/appcontroller_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/argoproj/gitops-engine/pkg/health" "github.com/argoproj/gitops-engine/pkg/utils/kube/kubetest" "github.com/sirupsen/logrus" "github.com/stretchr/testify/require" @@ -19,6 +20,7 @@ import ( statecache "github.com/argoproj/argo-cd/v2/controller/cache" "github.com/argoproj/argo-cd/v2/controller/sharding" + dbmocks "github.com/argoproj/argo-cd/v2/util/db/mocks" "github.com/argoproj/gitops-engine/pkg/cache/mocks" synccommon "github.com/argoproj/gitops-engine/pkg/sync/common" "github.com/argoproj/gitops-engine/pkg/utils/kube" @@ -35,15 +37,12 @@ import ( "k8s.io/client-go/tools/cache" "sigs.k8s.io/yaml" - dbmocks "github.com/argoproj/argo-cd/v2/util/db/mocks" - mockstatecache "github.com/argoproj/argo-cd/v2/controller/cache/mocks" "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" appclientset "github.com/argoproj/argo-cd/v2/pkg/client/clientset/versioned/fake" "github.com/argoproj/argo-cd/v2/reposerver/apiclient" mockrepoclient "github.com/argoproj/argo-cd/v2/reposerver/apiclient/mocks" "github.com/argoproj/argo-cd/v2/test" - "github.com/argoproj/argo-cd/v2/util/argo/normalizers" cacheutil "github.com/argoproj/argo-cd/v2/util/cache" appstatecache "github.com/argoproj/argo-cd/v2/util/cache/appstate" "github.com/argoproj/argo-cd/v2/util/settings" @@ -55,15 +54,14 @@ type namespacedResource struct { } type fakeData struct { - apps []runtime.Object - manifestResponse *apiclient.ManifestResponse - manifestResponses []*apiclient.ManifestResponse - managedLiveObjs map[kube.ResourceKey]*unstructured.Unstructured - namespacedResources map[kube.ResourceKey]namespacedResource - configMapData map[string]string - metricsCacheExpiration time.Duration - applicationNamespaces []string - updateRevisionForPathsResponse *apiclient.UpdateRevisionForPathsResponse + apps []runtime.Object + manifestResponse *apiclient.ManifestResponse + manifestResponses []*apiclient.ManifestResponse + managedLiveObjs map[kube.ResourceKey]*unstructured.Unstructured + namespacedResources map[kube.ResourceKey]namespacedResource + configMapData map[string]string + metricsCacheExpiration time.Duration + applicationNamespaces []string } type MockKubectl struct { @@ -109,8 +107,6 @@ func newFakeController(data *fakeData, repoErr error) *ApplicationController { } } - mockRepoClient.On("UpdateRevisionForPaths", mock.Anything, mock.Anything).Return(data.updateRevisionForPathsResponse, nil) - mockRepoClientset := mockrepoclient.Clientset{RepoServerServiceClient: &mockRepoClient} secret := corev1.Secret{ @@ -160,9 +156,9 @@ func newFakeController(data *fakeData, repoErr error) *ApplicationController { nil, data.applicationNamespaces, nil, + false, false, - normalizers.IgnoreNormalizerOpts{}, ) db := &dbmocks.ArgoDB{} db.On("GetApplicationControllerReplicas").Return(1) @@ -407,6 +403,10 @@ func newFakeApp() *v1alpha1.Application { return createFakeApp(fakeApp) } +func newFakeAppWithHealthAndTime(status health.HealthStatusCode, timestamp metav1.Time) *v1alpha1.Application { + return createFakeAppWithHealthAndTime(fakeApp, status, timestamp) +} + func newFakeMultiSourceApp() *v1alpha1.Application { return createFakeApp(fakeMultiSourceApp) } @@ -428,6 +428,15 @@ func createFakeApp(testApp string) *v1alpha1.Application { return &app } +func createFakeAppWithHealthAndTime(testApp string, status health.HealthStatusCode, timestamp metav1.Time) *v1alpha1.Application { + app := createFakeApp(testApp) + app.Status.Health = v1alpha1.HealthStatus{ + Status: status, + Timestamp: timestamp, + } + return app +} + func newFakeCM() map[string]interface{} { var cm map[string]interface{} err := yaml.Unmarshal([]byte(fakeStrayResource), &cm) @@ -990,7 +999,7 @@ func TestNormalizeApplication(t *testing.T) { normalized := false fakeAppCs.AddReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) { if patchAction, ok := action.(kubetesting.PatchAction); ok { - if string(patchAction.GetPatch()) == `{"spec":{"project":"default"},"status":{"sync":{"comparedTo":{"destination":{},"source":{"repoURL":""}}}}}` { + if string(patchAction.GetPatch()) == `{"spec":{"project":"default"}}` { normalized = true } } @@ -1012,7 +1021,7 @@ func TestNormalizeApplication(t *testing.T) { normalized := false fakeAppCs.AddReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) { if patchAction, ok := action.(kubetesting.PatchAction); ok { - if string(patchAction.GetPatch()) == `{"spec":{"project":"default"},"status":{"sync":{"comparedTo":{"destination":{},"source":{"repoURL":""}}}}}` { + if string(patchAction.GetPatch()) == `{"spec":{"project":"default"}}` { normalized = true } } @@ -1102,8 +1111,8 @@ func TestGetResourceTree_HasOrphanedResources(t *testing.T) { }}) assert.NoError(t, err) - assert.Equal(t, []v1alpha1.ResourceNode{managedDeploy}, tree.Nodes) - assert.Equal(t, []v1alpha1.ResourceNode{orphanedDeploy1, orphanedDeploy2}, tree.OrphanedNodes) + assert.Equal(t, tree.Nodes, []v1alpha1.ResourceNode{managedDeploy}) + assert.Equal(t, tree.OrphanedNodes, []v1alpha1.ResourceNode{orphanedDeploy1, orphanedDeploy2}) } func TestSetOperationStateOnDeletedApp(t *testing.T) { @@ -1410,7 +1419,7 @@ func TestRefreshAppConditions(t *testing.T) { _, hasErrors := ctrl.refreshAppConditions(app) assert.False(t, hasErrors) - assert.Empty(t, app.Status.Conditions) + assert.Len(t, app.Status.Conditions, 0) }) t.Run("PreserveExistingWarningCondition", func(t *testing.T) { @@ -1719,36 +1728,6 @@ func TestProcessRequestedAppOperation_HasRetriesTerminated(t *testing.T) { assert.Equal(t, string(synccommon.OperationFailed), phase) } -func TestProcessRequestedAppOperation_Successful(t *testing.T) { - app := newFakeApp() - app.Spec.Project = "default" - app.Operation = &v1alpha1.Operation{ - Sync: &v1alpha1.SyncOperation{}, - } - ctrl := newFakeController(&fakeData{ - apps: []runtime.Object{app, &defaultProj}, - manifestResponses: []*apiclient.ManifestResponse{{ - Manifests: []string{}, - }}, - }, nil) - fakeAppCs := ctrl.applicationClientset.(*appclientset.Clientset) - receivedPatch := map[string]interface{}{} - fakeAppCs.PrependReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) { - if patchAction, ok := action.(kubetesting.PatchAction); ok { - assert.NoError(t, json.Unmarshal(patchAction.GetPatch(), &receivedPatch)) - } - return true, &v1alpha1.Application{}, nil - }) - - ctrl.processRequestedAppOperation(app) - - phase, _, _ := unstructured.NestedString(receivedPatch, "status", "operationState", "phase") - assert.Equal(t, string(synccommon.OperationSucceeded), phase) - ok, level := ctrl.isRefreshRequested(ctrl.toAppKey(app.Name)) - assert.True(t, ok) - assert.Equal(t, CompareWithLatestForceResolve, level) -} - func TestGetAppHosts(t *testing.T) { app := newFakeApp() data := &fakeData{ diff --git a/controller/health.go b/controller/health.go index f713a574f57d3..4c11571f4f19c 100644 --- a/controller/health.go +++ b/controller/health.go @@ -8,6 +8,7 @@ import ( "github.com/argoproj/gitops-engine/pkg/sync/ignore" kubeutil "github.com/argoproj/gitops-engine/pkg/utils/kube" log "github.com/sirupsen/logrus" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime/schema" "github.com/argoproj/argo-cd/v2/pkg/apis/application" @@ -15,12 +16,30 @@ import ( "github.com/argoproj/argo-cd/v2/util/lua" ) +func getOldTimestamp(statuses []appv1.ResourceStatus, i int) metav1.Time { + if len(statuses) == 0 { + return metav1.Now() + } + + oldTimestamp := statuses[i].Health.Timestamp + + if oldTimestamp.IsZero() { + oldTimestamp = metav1.Now() + } + + return oldTimestamp +} + // setApplicationHealth updates the health statuses of all resources performed in the comparison func setApplicationHealth(resources []managedResource, statuses []appv1.ResourceStatus, resourceOverrides map[string]appv1.ResourceOverride, app *appv1.Application, persistResourceHealth bool) (*appv1.HealthStatus, error) { var savedErr error var errCount uint + + // All statuses have the same timestamp, so we can safely get the first one + oldTimestamp := getOldTimestamp(statuses, 0) appHealth := appv1.HealthStatus{Status: health.HealthStatusHealthy} for i, res := range resources { + timestamp := metav1.Now() if res.Target != nil && hookutil.Skip(res.Target) { continue } @@ -54,7 +73,13 @@ func setApplicationHealth(resources []managedResource, statuses []appv1.Resource } if persistResourceHealth { - resHealth := appv1.HealthStatus{Status: healthStatus.Status, Message: healthStatus.Message} + + // If the status didn't change, we don't want to update the timestamp + if healthStatus.Status == statuses[i].Health.Status { + timestamp = getOldTimestamp(statuses, i) + } + + resHealth := appv1.HealthStatus{Status: healthStatus.Status, Message: healthStatus.Message, Timestamp: timestamp} statuses[i].Health = &resHealth } else { statuses[i].Health = nil @@ -72,6 +97,13 @@ func setApplicationHealth(resources []managedResource, statuses []appv1.Resource if health.IsWorse(appHealth.Status, healthStatus.Status) { appHealth.Status = healthStatus.Status + if persistResourceHealth { + appHealth.Timestamp = statuses[i].Health.Timestamp + } else { + appHealth.Timestamp = timestamp + } + } else if healthStatus.Status == health.HealthStatusHealthy { + appHealth.Timestamp = oldTimestamp } } if persistResourceHealth { @@ -80,7 +112,7 @@ func setApplicationHealth(resources []managedResource, statuses []appv1.Resource app.Status.ResourceHealthSource = appv1.ResourceHealthLocationAppTree } if savedErr != nil && errCount > 1 { - savedErr = fmt.Errorf("see application-controller logs for %d other errors; most recent error was: %w", errCount-1, savedErr) + savedErr = fmt.Errorf("see applicaton-controller logs for %d other errors; most recent error was: %w", errCount-1, savedErr) } return &appHealth, savedErr } diff --git a/controller/health_test.go b/controller/health_test.go index ca35a0a25caf2..c456248833ddb 100644 --- a/controller/health_test.go +++ b/controller/health_test.go @@ -3,6 +3,7 @@ package controller import ( "os" "testing" + "time" "github.com/argoproj/gitops-engine/pkg/health" synccommon "github.com/argoproj/gitops-engine/pkg/sync/common" @@ -18,12 +19,20 @@ import ( "github.com/argoproj/argo-cd/v2/util/lua" ) -var app = &appv1.Application{} +var ( + app = &appv1.Application{} + testTimestamp = metav1.NewTime(time.Date(2020, time.January, 1, 12, 0, 0, 0, time.UTC)) +) func initStatuses(resources []managedResource) []appv1.ResourceStatus { statuses := make([]appv1.ResourceStatus, len(resources)) for i := range resources { - statuses[i] = appv1.ResourceStatus{Group: resources[i].Group, Kind: resources[i].Kind, Version: resources[i].Version} + statuses[i] = appv1.ResourceStatus{ + Group: resources[i].Group, + Kind: resources[i].Kind, + Version: resources[i].Version, + Health: &appv1.HealthStatus{Timestamp: testTimestamp}, + } } return statuses } @@ -51,18 +60,30 @@ func TestSetApplicationHealth(t *testing.T) { }} resourceStatuses := initStatuses(resources) + // Populate health status + resourceStatuses[0].Health.Status = health.HealthStatusHealthy + healthStatus, err := setApplicationHealth(resources, resourceStatuses, lua.ResourceHealthOverrides{}, app, true) + firstHealthStatusTimestamp := healthStatus.Timestamp assert.NoError(t, err) assert.Equal(t, health.HealthStatusDegraded, healthStatus.Status) + assert.NotEqual(t, testTimestamp, firstHealthStatusTimestamp) + + assert.Equal(t, resourceStatuses[0].Health.Status, health.HealthStatusHealthy) + assert.Equal(t, testTimestamp, resourceStatuses[0].Health.Timestamp) + assert.Equal(t, resourceStatuses[1].Health.Status, health.HealthStatusDegraded) + assert.Equal(t, firstHealthStatusTimestamp, resourceStatuses[1].Health.Timestamp) - assert.Equal(t, health.HealthStatusHealthy, resourceStatuses[0].Health.Status) - assert.Equal(t, health.HealthStatusDegraded, resourceStatuses[1].Health.Status) + // Mark both health statuses as degraded, as app is degraded. + resourceStatuses[0].Health.Status = health.HealthStatusDegraded + resourceStatuses[1].Health.Status = health.HealthStatusDegraded // now mark the job as a hook and retry. it should ignore the hook and consider the app healthy failedJob.SetAnnotations(map[string]string{synccommon.AnnotationKeyHook: "PreSync"}) healthStatus, err = setApplicationHealth(resources, resourceStatuses, nil, app, true) assert.NoError(t, err) assert.Equal(t, health.HealthStatusHealthy, healthStatus.Status) + assert.Equal(t, testTimestamp, healthStatus.Timestamp) } func TestSetApplicationHealth_ResourceHealthNotPersisted(t *testing.T) { @@ -78,6 +99,7 @@ func TestSetApplicationHealth_ResourceHealthNotPersisted(t *testing.T) { assert.Equal(t, health.HealthStatusDegraded, healthStatus.Status) assert.Nil(t, resourceStatuses[0].Health) + assert.False(t, healthStatus.Timestamp.IsZero()) } func TestSetApplicationHealth_MissingResource(t *testing.T) { @@ -90,6 +112,32 @@ func TestSetApplicationHealth_MissingResource(t *testing.T) { healthStatus, err := setApplicationHealth(resources, resourceStatuses, lua.ResourceHealthOverrides{}, app, true) assert.NoError(t, err) assert.Equal(t, health.HealthStatusMissing, healthStatus.Status) + assert.False(t, healthStatus.Timestamp.IsZero()) +} + +func TestSetApplicationHealth_HealthImproves(t *testing.T) { + overrides := lua.ResourceHealthOverrides{ + lua.GetConfigMapKey(appv1.ApplicationSchemaGroupVersionKind): appv1.ResourceOverride{ + HealthLua: ` +hs = {} +hs.status = "Progressing" +hs.message = "" +return hs`, + }, + } + + degradedApp := newAppLiveObj(health.HealthStatusDegraded) + timestamp := metav1.Now() + resources := []managedResource{{ + Group: application.Group, Version: "v1alpha1", Kind: application.ApplicationKind, Live: degradedApp}, {}} + resourceStatuses := initStatuses(resources) + resourceStatuses[0].Health.Status = health.HealthStatusDegraded + resourceStatuses[0].Health.Timestamp = timestamp + + healthStatus, err := setApplicationHealth(resources, resourceStatuses, overrides, app, true) + assert.NoError(t, err) + assert.Equal(t, health.HealthStatusProgressing, healthStatus.Status) + assert.NotEqual(t, healthStatus.Timestamp, timestamp) } func TestSetApplicationHealth_MissingResourceNoBuiltHealthCheck(t *testing.T) { @@ -114,6 +162,7 @@ func TestSetApplicationHealth_MissingResourceNoBuiltHealthCheck(t *testing.T) { }, app, true) assert.NoError(t, err) assert.Equal(t, health.HealthStatusMissing, healthStatus.Status) + assert.False(t, healthStatus.Timestamp.IsZero()) }) } diff --git a/controller/state.go b/controller/state.go index d0e5a159287e9..14001dcdb2c1a 100644 --- a/controller/state.go +++ b/controller/state.go @@ -33,10 +33,8 @@ import ( "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" appclientset "github.com/argoproj/argo-cd/v2/pkg/client/clientset/versioned" "github.com/argoproj/argo-cd/v2/reposerver/apiclient" - "github.com/argoproj/argo-cd/v2/util/app/path" "github.com/argoproj/argo-cd/v2/util/argo" argodiff "github.com/argoproj/argo-cd/v2/util/argo/diff" - "github.com/argoproj/argo-cd/v2/util/argo/normalizers" appstatecache "github.com/argoproj/argo-cd/v2/util/cache/appstate" "github.com/argoproj/argo-cd/v2/util/db" "github.com/argoproj/argo-cd/v2/util/gpg" @@ -71,9 +69,9 @@ type managedResource struct { // AppStateManager defines methods which allow to compare application spec and actual application state. type AppStateManager interface { - CompareAppState(app *v1alpha1.Application, project *v1alpha1.AppProject, revisions []string, sources []v1alpha1.ApplicationSource, noCache bool, noRevisionCache bool, localObjects []string, hasMultipleSources bool, rollback bool) (*comparisonResult, error) + 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, rollback bool) ([]*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, error) } // comparisonResult holds the state of an application after the reconciliation @@ -119,14 +117,13 @@ type appStateManager struct { repoErrorCache goSync.Map repoErrorGracePeriod time.Duration serverSideDiff bool - ignoreNormalizerOpts normalizers.IgnoreNormalizerOpts } // GetRepoObjs will generate the manifests for the given application delegating the // 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, rollback bool) ([]*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, error) { ts := stats.NewTimingStats() helmRepos, err := m.db.ListHelmRepositories(context.Background()) if err != nil { @@ -178,9 +175,7 @@ func (m *appStateManager) GetRepoObjs(app *v1alpha1.Application, sources []v1alp targetObjs := make([]*unstructured.Unstructured, 0) // Store the map of all sources having ref field into a map for applications with sources field - // If it's for a rollback process, the refSources[*].targetRevision fields are the desired - // revisions for the rollback - refSources, err := argo.GetRefSources(context.Background(), sources, app.Spec.Project, m.db.GetRepository, revisions, rollback) + 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) } @@ -190,7 +185,7 @@ func (m *appStateManager) GetRepoObjs(app *v1alpha1.Application, sources []v1alp revisions[i] = source.TargetRevision } ts.AddCheckpoint("helm_ms") - repo, err := m.db.GetRepository(context.Background(), source.RepoURL, proj.Name) + 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) } @@ -199,38 +194,6 @@ func (m *appStateManager) GetRepoObjs(app *v1alpha1.Application, sources []v1alp return 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 - if app.Spec.HasMultipleSources() { - if i < len(app.Status.Sync.Revisions) { - syncedRevision = app.Status.Sync.Revisions[i] - } else { - syncedRevision = "" - } - } - - 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{ - Repo: repo, - Revision: revisions[i], - SyncedRevision: syncedRevision, - Paths: path.GetAppRefreshPaths(app), - AppLabelKey: appLabelKey, - AppName: app.InstanceName(m.namespace), - Namespace: app.Spec.Destination.Namespace, - ApplicationSource: &source, - KubeVersion: serverVersion, - ApiVersions: argo.APIResourcesToStrings(apiResources, true), - TrackingMethod: string(argo.GetTrackingMethod(m.settingsMgr)), - RefSources: refSources, - HasMultipleSources: app.Spec.HasMultipleSources(), - }) - if err != nil { - return nil, nil, fmt.Errorf("failed to compare revisions for source %d of %d: %w", i+1, len(sources), err) - } - } - ts.AddCheckpoint("version_ms") log.Debugf("Generating Manifest for source %s revision %s", source, revisions[i]) manifestInfo, err := repoClient.GenerateManifest(context.Background(), &apiclient.ManifestRequest{ @@ -266,6 +229,7 @@ func (m *appStateManager) GetRepoObjs(app *v1alpha1.Application, sources []v1alp return nil, nil, fmt.Errorf("failed to unmarshal manifests for source %d of %d: %w", i+1, len(sources), err) } targetObjs = append(targetObjs, targetObj...) + manifestInfos = append(manifestInfos, manifestInfo) } @@ -396,7 +360,7 @@ func isManagedNamespace(ns *unstructured.Unstructured, app *v1alpha1.Application // CompareAppState compares application git state to the live app state, using the specified // revision and supplied source. If revision or overrides are empty, then compares against // revision and overrides in the app spec. -func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1alpha1.AppProject, revisions []string, sources []v1alpha1.ApplicationSource, noCache bool, noRevisionCache bool, localManifests []string, hasMultipleSources bool, rollback bool) (*comparisonResult, error) { +func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1alpha1.AppProject, revisions []string, sources []v1alpha1.ApplicationSource, noCache bool, noRevisionCache bool, localManifests []string, hasMultipleSources bool) (*comparisonResult, error) { ts := stats.NewTimingStats() appLabelKey, resourceOverrides, resFilter, err := m.getComparisonSettings() @@ -411,7 +375,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1 Status: v1alpha1.SyncStatusCodeUnknown, Revisions: revisions, }, - healthStatus: &v1alpha1.HealthStatus{Status: health.HealthStatusUnknown}, + healthStatus: &v1alpha1.HealthStatus{Status: health.HealthStatusUnknown, Timestamp: metav1.Now()}, }, nil } else { return &comparisonResult{ @@ -420,7 +384,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1 Status: v1alpha1.SyncStatusCodeUnknown, Revision: revisions[0], }, - healthStatus: &v1alpha1.HealthStatus{Status: health.HealthStatusUnknown}, + healthStatus: &v1alpha1.HealthStatus{Status: health.HealthStatusUnknown, Timestamp: metav1.Now()}, }, nil } } @@ -454,7 +418,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1 } } - targetObjs, manifestInfos, err = m.GetRepoObjs(app, sources, appLabelKey, revisions, noCache, noRevisionCache, verifySignature, project, rollback) + targetObjs, manifestInfos, 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()) @@ -641,7 +605,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1 useDiffCache := useDiffCache(noCache, manifestInfos, sources, app, manifestRevisions, m.statusRefreshTimeout, serverSideDiff, logCtx) diffConfigBuilder := argodiff.NewDiffConfigBuilder(). - WithDiffSettings(app.Spec.IgnoreDifferences, resourceOverrides, compareOptions.IgnoreAggregatedRoles, m.ignoreNormalizerOpts). + WithDiffSettings(app.Spec.IgnoreDifferences, resourceOverrides, compareOptions.IgnoreAggregatedRoles). WithTracking(appLabelKey, string(trackingMethod)) if useDiffCache { @@ -713,6 +677,7 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1 Kind: gvk.Kind, Version: gvk.Version, Group: gvk.Group, + Health: &app.Status.Health, Hook: isHook(obj), RequiresPruning: targetObj == nil && liveObj != nil && isSelfReferencedObj, } @@ -982,7 +947,6 @@ func NewAppStateManager( persistResourceHealth bool, repoErrorGracePeriod time.Duration, serverSideDiff bool, - ignoreNormalizerOpts normalizers.IgnoreNormalizerOpts, ) AppStateManager { return &appStateManager{ liveStateCache: liveStateCache, @@ -1000,7 +964,6 @@ func NewAppStateManager( persistResourceHealth: persistResourceHealth, repoErrorGracePeriod: repoErrorGracePeriod, serverSideDiff: serverSideDiff, - ignoreNormalizerOpts: ignoreNormalizerOpts, } } diff --git a/controller/state_test.go b/controller/state_test.go index 95dba246a72cf..d483c2b33081c 100644 --- a/controller/state_test.go +++ b/controller/state_test.go @@ -23,11 +23,9 @@ import ( "k8s.io/apimachinery/pkg/runtime" "github.com/argoproj/argo-cd/v2/common" - "github.com/argoproj/argo-cd/v2/controller/testdata" "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" argoappv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" "github.com/argoproj/argo-cd/v2/reposerver/apiclient" - mockrepoclient "github.com/argoproj/argo-cd/v2/reposerver/apiclient/mocks" "github.com/argoproj/argo-cd/v2/test" "github.com/argoproj/argo-cd/v2/util/argo" ) @@ -49,14 +47,14 @@ func TestCompareAppStateEmpty(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) - assert.Empty(t, compRes.resources) - assert.Empty(t, compRes.managedResources) - assert.Empty(t, app.Status.Conditions) + assert.Len(t, compRes.resources, 0) + assert.Len(t, compRes.managedResources, 0) + assert.Len(t, app.Status.Conditions, 0) } // TestCompareAppStateRepoError tests the case when CompareAppState notices a repo error @@ -67,21 +65,21 @@ func TestCompareAppStateRepoError(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) assert.Nil(t, compRes) assert.EqualError(t, err, CompareStateRepoError.Error()) // expect to still get compare state error to as inside grace period - compRes, err = ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err = ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) assert.Nil(t, compRes) assert.EqualError(t, err, CompareStateRepoError.Error()) time.Sleep(10 * time.Second) // expect to not get error as outside of grace period, but status should be unknown - compRes, err = ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err = ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) assert.NotNil(t, compRes) - assert.NoError(t, err) - assert.Equal(t, argoappv1.SyncStatusCodeUnknown, compRes.syncStatus.Status) + assert.Nil(t, err) + assert.Equal(t, compRes.syncStatus.Status, argoappv1.SyncStatusCodeUnknown) } // TestCompareAppStateNamespaceMetadataDiffers tests comparison when managed namespace metadata differs @@ -113,14 +111,14 @@ func TestCompareAppStateNamespaceMetadataDiffers(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeOutOfSync, compRes.syncStatus.Status) - assert.Empty(t, compRes.resources) - assert.Empty(t, compRes.managedResources) - assert.Empty(t, app.Status.Conditions) + assert.Len(t, compRes.resources, 0) + assert.Len(t, compRes.managedResources, 0) + assert.Len(t, app.Status.Conditions, 0) } // TestCompareAppStateNamespaceMetadataDiffers tests comparison when managed namespace metadata differs to live and manifest ns @@ -162,8 +160,8 @@ func TestCompareAppStateNamespaceMetadataDiffersToManifest(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeOutOfSync, compRes.syncStatus.Status) @@ -181,7 +179,7 @@ func TestCompareAppStateNamespaceMetadataDiffersToManifest(t *testing.T) { assert.Equal(t, map[string]string{}, labels) // Manifests override definitions in managedNamespaceMetadata assert.Equal(t, map[string]string{"bar": "bat"}, result.GetAnnotations()) - assert.Empty(t, app.Status.Conditions) + assert.Len(t, app.Status.Conditions, 0) } // TestCompareAppStateNamespaceMetadata tests comparison when managed namespace metadata differs to live @@ -220,8 +218,8 @@ func TestCompareAppStateNamespaceMetadata(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeOutOfSync, compRes.syncStatus.Status) @@ -238,7 +236,7 @@ func TestCompareAppStateNamespaceMetadata(t *testing.T) { assert.Equal(t, map[string]string{"foo": "bar"}, labels) assert.Equal(t, map[string]string{"argocd.argoproj.io/sync-options": "ServerSideApply=true", "bar": "bat", "foo": "bar"}, result.GetAnnotations()) - assert.Empty(t, app.Status.Conditions) + assert.Len(t, app.Status.Conditions, 0) } // TestCompareAppStateNamespaceMetadataIsTheSame tests comparison when managed namespace metadata is the same @@ -279,14 +277,14 @@ func TestCompareAppStateNamespaceMetadataIsTheSame(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) - assert.Empty(t, compRes.resources) - assert.Empty(t, compRes.managedResources) - assert.Empty(t, app.Status.Conditions) + assert.Len(t, compRes.resources, 0) + assert.Len(t, compRes.managedResources, 0) + assert.Len(t, app.Status.Conditions, 0) } // TestCompareAppStateMissing tests when there is a manifest defined in the repo which doesn't exist in live @@ -307,14 +305,14 @@ func TestCompareAppStateMissing(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeOutOfSync, compRes.syncStatus.Status) assert.Len(t, compRes.resources, 1) assert.Len(t, compRes.managedResources, 1) - assert.Empty(t, app.Status.Conditions) + assert.Len(t, app.Status.Conditions, 0) } // TestCompareAppStateExtra tests when there is an extra object in live but not defined in git @@ -339,13 +337,13 @@ func TestCompareAppStateExtra(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.Equal(t, argoappv1.SyncStatusCodeOutOfSync, compRes.syncStatus.Status) - assert.Len(t, compRes.resources, 1) - assert.Len(t, compRes.managedResources, 1) - assert.Empty(t, app.Status.Conditions) + assert.Equal(t, 1, len(compRes.resources)) + assert.Equal(t, 1, len(compRes.managedResources)) + assert.Equal(t, 0, len(app.Status.Conditions)) } // TestCompareAppStateHook checks that hooks are detected during manifest generation, and not @@ -370,14 +368,14 @@ func TestCompareAppStateHook(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) - assert.Empty(t, compRes.resources) - assert.Empty(t, compRes.managedResources) - assert.Len(t, compRes.reconciliationResult.Hooks, 1) - assert.Empty(t, app.Status.Conditions) + assert.Equal(t, 0, len(compRes.resources)) + assert.Equal(t, 0, len(compRes.managedResources)) + assert.Equal(t, 1, len(compRes.reconciliationResult.Hooks)) + assert.Equal(t, 0, len(app.Status.Conditions)) } // TestCompareAppStateSkipHook checks that skipped resources are detected during manifest generation, and not @@ -402,14 +400,14 @@ func TestCompareAppStateSkipHook(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) - assert.Len(t, compRes.resources, 1) - assert.Len(t, compRes.managedResources, 1) - assert.Empty(t, compRes.reconciliationResult.Hooks) - assert.Empty(t, app.Status.Conditions) + assert.Equal(t, 1, len(compRes.resources)) + assert.Equal(t, 1, len(compRes.managedResources)) + assert.Equal(t, 0, len(compRes.reconciliationResult.Hooks)) + assert.Equal(t, 0, len(app.Status.Conditions)) } // checks that ignore resources are detected, but excluded from status @@ -433,14 +431,14 @@ func TestCompareAppStateCompareOptionIgnoreExtraneous(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) - assert.Empty(t, compRes.resources) - assert.Empty(t, compRes.managedResources) - assert.Empty(t, app.Status.Conditions) + assert.Len(t, compRes.resources, 0) + assert.Len(t, compRes.managedResources, 0) + assert.Len(t, app.Status.Conditions, 0) } // TestCompareAppStateExtraHook tests when there is an extra _hook_ object in live but not defined in git @@ -466,15 +464,15 @@ func TestCompareAppStateExtraHook(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) - assert.Len(t, compRes.resources, 1) - assert.Len(t, compRes.managedResources, 1) - assert.Empty(t, compRes.reconciliationResult.Hooks) - assert.Empty(t, app.Status.Conditions) + assert.Equal(t, 1, len(compRes.resources)) + assert.Equal(t, 1, len(compRes.managedResources)) + assert.Equal(t, 0, len(compRes.reconciliationResult.Hooks)) + assert.Equal(t, 0, len(app.Status.Conditions)) } // TestAppRevisions tests that revisions are properly propagated for a single source app @@ -495,12 +493,12 @@ func TestAppRevisionsSingleSource(t *testing.T) { app := newFakeApp() revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, app.Spec.GetSources(), false, false, nil, app.Spec.HasMultipleSources(), false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, app.Spec.GetSources(), false, false, nil, app.Spec.HasMultipleSources()) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.NotEmpty(t, compRes.syncStatus.Revision) - assert.Empty(t, compRes.syncStatus.Revisions) + assert.Len(t, compRes.syncStatus.Revisions, 0) } // TestAppRevisions tests that revisions are properly propagated for a multi source app @@ -535,8 +533,8 @@ func TestAppRevisionsMultiSource(t *testing.T) { app := newFakeMultiSourceApp() revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, app.Spec.GetSources(), false, false, nil, app.Spec.HasMultipleSources(), false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, app.Spec.GetSources(), false, false, nil, app.Spec.HasMultipleSources()) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Empty(t, compRes.syncStatus.Revision) @@ -583,15 +581,15 @@ func TestCompareAppStateDuplicatedNamespacedResources(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) - assert.Len(t, app.Status.Conditions, 1) + assert.Equal(t, 1, len(app.Status.Conditions)) assert.NotNil(t, app.Status.Conditions[0].LastTransitionTime) assert.Equal(t, argoappv1.ApplicationConditionRepeatedResourceWarning, app.Status.Conditions[0].Type) assert.Equal(t, "Resource /Pod/fake-dest-ns/my-pod appeared 2 times among application resources.", app.Status.Conditions[0].Message) - assert.Len(t, compRes.resources, 4) + assert.Equal(t, 4, len(compRes.resources)) } func TestCompareAppStateManagedNamespaceMetadataWithLiveNsDoesNotGetPruned(t *testing.T) { @@ -620,11 +618,11 @@ func TestCompareAppStateManagedNamespaceMetadataWithLiveNsDoesNotGetPruned(t *te }, } ctrl := newFakeController(&data, nil) - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, []string{}, app.Spec.Sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, []string{}, app.Spec.Sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) - assert.Empty(t, app.Status.Conditions) + assert.Equal(t, 0, len(app.Status.Conditions)) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) // Ensure that ns does not get pruned @@ -652,38 +650,45 @@ var defaultProj = argoappv1.AppProject{ }, } -// TestCompareAppStateWithManifestGeneratePath tests that it compares revisions when the manifest-generate-path annotation is set. -func TestCompareAppStateWithManifestGeneratePath(t *testing.T) { +func TestSetHealth(t *testing.T) { app := newFakeApp() - app.SetAnnotations(map[string]string{argoappv1.AnnotationKeyManifestGeneratePaths: "."}) - app.Status.Sync = argoappv1.SyncStatus{ - Revision: "abc123", - Status: argoappv1.SyncStatusCodeSynced, - } - - data := fakeData{ + deployment := kube.MustToUnstructured(&v1.Deployment{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apps/v1", + Kind: "Deployment", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "demo", + Namespace: "default", + }, + }) + ctrl := newFakeController(&fakeData{ + apps: []runtime.Object{app, &defaultProj}, manifestResponse: &apiclient.ManifestResponse{ Manifests: []string{}, Namespace: test.FakeDestNamespace, Server: test.FakeClusterURL, Revision: "abc123", }, - updateRevisionForPathsResponse: &apiclient.UpdateRevisionForPathsResponse{}, - } + managedLiveObjs: map[kube.ResourceKey]*unstructured.Unstructured{ + kube.GetResourceKey(deployment): deployment, + }, + }, nil) - ctrl := newFakeController(&data, nil) + sources := make([]argoappv1.ApplicationSource, 0) + sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) - revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, app.Spec.GetSources(), false, false, nil, false, false) - assert.NoError(t, err) - assert.NotNil(t, compRes) - assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) - assert.Equal(t, "abc123", compRes.syncStatus.Revision) - ctrl.repoClientset.(*mockrepoclient.Clientset).RepoServerServiceClient.(*mockrepoclient.RepoServerServiceClient).AssertNumberOfCalls(t, "UpdateRevisionForPaths", 1) + revisions = append(revisions, "") + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) + + assert.Equal(t, health.HealthStatusHealthy, compRes.healthStatus.Status) + assert.False(t, compRes.healthStatus.Timestamp.IsZero()) } -func TestSetHealth(t *testing.T) { - app := newFakeApp() +func TestPreserveStatusTimestamp(t *testing.T) { + timestamp := metav1.Now() + app := newFakeAppWithHealthAndTime(health.HealthStatusHealthy, timestamp) deployment := kube.MustToUnstructured(&v1.Deployment{ TypeMeta: metav1.TypeMeta{ APIVersion: "apps/v1", @@ -711,10 +716,11 @@ func TestSetHealth(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.Equal(t, health.HealthStatusHealthy, compRes.healthStatus.Status) + assert.Equal(t, timestamp, compRes.healthStatus.Timestamp) } func TestSetHealthSelfReferencedApp(t *testing.T) { @@ -748,10 +754,11 @@ func TestSetHealthSelfReferencedApp(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.Equal(t, health.HealthStatusHealthy, compRes.healthStatus.Status) + assert.False(t, compRes.healthStatus.Timestamp.IsZero()) } func TestSetManagedResourcesWithOrphanedResources(t *testing.T) { @@ -774,7 +781,7 @@ func TestSetManagedResourcesWithOrphanedResources(t *testing.T) { tree, err := ctrl.setAppManagedResources(app, &comparisonResult{managedResources: make([]managedResource, 0)}) assert.NoError(t, err) - assert.Len(t, tree.OrphanedNodes, 1) + assert.Equal(t, len(tree.OrphanedNodes), 1) assert.Equal(t, "guestbook", tree.OrphanedNodes[0].Name) assert.Equal(t, app.Namespace, tree.OrphanedNodes[0].Namespace) } @@ -803,7 +810,7 @@ func TestSetManagedResourcesWithResourcesOfAnotherApp(t *testing.T) { tree, err := ctrl.setAppManagedResources(app1, &comparisonResult{managedResources: make([]managedResource, 0)}) assert.NoError(t, err) - assert.Empty(t, tree.OrphanedNodes) + assert.Equal(t, 0, len(tree.OrphanedNodes)) } func TestReturnUnknownComparisonStateOnSettingLoadError(t *testing.T) { @@ -823,10 +830,11 @@ func TestReturnUnknownComparisonStateOnSettingLoadError(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.Equal(t, health.HealthStatusUnknown, compRes.healthStatus.Status) + assert.False(t, compRes.healthStatus.Timestamp.IsZero()) assert.Equal(t, argoappv1.SyncStatusCodeUnknown, compRes.syncStatus.Status) } @@ -964,14 +972,14 @@ func TestSignedResponseNoSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) - assert.Empty(t, compRes.resources) - assert.Empty(t, compRes.managedResources) - assert.Empty(t, app.Status.Conditions) + assert.Len(t, compRes.resources, 0) + assert.Len(t, compRes.managedResources, 0) + assert.Len(t, app.Status.Conditions, 0) } // We have a bad signature response, but project does not require signed commits { @@ -991,14 +999,14 @@ func TestSignedResponseNoSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) - assert.Empty(t, compRes.resources) - assert.Empty(t, compRes.managedResources) - assert.Empty(t, app.Status.Conditions) + assert.Len(t, compRes.resources, 0) + assert.Len(t, compRes.managedResources, 0) + assert.Len(t, app.Status.Conditions, 0) } } @@ -1023,14 +1031,14 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) - assert.Empty(t, compRes.resources) - assert.Empty(t, compRes.managedResources) - assert.Empty(t, app.Status.Conditions) + assert.Len(t, compRes.resources, 0) + assert.Len(t, compRes.managedResources, 0) + assert.Len(t, app.Status.Conditions, 0) } // We have a bad signature response and signing is required - do not sync { @@ -1050,13 +1058,13 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) - assert.Empty(t, compRes.resources) - assert.Empty(t, compRes.managedResources) + assert.Len(t, compRes.resources, 0) + assert.Len(t, compRes.managedResources, 0) assert.Len(t, app.Status.Conditions, 1) } // We have a malformed signature response and signing is required - do not sync @@ -1077,13 +1085,13 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) - assert.Empty(t, compRes.resources) - assert.Empty(t, compRes.managedResources) + assert.Len(t, compRes.resources, 0) + assert.Len(t, compRes.managedResources, 0) assert.Len(t, app.Status.Conditions, 1) } // We have no signature response (no signature made) and signing is required - do not sync @@ -1104,13 +1112,13 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) - assert.Empty(t, compRes.resources) - assert.Empty(t, compRes.managedResources) + assert.Len(t, compRes.resources, 0) + assert.Len(t, compRes.managedResources, 0) assert.Len(t, app.Status.Conditions, 1) } @@ -1134,13 +1142,13 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(app, &testProj, revisions, sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &testProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) - assert.Empty(t, compRes.resources) - assert.Empty(t, compRes.managedResources) + assert.Len(t, compRes.resources, 0) + assert.Len(t, compRes.managedResources, 0) assert.Len(t, app.Status.Conditions, 1) assert.Contains(t, app.Status.Conditions[0].Message, "key is not allowed") } @@ -1164,13 +1172,13 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, localManifests, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, localManifests, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeUnknown, compRes.syncStatus.Status) - assert.Empty(t, compRes.resources) - assert.Empty(t, compRes.managedResources) + assert.Len(t, compRes.resources, 0) + assert.Len(t, compRes.managedResources, 0) assert.Len(t, app.Status.Conditions, 1) assert.Contains(t, app.Status.Conditions[0].Message, "Cannot use local manifests") } @@ -1194,14 +1202,14 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) - assert.Empty(t, compRes.resources) - assert.Empty(t, compRes.managedResources) - assert.Empty(t, app.Status.Conditions) + assert.Len(t, compRes.resources, 0) + assert.Len(t, compRes.managedResources, 0) + assert.Len(t, app.Status.Conditions, 0) } // Signature required and local manifests supplied and GPG subsystem is disabled - sync @@ -1224,14 +1232,14 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, localManifests, false, false) - assert.NoError(t, err) + compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, localManifests, false) + assert.Nil(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) - assert.Empty(t, compRes.resources) - assert.Empty(t, compRes.managedResources) - assert.Empty(t, app.Status.Conditions) + assert.Len(t, compRes.resources, 0) + assert.Len(t, compRes.managedResources, 0) + assert.Len(t, app.Status.Conditions, 0) } } @@ -1541,17 +1549,6 @@ func TestUseDiffCache(t *testing.T) { expectedUseCache: true, serverSideDiff: false, }, - { - testName: "will use diff cache with sync policy", - noCache: false, - manifestInfos: manifestInfos("rev1"), - sources: sources(), - app: test.YamlToApplication(testdata.DiffCacheYaml), - manifestRevisions: []string{"rev1"}, - statusRefreshTimeout: time.Hour * 24, - expectedUseCache: true, - serverSideDiff: true, - }, { testName: "will use diff cache for multisource", noCache: false, @@ -1705,7 +1702,7 @@ func TestUseDiffCache(t *testing.T) { useDiffCache := useDiffCache(tc.noCache, tc.manifestInfos, tc.sources, tc.app, tc.manifestRevisions, tc.statusRefreshTimeout, tc.serverSideDiff, log) // Then - assert.Equal(t, tc.expectedUseCache, useDiffCache) + assert.Equal(t, useDiffCache, tc.expectedUseCache) }) } } diff --git a/pkg/apis/application/v1alpha1/types.go b/pkg/apis/application/v1alpha1/types.go index 428fdcc00d255..4d5b7732e5b04 100644 --- a/pkg/apis/application/v1alpha1/types.go +++ b/pkg/apis/application/v1alpha1/types.go @@ -51,7 +51,6 @@ import ( // +kubebuilder:printcolumn:name="Sync Status",type=string,JSONPath=`.status.sync.status` // +kubebuilder:printcolumn:name="Health Status",type=string,JSONPath=`.status.health.status` // +kubebuilder:printcolumn:name="Revision",type=string,JSONPath=`.status.sync.revision`,priority=10 -// +kubebuilder:printcolumn:name="Project",type=string,JSONPath=`.spec.project`,priority=10 type Application struct { metav1.TypeMeta `json:",inline"` metav1.ObjectMeta `json:"metadata" protobuf:"bytes,1,opt,name=metadata"` @@ -206,11 +205,6 @@ func (s ApplicationSources) Equals(other ApplicationSources) bool { return true } -// IsZero returns true if the application source is considered empty -func (a ApplicationSources) IsZero() bool { - return len(a) == 0 -} - func (a *ApplicationSpec) GetSource() ApplicationSource { // if Application has multiple sources, return the first source in sources if a.HasMultipleSources() { @@ -236,17 +230,9 @@ func (a *ApplicationSpec) HasMultipleSources() bool { return a.Sources != nil && len(a.Sources) > 0 } -func (a *ApplicationSpec) GetSourcePtrByPosition(sourcePosition int) *ApplicationSource { - // if Application has multiple sources, return the first source in sources - return a.GetSourcePtrByIndex(sourcePosition - 1) -} - -func (a *ApplicationSpec) GetSourcePtrByIndex(sourceIndex int) *ApplicationSource { +func (a *ApplicationSpec) GetSourcePtr() *ApplicationSource { // if Application has multiple sources, return the first source in sources if a.HasMultipleSources() { - if sourceIndex > 0 { - return &a.Sources[sourceIndex] - } return &a.Sources[0] } return a.Source @@ -262,11 +248,6 @@ func (a *ApplicationSource) AllowsConcurrentProcessing() bool { return true } -// IsRef returns true when the application source is of type Ref -func (a *ApplicationSource) IsRef() bool { - return a.Ref != "" -} - // IsHelm returns true when the application source is of type Helm func (a *ApplicationSource) IsHelm() bool { return a.Chart != "" @@ -488,8 +469,6 @@ type ApplicationSourceKustomize struct { Patches KustomizePatches `json:"patches,omitempty" protobuf:"bytes,12,opt,name=patches"` // Components specifies a list of kustomize components to add to the kustomization before building Components []string `json:"components,omitempty" protobuf:"bytes,13,rep,name=components"` - //LabelWithoutSelector specifies whether to apply common labels to resource selectors or not - LabelWithoutSelector bool `json:"labelWithoutSelector,omitempty" protobuf:"bytes,14,opt,name=labelWithoutSelector"` } type KustomizeReplica struct { @@ -1520,8 +1499,7 @@ type SyncStatus struct { // Status is the sync state of the comparison Status SyncStatusCode `json:"status" protobuf:"bytes,1,opt,name=status,casttype=SyncStatusCode"` // ComparedTo contains information about what has been compared - // +patchStrategy=replace - ComparedTo ComparedTo `json:"comparedTo,omitempty" protobuf:"bytes,2,opt,name=comparedTo" patchStrategy:"replace"` + ComparedTo ComparedTo `json:"comparedTo,omitempty" protobuf:"bytes,2,opt,name=comparedTo"` // Revision contains information about the revision the comparison has been performed to Revision string `json:"revision,omitempty" protobuf:"bytes,3,opt,name=revision"` // Revisions contains information about the revisions of multiple sources the comparison has been performed to @@ -1534,6 +1512,8 @@ type HealthStatus struct { Status health.HealthStatusCode `json:"status,omitempty" protobuf:"bytes,1,opt,name=status"` // Message is a human-readable informational message describing the health status Message string `json:"message,omitempty" protobuf:"bytes,2,opt,name=message"` + // Timestamp is the time the HealthStatus was set + Timestamp metav1.Time `json:"timestamp,omitempty" protobuf:"bytes,3,opt,name=timestamp"` } // InfoItem contains arbitrary, human readable information about an application @@ -1705,7 +1685,7 @@ type ResourceStatus struct { SyncWave int64 `json:"syncWave,omitempty" protobuf:"bytes,10,opt,name=syncWave"` } -// GroupVersionKind returns the GVK schema type for given resource status +// GroupKindVersion returns the GVK schema type for given resource status func (r *ResourceStatus) GroupVersionKind() schema.GroupVersionKind { return schema.GroupVersionKind{Group: r.Group, Version: r.Version, Kind: r.Kind} } @@ -2101,12 +2081,6 @@ func isValidResource(resource string) bool { return validResources[resource] } -func isValidObject(proj string, object string) bool { - // match against [/]/ - objectRegexp, err := regexp.Compile(fmt.Sprintf(`^%s(/[*\w-.]+)?/[*\w-.]+$`, regexp.QuoteMeta(proj))) - return objectRegexp.MatchString(object) && err == nil -} - func validatePolicy(proj string, role string, policy string) error { policyComponents := strings.Split(policy, ",") if len(policyComponents) != 6 || strings.Trim(policyComponents[0], " ") != "p" { @@ -2130,8 +2104,9 @@ func validatePolicy(proj string, role string, policy string) error { } // object object := strings.Trim(policyComponents[4], " ") - if !isValidObject(proj, object) { - return status.Errorf(codes.InvalidArgument, "invalid policy rule '%s': object must be of form '%s/*', '%s[/]/' or '%s/', not '%s'", policy, proj, proj, proj, object) + objectRegexp, err := regexp.Compile(fmt.Sprintf(`^%s/[*\w-.]+$`, regexp.QuoteMeta(proj))) + if err != nil || !objectRegexp.MatchString(object) { + return status.Errorf(codes.InvalidArgument, "invalid policy rule '%s': object must be of form '%s/*' or '%s/', not '%s'", policy, proj, proj, object) } // effect effect := strings.Trim(policyComponents[5], " ") @@ -2587,11 +2562,11 @@ func (w *SyncWindow) Validate() error { specParser := cron.NewParser(cron.Minute | cron.Hour | cron.Dom | cron.Month | cron.Dow) _, err := specParser.Parse(w.Schedule) if err != nil { - return fmt.Errorf("cannot parse schedule '%s': %w", w.Schedule, err) + return fmt.Errorf("cannot parse schedule '%s': %s", w.Schedule, err) } _, err = time.ParseDuration(w.Duration) if err != nil { - return fmt.Errorf("cannot parse duration '%s': %w", w.Duration, err) + return fmt.Errorf("cannot parse duration '%s': %s", w.Duration, err) } return nil }