Skip to content

Commit cd965cc

Browse files
kichristensenkichristensen
committed
refactor(cmd): centralize error logging in installation
commands Update all installation command handlers to explicitly log errors at the top level using log.Error(). This ensures errors are logged to console exactly once at the command handler level, preventing duplicate error messages. Changes: - Add tracing package import - Update 9 command handlers (list, show, apply, delete, runs list, install, upgrade, invoke, uninstall) - Add explicit error logging in RunE functions - Keep PreRunE unchanged for validation only This is part of Phase 3 (Command Handler Updates) of the centralized error logging implementation plan. Signed-off-by: Kim Christensen <[email protected]> fixup Signed-off-by: Kim Christensen <[email protected]>
1 parent f300cc2 commit cd965cc

File tree

3 files changed

+74
-18
lines changed

3 files changed

+74
-18
lines changed

‎cmd/porter/installations.go‎

Lines changed: 64 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package main
22

33
import (
44
"get.porter.sh/porter/pkg/porter"
5+
"get.porter.sh/porter/pkg/tracing"
56
"github.com/spf13/cobra"
67
"github.com/spf13/pflag"
78
)
@@ -55,7 +56,13 @@ Optional output formats include json and yaml.`,
5556
return opts.Validate()
5657
},
5758
RunE: func(cmd *cobra.Command, args []string) error {
58-
return p.PrintInstallations(cmd.Context(), opts)
59+
ctx := cmd.Context()
60+
log := tracing.GetLogger(ctx)
61+
62+
if err := p.PrintInstallations(ctx, opts); err != nil {
63+
return log.Error(err)
64+
}
65+
return nil
5966
},
6067
}
6168

@@ -95,7 +102,13 @@ Optional output formats include json and yaml.
95102
return opts.Validate(args, p.Context)
96103
},
97104
RunE: func(cmd *cobra.Command, args []string) error {
98-
return p.ShowInstallation(cmd.Context(), opts)
105+
ctx := cmd.Context()
106+
log := tracing.GetLogger(ctx)
107+
108+
if err := p.ShowInstallation(ctx, opts); err != nil {
109+
return log.Error(err)
110+
}
111+
return nil
99112
},
100113
}
101114

@@ -129,7 +142,13 @@ You can use the show command to create the initial file:
129142
return opts.Validate(p.Context, args)
130143
},
131144
RunE: func(cmd *cobra.Command, args []string) error {
132-
return p.InstallationApply(cmd.Context(), opts)
145+
ctx := cmd.Context()
146+
log := tracing.GetLogger(ctx)
147+
148+
if err := p.InstallationApply(ctx, opts); err != nil {
149+
return log.Error(err)
150+
}
151+
return nil
133152
},
134153
}
135154

@@ -158,7 +177,13 @@ func buildInstallationDeleteCommand(p *porter.Porter) *cobra.Command {
158177
return opts.Validate(args, p.Context)
159178
},
160179
RunE: func(cmd *cobra.Command, args []string) error {
161-
return p.DeleteInstallation(cmd.Context(), opts)
180+
ctx := cmd.Context()
181+
log := tracing.GetLogger(ctx)
182+
183+
if err := p.DeleteInstallation(ctx, opts); err != nil {
184+
return log.Error(err)
185+
}
186+
return nil
162187
},
163188
}
164189

@@ -200,7 +225,13 @@ func buildInstallationRunsListCommand(p *porter.Porter) *cobra.Command {
200225
return opts.Validate(args, p.Context)
201226
},
202227
RunE: func(cmd *cobra.Command, args []string) error {
203-
return p.PrintInstallationRuns(cmd.Context(), opts)
228+
ctx := cmd.Context()
229+
log := tracing.GetLogger(ctx)
230+
231+
if err := p.PrintInstallationRuns(ctx, opts); err != nil {
232+
return log.Error(err)
233+
}
234+
return nil
204235
},
205236
}
206237

@@ -246,7 +277,13 @@ The docker driver runs the bundle container using the local Docker host. To use
246277
return opts.Validate(cmd.Context(), args, p)
247278
},
248279
RunE: func(cmd *cobra.Command, args []string) error {
249-
return p.InstallBundle(cmd.Context(), opts)
280+
ctx := cmd.Context()
281+
log := tracing.GetLogger(ctx)
282+
283+
if err := p.InstallBundle(ctx, opts); err != nil {
284+
return log.Error(err)
285+
}
286+
return nil
250287
},
251288
}
252289

@@ -296,7 +333,13 @@ The docker driver runs the bundle container using the local Docker host. To use
296333
return opts.Validate(cmd.Context(), args, p)
297334
},
298335
RunE: func(cmd *cobra.Command, args []string) error {
299-
return p.UpgradeBundle(cmd.Context(), opts)
336+
ctx := cmd.Context()
337+
log := tracing.GetLogger(ctx)
338+
339+
if err := p.UpgradeBundle(ctx, opts); err != nil {
340+
return log.Error(err)
341+
}
342+
return nil
300343
},
301344
}
302345

@@ -347,7 +390,13 @@ The docker driver runs the bundle container using the local Docker host. To use
347390
return opts.Validate(cmd.Context(), args, p)
348391
},
349392
RunE: func(cmd *cobra.Command, args []string) error {
350-
return p.InvokeBundle(cmd.Context(), opts)
393+
ctx := cmd.Context()
394+
log := tracing.GetLogger(ctx)
395+
396+
if err := p.InvokeBundle(ctx, opts); err != nil {
397+
return log.Error(err)
398+
}
399+
return nil
351400
},
352401
}
353402

@@ -397,7 +446,13 @@ The docker driver runs the bundle container using the local Docker host. To use
397446
return opts.Validate(cmd.Context(), args, p)
398447
},
399448
RunE: func(cmd *cobra.Command, args []string) error {
400-
return p.UninstallBundle(cmd.Context(), opts)
449+
ctx := cmd.Context()
450+
log := tracing.GetLogger(ctx)
451+
452+
if err := p.UninstallBundle(ctx, opts); err != nil {
453+
return log.Error(err)
454+
}
455+
return nil
401456
},
402457
}
403458

‎cmd/porter/main.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,10 @@ func main() {
8282
}()
8383

8484
if err := rootCmd.ExecuteContext(ctx); err != nil {
85-
// Ideally we log all errors in the span that generated it,
86-
// but as a failsafe, always log the error at the root span as well
87-
_ = log.Error(err)
85+
// Record the error to the root span for tracing purposes.
86+
// Command handlers are responsible for logging to console via log.Error(),
87+
// so we only record to span here to avoid duplicate console output.
88+
_ = log.RecordError(err)
8889
return cli.ExitCodeErr
8990
}
9091
return cli.ExitCodeSuccess

‎pkg/porter/install.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,14 +65,14 @@ func (p *Porter) InstallBundle(ctx context.Context, opts InstallOptions) error {
6565
// Validate that we are not overwriting an existing installation
6666
if i.IsInstalled() && !opts.Force {
6767
err = errors.New("The installation has already been successfully installed and as a protection against accidentally overwriting existing installations, porter install cannot be repeated. Verify the installation name and namespace, and if correct, use porter upgrade. You can skip this check by using the --force flag.")
68-
return log.Error(err)
68+
return log.RecordError(err)
6969
}
7070
} else if errors.Is(err, storage.ErrNotFound{}) {
7171
// Create the installation record
7272
i = storage.NewInstallation(opts.Namespace, opts.Name)
7373
} else {
7474
err = fmt.Errorf("could not retrieve the installation record: %w", err)
75-
return log.Error(err)
75+
return log.RecordError(err)
7676
}
7777

7878
// Apply labels that were specified as flags to the installation record
@@ -95,17 +95,17 @@ func (p *Porter) InstallBundle(ctx context.Context, opts InstallOptions) error {
9595
}
9696
log.Debugf("verifying bundle signature for %s", ref.String())
9797
if !ok {
98-
return log.Errorf("unable to get reference for bundle %s: %w", ref.String(), err)
98+
return log.RecordErrorf("unable to get reference for bundle %s: %w", ref.String(), err)
9999
}
100100
err = p.Signer.Verify(ctx, ref.String())
101101
if err != nil {
102-
return log.Errorf("unable to verify signature: %w", err)
102+
return log.RecordErrorf("unable to verify signature: %w", err)
103103
}
104104
log.Debugf("bundle signature verified for %s", ref.String())
105105

106106
bun, err := opts.GetOptions().GetBundleReference(ctx, p)
107107
if err != nil {
108-
return log.Errorf("unable to get bundle reference")
108+
return log.RecordErrorf("unable to get bundle reference")
109109
}
110110

111111
invocationImage := bun.Definition.InvocationImages[0].Image
@@ -115,7 +115,7 @@ func (p *Porter) InstallBundle(ctx context.Context, opts InstallOptions) error {
115115
log.Debugf("verifying bundle image signature for %s", invocationImage)
116116
err = p.Signer.Verify(ctx, invocationImage)
117117
if err != nil {
118-
return log.Errorf("unable to verify signature: %w", err)
118+
return log.RecordErrorf("unable to verify signature: %w", err)
119119
}
120120
log.Debugf("bundle image signature verified for %s", invocationImage)
121121
}

0 commit comments

Comments
 (0)