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: give context to errors #15019

Merged
merged 4 commits into from
Aug 14, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
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
2 changes: 1 addition & 1 deletion applicationset/generators/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func (g *ClusterGenerator) GenerateParams(appSetGenerator *argoappsetv1alpha1.Ap
// ListCluster from Argo CD's util/db package will include the local cluster in the list of clusters
clustersFromArgoCD, err := utils.ListClusters(g.ctx, g.clientset, g.namespace)
if err != nil {
return nil, err
return nil, fmt.Errorf("error listing clusters: %w", err)
}

if clustersFromArgoCD == nil {
Expand Down
8 changes: 4 additions & 4 deletions applicationset/generators/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func (g *GitGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.Applic
return nil, EmptyAppSetGeneratorError
}
if err != nil {
return nil, err
return nil, fmt.Errorf("error generating params from git: %w", err)
}

return res, nil
Expand All @@ -77,7 +77,7 @@ func (g *GitGenerator) generateParamsForGitDirectories(appSetGenerator *argoproj
// Directories, not files
allPaths, err := g.repos.GetDirectories(context.TODO(), appSetGenerator.Git.RepoURL, appSetGenerator.Git.Revision)
if err != nil {
return nil, err
return nil, fmt.Errorf("error getting directories from repo: %w", err)
}

log.WithFields(log.Fields{
Expand All @@ -92,7 +92,7 @@ func (g *GitGenerator) generateParamsForGitDirectories(appSetGenerator *argoproj

res, err := g.generateParamsFromApps(requestedApps, appSetGenerator, useGoTemplate, goTemplateOptions)
if err != nil {
return nil, fmt.Errorf("failed to generate params from apps: %w", err)
return nil, fmt.Errorf("error generating params from apps: %w", err)
}

return res, nil
Expand Down Expand Up @@ -177,7 +177,7 @@ func (g *GitGenerator) generateParamsFromGitFile(filePath string, fileContent []
} else {
flat, err := flatten.Flatten(objectFound, "", flatten.DotStyle)
if err != nil {
return nil, err
return nil, fmt.Errorf("error flattening object: %w", err)
}
for k, v := range flat {
params[k] = fmt.Sprintf("%v", v)
Expand Down
14 changes: 7 additions & 7 deletions applicationset/generators/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (m *MergeGenerator) getParamSetsForAllGenerators(generators []argoprojiov1a
for _, generator := range generators {
generatorParamSets, err := m.getParams(generator, appSet)
if err != nil {
return nil, err
return nil, fmt.Errorf("error getting params from generator: %w", err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about getting just slightly fancier?

Suggested change
return nil, fmt.Errorf("error getting params from generator: %w", err)
return nil, fmt.Errorf("error getting params from generator %d of %d: %w", i+1, len(generators), err)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense, its definitely better to give more context

Copy link
Contributor Author

@ashinsabu3 ashinsabu3 Aug 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming the index variable should also be changed to i

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep!

}
// concatenate param lists produced by each generator
paramSets = append(paramSets, generatorParamSets)
Expand All @@ -61,32 +61,32 @@ func (m *MergeGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.Appl

paramSetsFromGenerators, err := m.getParamSetsForAllGenerators(appSetGenerator.Merge.Generators, appSet)
if err != nil {
return nil, err
return nil, fmt.Errorf("error getting param sets from generators: %w", err)
}

baseParamSetsByMergeKey, err := getParamSetsByMergeKey(appSetGenerator.Merge.MergeKeys, paramSetsFromGenerators[0])
if err != nil {
return nil, err
return nil, fmt.Errorf("error getting param sets by merge key: %w", err)
}

for _, paramSets := range paramSetsFromGenerators[1:] {
paramSetsByMergeKey, err := getParamSetsByMergeKey(appSetGenerator.Merge.MergeKeys, paramSets)
if err != nil {
return nil, err
return nil, fmt.Errorf("error getting param sets by merge key: %w", err)
}

for mergeKeyValue, baseParamSet := range baseParamSetsByMergeKey {
if overrideParamSet, exists := paramSetsByMergeKey[mergeKeyValue]; exists {

if appSet.Spec.GoTemplate {
if err := mergo.Merge(&baseParamSet, overrideParamSet, mergo.WithOverride); err != nil {
return nil, fmt.Errorf("failed to merge base param set with override param set: %w", err)
return nil, fmt.Errorf("error merging base param set with override param set: %w", err)
}
baseParamSetsByMergeKey[mergeKeyValue] = baseParamSet
} else {
overriddenParamSet, err := utils.CombineStringMapsAllowDuplicates(baseParamSet, overrideParamSet)
if err != nil {
return nil, err
return nil, fmt.Errorf("error combining string maps: %w", err)
}
baseParamSetsByMergeKey[mergeKeyValue] = utils.ConvertToMapStringInterface(overriddenParamSet)
}
Expand Down Expand Up @@ -125,7 +125,7 @@ func getParamSetsByMergeKey(mergeKeys []string, paramSets []map[string]interface
}
paramSetKeyJson, err := json.Marshal(paramSetKey)
if err != nil {
return nil, err
return nil, fmt.Errorf("error marshalling param set key json: %w", err)
}
paramSetKeyString := string(paramSetKeyJson)
if _, exists := paramSetsByMergeKey[paramSetKeyString]; exists {
Expand Down
4 changes: 2 additions & 2 deletions applicationset/generators/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func (g *PluginGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.App

pluginClient, err := g.getPluginFromGenerator(ctx, applicationSetInfo.Name, providerConfig)
if err != nil {
return nil, err
return nil, fmt.Errorf("error getting plugin from generator: %w", err)
}

list, err := pluginClient.List(ctx, providerConfig.Input.Parameters)
Expand Down Expand Up @@ -108,7 +108,7 @@ func (g *PluginGenerator) getPluginFromGenerator(ctx context.Context, appSetName

pluginClient, err := plugin.NewPluginService(ctx, appSetName, cm["baseUrl"], token, requestTimeout)
if err != nil {
return nil, err
return nil, fmt.Errorf("error initializing plugin client: %w", err)
}
return pluginClient, nil
}
Expand Down
32 changes: 16 additions & 16 deletions reposerver/repository/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -1061,7 +1061,7 @@ func runHelmBuild(appPath string, h helm.Helm) error {

err = h.DependencyBuild()
if err != nil {
return err
return fmt.Errorf("error building helm chart dependencies: %w", err)
}
return os.WriteFile(markerFile, []byte("marker"), 0644)
}
Expand Down Expand Up @@ -1102,15 +1102,15 @@ func helmTemplate(appPath string, repoRoot string, env *v1alpha1.Env, q *apiclie

resolvedValueFiles, err := getResolvedValueFiles(appPath, repoRoot, env, q.GetValuesFileSchemes(), appHelm.ValueFiles, q.RefSources, gitRepoPaths, appHelm.IgnoreMissingValueFiles)
if err != nil {
return nil, err
return nil, fmt.Errorf("error resolving helm value files: %w", err)
}

templateOpts.Values = resolvedValueFiles

if !appHelm.ValuesIsEmpty() {
rand, err := uuid.NewRandom()
if err != nil {
return nil, err
return nil, fmt.Errorf("error generating random unique id: %w", err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return nil, fmt.Errorf("error generating random unique id: %w", err)
return nil, fmt.Errorf("error generating random filename for Helm values file: %w", err)

}
p := path.Join(os.TempDir(), rand.String())
defer func() {
Expand All @@ -1121,7 +1121,7 @@ func helmTemplate(appPath string, repoRoot string, env *v1alpha1.Env, q *apiclie
}()
err = os.WriteFile(p, appHelm.ValuesYAML(), 0644)
if err != nil {
return nil, err
return nil, fmt.Errorf("error writing helm values file: %w", err)
}
templateOpts.Values = append(templateOpts.Values, pathutil.ResolvedFilePath(p))
}
Expand All @@ -1136,7 +1136,7 @@ func helmTemplate(appPath string, repoRoot string, env *v1alpha1.Env, q *apiclie
for _, p := range appHelm.FileParameters {
resolvedPath, _, err := pathutil.ResolveValueFilePathOrUrl(appPath, repoRoot, env.Envsubst(p.Path), q.GetValuesFileSchemes())
if err != nil {
return nil, err
return nil, fmt.Errorf("error resolving helm value file path: %w", err)
}
templateOpts.SetFile[p.Name] = resolvedPath
}
Expand All @@ -1160,17 +1160,17 @@ func helmTemplate(appPath string, repoRoot string, env *v1alpha1.Env, q *apiclie

helmRepos, err := getHelmRepos(appPath, q.Repos, q.HelmRepoCreds)
if err != nil {
return nil, err
return nil, fmt.Errorf("error getting helm repos: %w", err)
}
h, err := helm.NewHelmApp(appPath, helmRepos, isLocal, version, proxy, passCredentials)
if err != nil {
return nil, err
return nil, fmt.Errorf("error initializing helm app object: %w", err)
}

defer h.Dispose()
err = h.Init()
if err != nil {
return nil, err
return nil, fmt.Errorf("error initializing helm app: %w", err)
}

out, err := h.Template(templateOpts)
Expand Down Expand Up @@ -1218,13 +1218,13 @@ func getResolvedValueFiles(
// If the $-prefixed path appears to reference another source, do env substitution _after_ resolving that source.
resolvedPath, err = getResolvedRefValueFile(rawValueFile, env, allowedValueFilesSchemas, referencedSource.Repo.Repo, gitRepoPaths)
if err != nil {
return nil, err
return nil, fmt.Errorf("error getting value file: %w", err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return nil, fmt.Errorf("error getting value file: %w", err)
return nil, fmt.Errorf("error resolving value file path: %w", err)

}
} else {
// This will resolve val to an absolute path (or an URL)
resolvedPath, isRemote, err = pathutil.ResolveValueFilePathOrUrl(appPath, repoRoot, env.Envsubst(rawValueFile), allowedValueFilesSchemas)
if err != nil {
return nil, err
return nil, fmt.Errorf("error resolving value file path: %w", err)
}
}

Expand Down Expand Up @@ -1261,7 +1261,7 @@ func getResolvedRefValueFile(
// Resolve the path relative to the referenced repo and block any attempt at traversal.
resolvedPath, _, err := pathutil.ResolveValueFilePathOrUrl(repoPath, repoPath, env.Envsubst(substitutedPath), allowedValueFilesSchemas)
if err != nil {
return "", err
return "", fmt.Errorf("error resolving value file path: %w", err)
}
return resolvedPath, nil
}
Expand Down Expand Up @@ -1325,7 +1325,7 @@ func GenerateManifests(ctx context.Context, appPath, repoRoot, revision string,

appSourceType, err := GetAppSourceType(ctx, q.ApplicationSource, appPath, repoRoot, q.AppName, q.EnabledSourceTypes, opt.cmpTarExcludedGlobs)
if err != nil {
return nil, err
return nil, fmt.Errorf("error getting app source type: %w", err)
}
repoURL := ""
if q.Repo != nil {
Expand Down Expand Up @@ -1505,7 +1505,7 @@ func GetAppSourceType(ctx context.Context, source *v1alpha1.ApplicationSource, a
}
appType, err := discovery.AppType(ctx, appPath, repoPath, enableGenerateManifests, tarExcludedGlobs)
if err != nil {
return "", err
return "", fmt.Errorf("error getting application type string: %v", err)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return "", fmt.Errorf("error getting application type string: %v", err)
return "", fmt.Errorf("error getting app source type: %v", err)

}
return v1alpha1.ApplicationSourceType(appType), nil
}
Expand Down Expand Up @@ -2047,7 +2047,7 @@ func loadFileIntoIfExists(path pathutil.ResolvedFilePath, destination *string) e
if err == nil && !info.IsDir() {
bytes, err := os.ReadFile(stringPath)
if err != nil {
return err
return fmt.Errorf("error reading file from %s: %w", stringPath, err)
}
*destination = string(bytes)
}
Expand All @@ -2060,7 +2060,7 @@ func findHelmValueFilesInPath(path string) ([]string, error) {

files, err := os.ReadDir(path)
if err != nil {
return result, err
return result, fmt.Errorf("error reading helm values file from %s: %w", path, err)
}

for _, f := range files {
Expand Down Expand Up @@ -2189,7 +2189,7 @@ func (s *Service) GetRevisionMetadata(ctx context.Context, q *apiclient.RepoServ
})

if err != nil {
return nil, err
return nil, fmt.Errorf("error acquiring repo lock: %w", err)
}

defer io.Close(closer)
Expand Down
Loading