Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

chore: wrap ComparisonError messages #14886

Merged
merged 2 commits into from
Aug 3, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions controller/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -620,7 +620,7 @@ func (c *liveStateCache) GetNamespaceTopLevelResources(server string, namespace
func (c *liveStateCache) GetManagedLiveObjs(a *appv1.Application, targetObjs []*unstructured.Unstructured) (map[kube.ResourceKey]*unstructured.Unstructured, error) {
clusterInfo, err := c.getSyncedCluster(a.Spec.Destination.Server)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to get cluster info for %q: %w", a.Spec.Destination.Server, err)
}
return clusterInfo.GetManagedLiveObjs(targetObjs, func(r *clustercache.Resource) bool {
return resInfo(r).AppName == a.InstanceName(c.settingsMgr.GetNamespace())
Expand All @@ -630,7 +630,7 @@ func (c *liveStateCache) GetManagedLiveObjs(a *appv1.Application, targetObjs []*
func (c *liveStateCache) GetVersionsInfo(serverURL string) (string, []kube.APIResourceInfo, error) {
clusterInfo, err := c.getSyncedCluster(serverURL)
if err != nil {
return "", nil, err
return "", nil, fmt.Errorf("failed to get cluster info for %q: %w", serverURL, err)
}
return clusterInfo.GetServerVersion(), clusterInfo.GetAPIResources(), nil
}
Expand Down
50 changes: 30 additions & 20 deletions controller/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,47 +111,47 @@ func (m *appStateManager) getRepoObjs(app *v1alpha1.Application, sources []v1alp
ts := stats.NewTimingStats()
helmRepos, err := m.db.ListHelmRepositories(context.Background())
if err != nil {
return nil, nil, err
return nil, nil, fmt.Errorf("failed to list Helm repositories: %w", err)
}
permittedHelmRepos, err := argo.GetPermittedRepos(proj, helmRepos)
if err != nil {
return nil, nil, err
return nil, nil, fmt.Errorf("failed to get permitted Helm repositories for project %q: %w", proj.Name, err)
}

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

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

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

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

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

Expand All @@ -171,11 +171,11 @@ func (m *appStateManager) getRepoObjs(app *v1alpha1.Application, sources []v1alp
ts.AddCheckpoint("helm_ms")
repo, err := m.db.GetRepository(context.Background(), source.RepoURL)
if err != nil {
return nil, nil, err
return nil, nil, fmt.Errorf("failed to get repo %q: %w", source.RepoURL, err)
}
kustomizeOptions, err := kustomizeSettings.GetOptions(source)
if err != nil {
return nil, nil, err
return nil, nil, fmt.Errorf("failed to get Kustomize options for source %d of %d: %w", i+1, len(sources), err)
}

ts.AddCheckpoint("version_ms")
Expand Down Expand Up @@ -204,13 +204,13 @@ func (m *appStateManager) getRepoObjs(app *v1alpha1.Application, sources []v1alp
ProjectSourceRepos: proj.Spec.SourceRepos,
})
if err != nil {
return nil, nil, err
return nil, nil, fmt.Errorf("failed to generate manifest for source %d of %d: %w", i+1, len(sources), err)
}

targetObj, err := unmarshalManifests(manifestInfo.Manifests)

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

Expand Down Expand Up @@ -400,7 +400,8 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
targetObjs, manifestInfos, err = m.getRepoObjs(app, sources, appLabelKey, revisions, noCache, noRevisionCache, verifySignature, project)
if err != nil {
targetObjs = make([]*unstructured.Unstructured, 0)
conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: err.Error(), LastTransitionTime: &now})
msg := fmt.Sprintf("Failed to load target state: %s", err.Error())
conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: msg, LastTransitionTime: &now})
failedToLoadObjs = true
}
} else {
Expand All @@ -415,7 +416,8 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
targetObjs, err = unmarshalManifests(localManifests)
if err != nil {
targetObjs = make([]*unstructured.Unstructured, 0)
conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: err.Error(), LastTransitionTime: &now})
msg := fmt.Sprintf("Failed to load local manifests: %s", err.Error())
conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: msg, LastTransitionTime: &now})
failedToLoadObjs = true
}
}
Expand All @@ -431,7 +433,8 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
}
targetObjs, dedupConditions, err := DeduplicateTargetObjects(app.Spec.Destination.Namespace, targetObjs, infoProvider)
if err != nil {
conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: err.Error(), LastTransitionTime: &now})
msg := fmt.Sprintf("Failed to deduplicate target state: %s", err.Error())
conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: msg, LastTransitionTime: &now})
}
conditions = append(conditions, dedupConditions...)
for i := len(targetObjs) - 1; i >= 0; i-- {
Expand All @@ -451,7 +454,8 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
liveObjByKey, err := m.liveStateCache.GetManagedLiveObjs(app, targetObjs)
if err != nil {
liveObjByKey = make(map[kubeutil.ResourceKey]*unstructured.Unstructured)
conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: err.Error(), LastTransitionTime: &now})
msg := fmt.Sprintf("Failed to load live state: %s", err.Error())
conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: msg, LastTransitionTime: &now})
failedToLoadObjs = true
}

Expand All @@ -460,11 +464,16 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
// filter out all resources which are not permitted in the application project
for k, v := range liveObjByKey {
permitted, err := project.IsLiveResourcePermitted(v, app.Spec.Destination.Server, app.Spec.Destination.Name, func(project string) ([]*v1alpha1.Cluster, error) {
return m.db.GetProjectClusters(context.TODO(), project)
clusters, err := m.db.GetProjectClusters(context.TODO(), project)
if err != nil {
return nil, fmt.Errorf("failed to get clusters for project %q: %v", project, err)
}
return clusters, nil
})

if err != nil {
conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: err.Error(), LastTransitionTime: &now})
msg := fmt.Sprintf("Failed to check if live resource %q is permitted in project %q: %s", k.String(), app.Spec.Project, err.Error())
conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: msg, LastTransitionTime: &now})
failedToLoadObjs = true
continue
}
Expand Down Expand Up @@ -541,7 +550,8 @@ func (m *appStateManager) CompareAppState(app *v1alpha1.Application, project *v1
if err != nil {
diffResults = &diff.DiffResultList{}
failedToLoadObjs = true
conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: err.Error(), LastTransitionTime: &now})
msg := fmt.Sprintf("Failed to compare desired state to live state: %s", err.Error())
conditions = append(conditions, v1alpha1.ApplicationCondition{Type: v1alpha1.ApplicationConditionComparisonError, Message: msg, LastTransitionTime: &now})
}
ts.AddCheckpoint("diff_ms")

Expand Down
3 changes: 2 additions & 1 deletion reposerver/apiclient/clientset.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package apiclient
import (
"crypto/tls"
"crypto/x509"
"fmt"
"time"

grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware"
Expand Down Expand Up @@ -48,7 +49,7 @@ type clientSet struct {
func (c *clientSet) NewRepoServerClient() (io.Closer, RepoServerServiceClient, error) {
conn, err := NewConnection(c.address, c.timeoutSeconds, &c.tlsConfig)
if err != nil {
return nil, nil, err
return nil, nil, fmt.Errorf("failed to open a new connection to repo server: %w", err)
}
return conn, NewRepoServerServiceClient(conn), nil
}
Expand Down
19 changes: 14 additions & 5 deletions util/argo/diff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ import (

"github.com/go-logr/logr"

k8smanagedfields "k8s.io/apimachinery/pkg/util/managedfields"

"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
"github.com/argoproj/argo-cd/v2/util/argo"
"github.com/argoproj/argo-cd/v2/util/argo/managedfields"
appstatecache "github.com/argoproj/argo-cd/v2/util/cache/appstate"
k8smanagedfields "k8s.io/apimachinery/pkg/util/managedfields"

"github.com/argoproj/gitops-engine/pkg/diff"
"github.com/argoproj/gitops-engine/pkg/utils/kube"
Expand Down Expand Up @@ -239,12 +240,12 @@ func StateDiff(live, config *unstructured.Unstructured, diffConfig DiffConfig) (
func StateDiffs(lives, configs []*unstructured.Unstructured, diffConfig DiffConfig) (*diff.DiffResultList, error) {
normResults, err := preDiffNormalize(lives, configs, diffConfig)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to perform pre-diff normalization: %w", err)
}

diffNormalizer, err := newDiffNormalizer(diffConfig.Ignores(), diffConfig.Overrides())
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to create diff normalizer: %w", err)
}

diffOpts := []diff.Option{
Expand All @@ -261,9 +262,17 @@ func StateDiffs(lives, configs []*unstructured.Unstructured, diffConfig DiffConf

useCache, cachedDiff := diffConfig.DiffFromCache(diffConfig.AppName())
if useCache && cachedDiff != nil {
return diffArrayCached(normResults.Targets, normResults.Lives, cachedDiff, diffOpts...)
cached, err := diffArrayCached(normResults.Targets, normResults.Lives, cachedDiff, diffOpts...)
if err != nil {
return nil, fmt.Errorf("failed to calculate diff from cache: %w", err)
}
return cached, nil
}
array, err := diff.DiffArray(normResults.Targets, normResults.Lives, diffOpts...)
if err != nil {
return nil, fmt.Errorf("failed to calculate diff: %w", err)
}
return diff.DiffArray(normResults.Targets, normResults.Lives, diffOpts...)
return array, nil
}

func diffArrayCached(configArray []*unstructured.Unstructured, liveArray []*unstructured.Unstructured, cachedDiff []*v1alpha1.ResourceDiff, opts ...diff.Option) (*diff.DiffResultList, error) {
Expand Down
9 changes: 5 additions & 4 deletions util/db/helmrepository.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package db

import (
"context"
"fmt"
"strings"

"google.golang.org/grpc/codes"
Expand Down Expand Up @@ -43,24 +44,24 @@ func (db *db) getHelmRepo(repoURL string, helmRepositories []settings.HelmRepoCr
return repo, err
}

// ListHelmRepoURLs lists configured helm repositories
// ListHelmRepositories lists configured helm repositories
func (db *db) ListHelmRepositories(ctx context.Context) ([]*v1alpha1.Repository, error) {
helmRepositories, err := db.settingsMgr.GetHelmRepositories()
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to get list of Helm repositories from settings manager: %w", err)
}

result := make([]*v1alpha1.Repository, len(helmRepositories))
for i, helmRepoInfo := range helmRepositories {
repo, err := db.getHelmRepo(helmRepoInfo.URL, helmRepositories)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to get Helm repository %q: %w", helmRepoInfo.URL, err)
}
result[i] = repo
}
repos, err := db.listRepositories(ctx, pointer.StringPtr("helm"))
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to list Helm repositories: %w", err)
}
result = append(result, v1alpha1.Repositories(repos).Filter(func(r *v1alpha1.Repository) bool {
return r.Type == "helm" && r.Name != ""
Expand Down
42 changes: 29 additions & 13 deletions util/db/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,11 @@ func (db *db) CreateRepository(ctx context.Context, r *appsv1.Repository) (*apps
func (db *db) GetRepository(ctx context.Context, repoURL string) (*appsv1.Repository, error) {
repository, err := db.getRepository(ctx, repoURL)
if err != nil {
return repository, err
return repository, fmt.Errorf("unable to get repository %q: %v", repoURL, err)
}

if err := db.enrichCredsToRepo(ctx, repository); err != nil {
return repository, err
return repository, fmt.Errorf("unable to enrich repository %q info with credentials: %v", repoURL, err)
}

return repository, err
Expand Down Expand Up @@ -123,17 +123,25 @@ func (db *db) getRepository(ctx context.Context, repoURL string) (*appsv1.Reposi
secretsBackend := db.repoBackend()
exists, err := secretsBackend.RepositoryExists(ctx, repoURL)
if err != nil {
return nil, err
return nil, fmt.Errorf("unable to check if repository %q exists from secrets backend: %v", repoURL, err)
} else if exists {
return secretsBackend.GetRepository(ctx, repoURL)
repository, err := secretsBackend.GetRepository(ctx, repoURL)
if err != nil {
return nil, fmt.Errorf("unable to get repository %q from secrets backend: %v", repoURL, err)
}
return repository, nil
}

legacyBackend := db.legacyRepoBackend()
exists, err = legacyBackend.RepositoryExists(ctx, repoURL)
if err != nil {
return nil, err
return nil, fmt.Errorf("unable to check if repository %q exists from legacy backend: %v", repoURL, err)
} else if exists {
return legacyBackend.GetRepository(ctx, repoURL)
repository, err := legacyBackend.GetRepository(ctx, repoURL)
if err != nil {
return nil, fmt.Errorf("unable to get repository %q from legacy backend: %v", repoURL, err)
}
return repository, nil
}

return &appsv1.Repository{Repo: repoURL}, nil
Expand Down Expand Up @@ -229,17 +237,25 @@ func (db *db) GetRepositoryCredentials(ctx context.Context, repoURL string) (*ap
secretsBackend := db.repoBackend()
exists, err := secretsBackend.RepoCredsExists(ctx, repoURL)
if err != nil {
return nil, err
return nil, fmt.Errorf("unable to check if repository credentials for %q exists from secrets backend: %w", repoURL, err)
} else if exists {
return secretsBackend.GetRepoCreds(ctx, repoURL)
creds, err := secretsBackend.GetRepoCreds(ctx, repoURL)
if err != nil {
return nil, fmt.Errorf("unable to get repository credentials for %q from secrets backend: %w", repoURL, err)
}
return creds, nil
}

legacyBackend := db.legacyRepoBackend()
exists, err = legacyBackend.RepoCredsExists(ctx, repoURL)
if err != nil {
return nil, err
return nil, fmt.Errorf("unable to check if repository credentials for %q exists from legacy backend: %w", repoURL, err)
} else if exists {
return legacyBackend.GetRepoCreds(ctx, repoURL)
creds, err := legacyBackend.GetRepoCreds(ctx, repoURL)
if err != nil {
return nil, fmt.Errorf("unable to get repository credentials for %q from legacy backend: %w", repoURL, err)
}
return creds, nil
}

return nil, nil
Expand All @@ -252,12 +268,12 @@ func (db *db) GetAllHelmRepositoryCredentials(ctx context.Context) ([]*appsv1.Re

secretRepoCreds, err := db.repoBackend().GetAllHelmRepoCreds(ctx)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to get all Helm repo creds: %w", err)
}

legacyRepoCreds, err := db.legacyRepoBackend().GetAllHelmRepoCreds(ctx)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed to get all legacy Helm repo creds: %w", err)
}

return append(secretRepoCreds, legacyRepoCreds...), nil
Expand Down Expand Up @@ -353,7 +369,7 @@ func (db *db) enrichCredsToRepo(ctx context.Context, repository *appsv1.Reposito
repository.InheritedCreds = true
}
} else {
return err
return fmt.Errorf("failed to get repository credentials for %q: %w", repository.Repo, err)
}
} else {
log.Debugf("%s has credentials", repository.Repo)
Expand Down
Loading