Skip to content

Commit

Permalink
be less radical
Browse files Browse the repository at this point in the history
Signed-off-by: Michael Crenshaw <[email protected]>
  • Loading branch information
crenshaw-dev committed Jan 3, 2025
1 parent 5cc575e commit 1807a45
Show file tree
Hide file tree
Showing 8 changed files with 79 additions and 70 deletions.
2 changes: 1 addition & 1 deletion cmd/argocd/commands/admin/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
11 changes: 5 additions & 6 deletions controller/appcontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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")

Expand Down
4 changes: 2 additions & 2 deletions controller/hook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
}
Expand Down
9 changes: 1 addition & 8 deletions controller/hydrator_dependencies.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand Down
22 changes: 16 additions & 6 deletions controller/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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)

Expand All @@ -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()
Expand Down
Loading

0 comments on commit 1807a45

Please sign in to comment.