diff --git a/cmd/argocd/commands/admin/app.go b/cmd/argocd/commands/admin/app.go index 01c03abce298c..972e0ab63b41a 100644 --- a/cmd/argocd/commands/admin/app.go +++ b/cmd/argocd/commands/admin/app.go @@ -442,7 +442,7 @@ func reconcileApplications( sources = append(sources, app.Spec.GetSource()) revisions = append(revisions, app.Spec.GetSource().TargetRevision) - res, err := appStateManager.CompareAppState(destCluster, &app, proj, revisions, sources, false, false, nil, false, false) + res, err := appStateManager.CompareAppState(&app, proj, revisions, sources, false, false, nil, false, false) if err != nil { return nil, fmt.Errorf("error comparing app states: %w", err) } diff --git a/controller/appcontroller.go b/controller/appcontroller.go index 028deb09c67e6..58dc017e775ea 100644 --- a/controller/appcontroller.go +++ b/controller/appcontroller.go @@ -1257,7 +1257,7 @@ func (ctrl *ApplicationController) finalizeApplicationDeletion(app *appv1.Applic return err } - done, err := ctrl.executePostDeleteHooks(destCluster, app, proj, objsMap, config, logCtx) + done, err := ctrl.executePostDeleteHooks(app, proj, objsMap, config, logCtx) if err != nil { return err } @@ -1410,11 +1410,12 @@ func (ctrl *ApplicationController) processRequestedAppOperation(app *appv1.Appli } ts.AddCheckpoint("initial_operation_stage_ms") - if destCluster, err := argo.GetDestinationCluster(context.Background(), app.Spec.Destination, ctrl.db); err != nil { + // Call GetDestinationCluster to validate the destination cluster. + if _, err := argo.GetDestinationCluster(context.Background(), app.Spec.Destination, ctrl.db); err != nil { state.Phase = synccommon.OperationFailed state.Message = err.Error() } else { - ctrl.appStateManager.SyncAppState(destCluster, app, state) + ctrl.appStateManager.SyncAppState(app, state) } ts.AddCheckpoint("validate_and_sync_app_state_ms") @@ -1712,9 +1713,7 @@ func (ctrl *ApplicationController) processAppRefreshQueueItem() (processNext boo sources = append(sources, app.Spec.GetSource()) } - compareResult, err := ctrl.appStateManager.CompareAppState(destCluster, app, project, revisions, sources, - refreshType == appv1.RefreshTypeHard, - comparisonLevel == CompareWithLatestForceResolve, localManifests, hasMultipleSources, false) + compareResult, err := ctrl.appStateManager.CompareAppState(app, project, revisions, sources, refreshType == appv1.RefreshTypeHard, comparisonLevel == CompareWithLatestForceResolve, localManifests, hasMultipleSources, false) ts.AddCheckpoint("compare_app_state_ms") diff --git a/controller/hook.go b/controller/hook.go index db453c29aebda..b0fd8ebb039b4 100644 --- a/controller/hook.go +++ b/controller/hook.go @@ -41,7 +41,7 @@ func isPostDeleteHook(obj *unstructured.Unstructured) bool { return false } -func (ctrl *ApplicationController) executePostDeleteHooks(destCluster *v1alpha1.Cluster, app *v1alpha1.Application, proj *v1alpha1.AppProject, liveObjs map[kube.ResourceKey]*unstructured.Unstructured, config *rest.Config, logCtx *log.Entry) (bool, error) { +func (ctrl *ApplicationController) executePostDeleteHooks(app *v1alpha1.Application, proj *v1alpha1.AppProject, liveObjs map[kube.ResourceKey]*unstructured.Unstructured, config *rest.Config, logCtx *log.Entry) (bool, error) { appLabelKey, err := ctrl.settingsMgr.GetAppInstanceLabelKey() if err != nil { return false, err @@ -51,7 +51,7 @@ func (ctrl *ApplicationController) executePostDeleteHooks(destCluster *v1alpha1. revisions = append(revisions, src.TargetRevision) } - targets, _, _, err := ctrl.appStateManager.GetRepoObjs(destCluster, app, app.Spec.GetSources(), appLabelKey, revisions, false, false, false, proj, false, true) + targets, _, _, err := ctrl.appStateManager.GetRepoObjs(app, app.Spec.GetSources(), appLabelKey, revisions, false, false, false, proj, false, true) if err != nil { return false, err } diff --git a/controller/hydrator_dependencies.go b/controller/hydrator_dependencies.go index 158686cf3e946..b9c0430a4206c 100644 --- a/controller/hydrator_dependencies.go +++ b/controller/hydrator_dependencies.go @@ -4,8 +4,6 @@ import ( "context" "fmt" - argoutil "github.com/argoproj/argo-cd/v2/util/argo" - "github.com/argoproj/argo-cd/v2/controller/hydrator" appv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" "github.com/argoproj/argo-cd/v2/reposerver/apiclient" @@ -41,13 +39,8 @@ func (ctrl *ApplicationController) GetRepoObjs(app *appv1.Application, source ap return nil, nil, fmt.Errorf("failed to get app instance label key: %w", err) } - destCluster, err := argoutil.GetDestinationCluster(context.Background(), app.Spec.Destination, ctrl.db) - if err != nil { - return nil, nil, fmt.Errorf("failed to get destination cluster: %w", err) - } - // FIXME: use cache and revision cache - objs, resp, _, err := ctrl.appStateManager.GetRepoObjs(destCluster, app, sources, appLabelKey, revisions, true, true, false, project, false, false) + objs, resp, _, err := ctrl.appStateManager.GetRepoObjs(app, sources, appLabelKey, revisions, true, true, false, project, false, false) if err != nil { return nil, nil, fmt.Errorf("failed to get repo objects: %w", err) } diff --git a/controller/state.go b/controller/state.go index 568f502b3fa8f..d8552d3e92e31 100644 --- a/controller/state.go +++ b/controller/state.go @@ -69,9 +69,9 @@ type managedResource struct { // AppStateManager defines methods which allow to compare application spec and actual application state. type AppStateManager interface { - CompareAppState(destCluster *v1alpha1.Cluster, app *v1alpha1.Application, project *v1alpha1.AppProject, revisions []string, sources []v1alpha1.ApplicationSource, noCache bool, noRevisionCache bool, localObjects []string, hasMultipleSources bool, rollback bool) (*comparisonResult, error) - SyncAppState(destCluster *v1alpha1.Cluster, app *v1alpha1.Application, state *v1alpha1.OperationState) - GetRepoObjs(destCluster *v1alpha1.Cluster, app *v1alpha1.Application, sources []v1alpha1.ApplicationSource, appLabelKey string, revisions []string, noCache, noRevisionCache, verifySignature bool, proj *v1alpha1.AppProject, rollback, sendRuntimeState bool) ([]*unstructured.Unstructured, []*apiclient.ManifestResponse, bool, error) + CompareAppState(app *v1alpha1.Application, project *v1alpha1.AppProject, revisions []string, sources []v1alpha1.ApplicationSource, noCache bool, noRevisionCache bool, localObjects []string, hasMultipleSources bool, rollback 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, sendRuntimeState bool) ([]*unstructured.Unstructured, []*apiclient.ManifestResponse, bool, error) } // comparisonResult holds the state of an application after the reconciliation @@ -125,7 +125,7 @@ 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(destCluster *v1alpha1.Cluster, app *v1alpha1.Application, sources []v1alpha1.ApplicationSource, appLabelKey string, revisions []string, noCache, noRevisionCache, verifySignature bool, proj *v1alpha1.AppProject, rollback, sendRuntimeState bool) ([]*unstructured.Unstructured, []*apiclient.ManifestResponse, bool, error) { +func (m *appStateManager) GetRepoObjs(app *v1alpha1.Application, sources []v1alpha1.ApplicationSource, appLabelKey string, revisions []string, noCache, noRevisionCache, verifySignature bool, proj *v1alpha1.AppProject, rollback, sendRuntimeState bool) ([]*unstructured.Unstructured, []*apiclient.ManifestResponse, bool, error) { ts := stats.NewTimingStats() helmRepos, err := m.db.ListHelmRepositories(context.Background()) if err != nil { @@ -167,6 +167,11 @@ func (m *appStateManager) GetRepoObjs(destCluster *v1alpha1.Cluster, app *v1alph return nil, nil, false, fmt.Errorf("failed to get installation ID: %w", err) } + destCluster, err := argo.GetDestinationCluster(context.Background(), app.Spec.Destination, m.db) + if err != nil { + return nil, nil, false, fmt.Errorf("failed to get destination cluster: %w", err) + } + ts.AddCheckpoint("build_options_ms") serverVersion, apiResources, err := m.liveStateCache.GetVersionsInfo(destCluster) if err != nil { @@ -470,7 +475,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(destCluster *v1alpha1.Cluster, 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, rollback bool) (*comparisonResult, error) { ts := stats.NewTimingStats() appLabelKey, resourceOverrides, resFilter, installationID, err := m.getComparisonSettings() @@ -510,6 +515,11 @@ func (m *appStateManager) CompareAppState(destCluster *v1alpha1.Cluster, app *v1 failedToLoadObjs := false conditions := make([]v1alpha1.ApplicationCondition, 0) + destCluster, err := argo.GetDestinationCluster(context.Background(), app.Spec.Destination, m.db) + if err != nil { + return nil, err + } + logCtx := log.WithField("application", app.QualifiedName()) logCtx.Infof("Comparing app state (cluster: %s, namespace: %s)", destCluster.Server, app.Spec.Destination.Namespace) @@ -531,7 +541,7 @@ func (m *appStateManager) CompareAppState(destCluster *v1alpha1.Cluster, app *v1 } } - targetObjs, manifestInfos, revisionUpdated, err = m.GetRepoObjs(destCluster, app, sources, appLabelKey, revisions, noCache, noRevisionCache, verifySignature, project, rollback, true) + targetObjs, manifestInfos, revisionUpdated, err = m.GetRepoObjs(app, sources, appLabelKey, revisions, noCache, noRevisionCache, verifySignature, project, rollback, true) if err != nil { targetObjs = make([]*unstructured.Unstructured, 0) msg := "Failed to load target state: " + err.Error() diff --git a/controller/state_test.go b/controller/state_test.go index 30c57a2d5e03a..e7cc131fde169 100644 --- a/controller/state_test.go +++ b/controller/state_test.go @@ -51,7 +51,7 @@ func TestCompareAppStateEmpty(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -69,18 +69,18 @@ func TestCompareAppStateRepoError(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) assert.Nil(t, compRes) require.EqualError(t, err, CompareStateRepoError.Error()) // expect to still get compare state error to as inside grace period - compRes, err = ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err = ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) assert.Nil(t, compRes) require.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(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err = ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) assert.NotNil(t, compRes) require.NoError(t, err) assert.Equal(t, argoappv1.SyncStatusCodeUnknown, compRes.syncStatus.Status) @@ -115,7 +115,7 @@ func TestCompareAppStateNamespaceMetadataDiffers(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -164,7 +164,7 @@ func TestCompareAppStateNamespaceMetadataDiffersToManifest(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -222,7 +222,7 @@ func TestCompareAppStateNamespaceMetadata(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -281,7 +281,7 @@ func TestCompareAppStateNamespaceMetadataIsTheSame(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -309,7 +309,7 @@ func TestCompareAppStateMissing(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -341,7 +341,7 @@ func TestCompareAppStateExtra(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.Equal(t, argoappv1.SyncStatusCodeOutOfSync, compRes.syncStatus.Status) @@ -372,7 +372,7 @@ func TestCompareAppStateHook(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) @@ -404,7 +404,7 @@ func TestCompareAppStateSkipHook(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) @@ -435,7 +435,7 @@ func TestCompareAppStateCompareOptionIgnoreExtraneous(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) @@ -468,7 +468,7 @@ func TestCompareAppStateExtraHook(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) @@ -497,7 +497,7 @@ func TestAppRevisionsSingleSource(t *testing.T) { app := newFakeApp() revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, app.Spec.GetSources(), false, false, nil, app.Spec.HasMultipleSources(), false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, app.Spec.GetSources(), false, false, nil, app.Spec.HasMultipleSources(), false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -537,7 +537,7 @@ func TestAppRevisionsMultiSource(t *testing.T) { app := newFakeMultiSourceApp() revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, app.Spec.GetSources(), false, false, nil, app.Spec.HasMultipleSources(), false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, app.Spec.GetSources(), false, false, nil, app.Spec.HasMultipleSources(), false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -586,7 +586,7 @@ func TestCompareAppStateDuplicatedNamespacedResources(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) @@ -623,7 +623,7 @@ func TestCompareAppStateManagedNamespaceMetadataWithLiveNsDoesNotGetPruned(t *te }, } ctrl := newFakeController(&data, nil) - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, []string{}, app.Spec.Sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, []string{}, app.Spec.Sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) @@ -677,7 +677,7 @@ func TestCompareAppStateWithManifestGeneratePath(t *testing.T) { ctrl := newFakeController(&data, nil) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, app.Spec.GetSources(), false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, app.Spec.GetSources(), false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.Equal(t, argoappv1.SyncStatusCodeSynced, compRes.syncStatus.Status) @@ -713,7 +713,7 @@ func TestSetHealth(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.Equal(t, health.HealthStatusHealthy, compRes.healthStatus.Status) @@ -750,7 +750,7 @@ func TestPreserveStatusTimestamp(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.Equal(t, health.HealthStatusHealthy, compRes.healthStatus.Status) @@ -788,7 +788,7 @@ func TestSetHealthSelfReferencedApp(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.Equal(t, health.HealthStatusHealthy, compRes.healthStatus.Status) @@ -864,7 +864,7 @@ func TestReturnUnknownComparisonStateOnSettingLoadError(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.Equal(t, health.HealthStatusUnknown, compRes.healthStatus.Status) @@ -1006,7 +1006,7 @@ func TestSignedResponseNoSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -1033,7 +1033,7 @@ func TestSignedResponseNoSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -1065,7 +1065,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &signedProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -1092,7 +1092,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &signedProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -1119,7 +1119,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &signedProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -1146,7 +1146,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &signedProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -1176,7 +1176,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &testProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &testProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -1206,7 +1206,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &signedProj, revisions, sources, false, false, localManifests, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, localManifests, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -1236,7 +1236,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &signedProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -1266,7 +1266,7 @@ func TestSignedResponseSignatureRequired(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "abc123") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &signedProj, revisions, sources, false, false, localManifests, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &signedProj, revisions, sources, false, false, localManifests, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -1770,7 +1770,7 @@ func TestCompareAppStateDefaultRevisionUpdated(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) @@ -1793,7 +1793,7 @@ func TestCompareAppStateRevisionUpdatedWithHelmSource(t *testing.T) { sources = append(sources, app.Spec.GetSource()) revisions := make([]string, 0) revisions = append(revisions, "") - compRes, err := ctrl.appStateManager.CompareAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, &defaultProj, revisions, sources, false, false, nil, false, false) + compRes, err := ctrl.appStateManager.CompareAppState(app, &defaultProj, revisions, sources, false, false, nil, false, false) require.NoError(t, err) assert.NotNil(t, compRes) assert.NotNil(t, compRes.syncStatus) diff --git a/controller/sync.go b/controller/sync.go index 74c5df93ee249..4424ca4c444ee 100644 --- a/controller/sync.go +++ b/controller/sync.go @@ -87,7 +87,7 @@ func (m *appStateManager) getResourceOperations(cluster *v1alpha1.Cluster) (kube return ops, cleanup, nil } -func (m *appStateManager) SyncAppState(destCluster *v1alpha1.Cluster, app *v1alpha1.Application, state *v1alpha1.OperationState) { +func (m *appStateManager) SyncAppState(app *v1alpha1.Application, state *v1alpha1.OperationState) { // Sync requests might be requested with ambiguous revisions (e.g. master, HEAD, v1.2.3). // This can change meaning when resuming operations (e.g a hook sync). After calculating a // concrete git commit SHA, the SHA is remembered in the status.operationState.syncResult field. @@ -194,7 +194,7 @@ func (m *appStateManager) SyncAppState(destCluster *v1alpha1.Cluster, app *v1alp } // ignore error if CompareStateRepoError, this shouldn't happen as noRevisionCache is true - compareResult, err := m.CompareAppState(destCluster, app, proj, revisions, sources, false, true, syncOp.Manifests, isMultiSourceRevision, rollback) + compareResult, err := m.CompareAppState(app, proj, revisions, sources, false, true, syncOp.Manifests, isMultiSourceRevision, rollback) if err != nil && !goerrors.Is(err, CompareStateRepoError) { state.Phase = common.OperationError state.Message = err.Error() @@ -216,6 +216,13 @@ func (m *appStateManager) SyncAppState(destCluster *v1alpha1.Cluster, app *v1alp return } + destCluster, err := argo.GetDestinationCluster(context.Background(), app.Spec.Destination, m.db) + if err != nil { + state.Phase = common.OperationError + state.Message = fmt.Sprintf("Failed to get destination cluster: %v", err) + return + } + rawConfig, err := destCluster.RawRestConfig() if err != nil { state.Phase = common.OperationError diff --git a/controller/sync_test.go b/controller/sync_test.go index 759da11c1f1b5..5560ffc2ec971 100644 --- a/controller/sync_test.go +++ b/controller/sync_test.go @@ -50,7 +50,7 @@ func TestPersistRevisionHistory(t *testing.T) { opState := &v1alpha1.OperationState{Operation: v1alpha1.Operation{ Sync: &v1alpha1.SyncOperation{}, }} - ctrl.appStateManager.SyncAppState(&v1alpha1.Cluster{Server: "https://localhost:6443", Name: "test"}, app, opState) + ctrl.appStateManager.SyncAppState(app, opState) // Ensure we record spec.source into sync result assert.Equal(t, app.Spec.GetSource(), opState.SyncResult.Source) @@ -96,7 +96,7 @@ func TestPersistManagedNamespaceMetadataState(t *testing.T) { opState := &v1alpha1.OperationState{Operation: v1alpha1.Operation{ Sync: &v1alpha1.SyncOperation{}, }} - ctrl.appStateManager.SyncAppState(&v1alpha1.Cluster{Server: "https://localhost:6443", Name: "test"}, app, opState) + ctrl.appStateManager.SyncAppState(app, opState) // Ensure we record spec.syncPolicy.managedNamespaceMetadata into sync result assert.Equal(t, app.Spec.SyncPolicy.ManagedNamespaceMetadata, opState.SyncResult.ManagedNamespaceMetadata) } @@ -139,7 +139,7 @@ func TestPersistRevisionHistoryRollback(t *testing.T) { Source: &source, }, }} - ctrl.appStateManager.SyncAppState(&v1alpha1.Cluster{Server: "https://localhost:6443", Name: "test"}, app, opState) + ctrl.appStateManager.SyncAppState(app, opState) // Ensure we record opState's source into sync result assert.Equal(t, source, opState.SyncResult.Source) @@ -182,7 +182,7 @@ func TestSyncComparisonError(t *testing.T) { Sync: &v1alpha1.SyncOperation{}, }} t.Setenv("ARGOCD_GPG_ENABLED", "true") - ctrl.appStateManager.SyncAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, app, opState) + ctrl.appStateManager.SyncAppState(app, opState) conditions := app.Status.GetConditions(map[v1alpha1.ApplicationConditionType]bool{v1alpha1.ApplicationConditionComparisonError: true}) assert.NotEmpty(t, conditions) @@ -249,7 +249,7 @@ func TestAppStateManager_SyncAppState(t *testing.T) { }} // when - f.controller.appStateManager.SyncAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, f.application, opState) + f.controller.appStateManager.SyncAppState(f.application, opState) // then assert.Equal(t, common.OperationFailed, opState.Phase) @@ -319,7 +319,7 @@ func TestSyncWindowDeniesSync(t *testing.T) { Phase: common.OperationRunning, } // when - f.controller.appStateManager.SyncAppState(&v1alpha1.Cluster{Server: test.FakeClusterURL, Name: "test"}, f.application, opState) + f.controller.appStateManager.SyncAppState(f.application, opState) // then assert.Equal(t, common.OperationRunning, opState.Phase) @@ -1340,7 +1340,7 @@ func TestSyncWithImpersonate(t *testing.T) { Phase: common.OperationRunning, } // when - f.controller.appStateManager.SyncAppState(&v1alpha1.Cluster{Server: "https://localhost:6443", Name: "test"}, f.application, opState) + f.controller.appStateManager.SyncAppState(f.application, opState) // then, app sync should fail with expected error message in operation state assert.Equal(t, common.OperationError, opState.Phase) @@ -1361,7 +1361,7 @@ func TestSyncWithImpersonate(t *testing.T) { Phase: common.OperationRunning, } // when - f.controller.appStateManager.SyncAppState(&v1alpha1.Cluster{Server: "https://localhost:6443", Name: "test"}, f.application, opState) + f.controller.appStateManager.SyncAppState(f.application, opState) // then app sync should fail with expected error message in operation state assert.Equal(t, common.OperationError, opState.Phase) @@ -1382,7 +1382,7 @@ func TestSyncWithImpersonate(t *testing.T) { Phase: common.OperationRunning, } // when - f.controller.appStateManager.SyncAppState(&v1alpha1.Cluster{Server: "https://localhost:6443", Name: "test"}, f.application, opState) + f.controller.appStateManager.SyncAppState(f.application, opState) // then app sync should not fail assert.Equal(t, common.OperationSucceeded, opState.Phase) @@ -1403,7 +1403,7 @@ func TestSyncWithImpersonate(t *testing.T) { Phase: common.OperationRunning, } // when - f.controller.appStateManager.SyncAppState(&v1alpha1.Cluster{Server: "https://localhost:6443", Name: "test"}, f.application, opState) + f.controller.appStateManager.SyncAppState(f.application, opState) // then application sync should pass using the control plane service account assert.Equal(t, common.OperationSucceeded, opState.Phase)