Skip to content

Commit 44e1233

Browse files
FedeDPFedeDP
committed
new(cmd,pkg/driver): properly use a spinner for long operations.
Signed-off-by: Federico Di Pierro <[email protected]>
1 parent c19ca32 commit 44e1233

File tree

8 files changed

+108
-66
lines changed

8 files changed

+108
-66
lines changed

‎cmd/artifact/install/install.go

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import (
2222
"path/filepath"
2323
"runtime"
2424

25-
"github.com/pterm/pterm"
2625
"github.com/spf13/cobra"
2726
"github.com/spf13/viper"
2827

@@ -182,8 +181,6 @@ Examples:
182181

183182
// RunArtifactInstall executes the business logic for the artifact install command.
184183
func (o *artifactInstallOptions) RunArtifactInstall(ctx context.Context, args []string) error {
185-
var sp *pterm.SpinnerPrinter
186-
187184
logger := o.Printer.Logger
188185
// Retrieve configuration for installer
189186
configuredInstaller, err := config.Installer()
@@ -317,7 +314,7 @@ func (o *artifactInstallOptions) RunArtifactInstall(ctx context.Context, args []
317314
logger.Info("Extracting and installing artifact", logger.Args("type", result.Type, "file", result.Filename))
318315

319316
if !o.Printer.DisableStyling {
320-
sp, _ = o.Printer.Spinner.Start("Extracting and installing")
317+
o.Printer.Spinner, _ = o.Printer.Spinner.Start("Extracting and installing")
321318
}
322319

323320
result.Filename = filepath.Join(tmpDir, result.Filename)
@@ -337,8 +334,8 @@ func (o *artifactInstallOptions) RunArtifactInstall(ctx context.Context, args []
337334
return err
338335
}
339336

340-
if sp != nil {
341-
_ = sp.Stop()
337+
if o.Printer.Spinner != nil {
338+
_ = o.Printer.Spinner.Stop()
342339
}
343340
logger.Info("Artifact successfully installed", logger.Args("name", ref, "type", result.Type, "digest", result.Digest, "directory", destDir))
344341
}

‎cmd/driver/cleanup/cleanup.go

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@
1616
package drivercleanup
1717

1818
import (
19+
"bytes"
20+
"strings"
21+
22+
"github.com/pterm/pterm"
1923
"github.com/spf13/cobra"
2024
"golang.org/x/net/context"
2125

@@ -51,5 +55,21 @@ func (o *driverCleanupOptions) RunDriverCleanup(_ context.Context) error {
5155
o.Printer.Logger.Info("Running falcoctl driver cleanup", o.Printer.Logger.Args(
5256
"driver type", o.Driver.Type,
5357
"driver name", o.Driver.Name))
54-
return o.Driver.Type.Cleanup(o.Printer, o.Driver.Name)
58+
var buf bytes.Buffer
59+
if !o.Printer.DisableStyling {
60+
o.Printer.Spinner, _ = o.Printer.Spinner.Start("Cleaning up existing drivers")
61+
}
62+
err := o.Driver.Type.Cleanup(o.Printer.WithWriter(&buf), o.Driver.Name)
63+
if o.Printer.Spinner != nil {
64+
_ = o.Printer.Spinner.Stop()
65+
}
66+
if o.Printer.Logger.Formatter == pterm.LogFormatterJSON {
67+
// Only print formatted text if we are formatting to json
68+
out := strings.ReplaceAll(buf.String(), "\n", ";")
69+
o.Printer.Logger.Info("Driver build", o.Printer.Logger.Args("output", out))
70+
} else {
71+
// Print much more readable output as-is
72+
o.Printer.DefaultText.Print(buf.String())
73+
}
74+
return err
5575
}

‎cmd/driver/install/install.go

Lines changed: 69 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,15 @@
1616
package driverinstall
1717

1818
import (
19+
"bytes"
1920
"crypto/tls"
2021
"errors"
2122
"fmt"
2223
"net/http"
24+
"strings"
2325
"time"
2426

27+
"github.com/pterm/pterm"
2528
"github.com/spf13/cobra"
2629
"golang.org/x/net/context"
2730

@@ -140,30 +143,88 @@ func (o *driverInstallOptions) RunDriverInstall(ctx context.Context) (string, er
140143
}
141144
o.Printer.Logger.Info("found distro", o.Printer.Logger.Args("target", d))
142145

143-
err = o.Driver.Type.Cleanup(o.Printer, o.Driver.Name)
146+
var (
147+
dest string
148+
buf bytes.Buffer
149+
)
150+
151+
if !o.Printer.DisableStyling {
152+
o.Printer.Spinner, _ = o.Printer.Spinner.Start("Cleaning up existing drivers")
153+
}
154+
err = o.Driver.Type.Cleanup(o.Printer.WithWriter(&buf), o.Driver.Name)
155+
if o.Printer.Spinner != nil {
156+
_ = o.Printer.Spinner.Stop()
157+
}
158+
if o.Printer.Logger.Formatter == pterm.LogFormatterJSON {
159+
// Only print formatted text if we are formatting to json
160+
out := strings.ReplaceAll(buf.String(), "\n", ";")
161+
o.Printer.Logger.Info("Driver build", o.Printer.Logger.Args("output", out))
162+
} else {
163+
// Print much more readable output as-is
164+
o.Printer.DefaultText.Print(buf.String())
165+
}
144166
if err != nil {
145167
return "", err
146168
}
147169

148-
setDefaultHTTPClientOpts(o.driverDownloadOptions)
149-
150-
var dest string
151170
if o.Download {
152-
dest, err = driverdistro.Download(ctx, d, o.Printer, kr, o.Driver.Name, o.Driver.Type, o.Driver.Version, o.Driver.Repos)
171+
setDefaultHTTPClientOpts(o.driverDownloadOptions)
172+
if !o.Printer.DisableStyling {
173+
o.Printer.Spinner, _ = o.Printer.Spinner.Start("Trying to download the driver")
174+
}
175+
dest, err = driverdistro.Download(ctx, d, o.Printer.WithWriter(&buf), kr, o.Driver.Name, o.Driver.Type, o.Driver.Version, o.Driver.Repos)
176+
if o.Printer.Spinner != nil {
177+
_ = o.Printer.Spinner.Stop()
178+
}
179+
if o.Printer.Logger.Formatter == pterm.LogFormatterJSON {
180+
// Only print formatted text if we are formatting to json
181+
out := strings.ReplaceAll(buf.String(), "\n", ";")
182+
o.Printer.Logger.Info("Driver build", o.Printer.Logger.Args("output", out))
183+
} else {
184+
// Print much more readable output as-is
185+
o.Printer.DefaultText.Print(buf.String())
186+
}
187+
buf.Reset()
153188
if err == nil {
189+
o.Printer.Logger.Info("Driver downloaded.", o.Printer.Logger.Args("path", dest))
190+
return dest, nil
191+
}
192+
if errors.Is(err, driverdistro.ErrAlreadyPresent) {
193+
o.Printer.Logger.Info("Skipping download, driver already present.", o.Printer.Logger.Args("path", dest))
154194
return dest, nil
155195
}
156196
// Print the error but go on
157197
// attempting a build if requested
158-
o.Printer.Logger.Warn(err.Error())
198+
if o.Compile {
199+
o.Printer.Logger.Warn(err.Error())
200+
}
159201
}
160202

161203
if o.Compile {
162-
dest, err = driverdistro.Build(ctx, d, o.Printer, kr, o.Driver.Name, o.Driver.Type, o.Driver.Version)
204+
if !o.Printer.DisableStyling {
205+
o.Printer.Spinner, _ = o.Printer.Spinner.Start("Trying to build the driver")
206+
}
207+
dest, err = driverdistro.Build(ctx, d, o.Printer.WithWriter(&buf), kr, o.Driver.Name, o.Driver.Type, o.Driver.Version)
208+
if o.Printer.Spinner != nil {
209+
_ = o.Printer.Spinner.Stop()
210+
}
211+
if o.Printer.Logger.Formatter == pterm.LogFormatterJSON {
212+
// Only print formatted text if we are formatting to json
213+
out := strings.ReplaceAll(buf.String(), "\n", ";")
214+
o.Printer.Logger.Info("Driver build", o.Printer.Logger.Args("output", out))
215+
} else {
216+
// Print much more readable output as-is
217+
o.Printer.DefaultText.Print(buf.String())
218+
}
219+
buf.Reset()
163220
if err == nil {
221+
o.Printer.Logger.Info("Driver built.", o.Printer.Logger.Args("path", dest))
222+
return dest, nil
223+
}
224+
if errors.Is(err, driverdistro.ErrAlreadyPresent) {
225+
o.Printer.Logger.Info("Skipping build, driver already present.", o.Printer.Logger.Args("path", dest))
164226
return dest, nil
165227
}
166-
o.Printer.Logger.Warn(err.Error())
167228
}
168229

169230
return o.Driver.Name, fmt.Errorf("failed: %w", err)

‎main.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ func main() {
4040
if opt.Printer != nil && opt.Printer.ProgressBar != nil && opt.Printer.ProgressBar.IsActive {
4141
_, _ = opt.Printer.ProgressBar.Stop()
4242
}
43+
if opt.Printer != nil && opt.Printer.Spinner != nil && opt.Printer.Spinner.IsActive {
44+
_ = opt.Printer.Spinner.Stop()
45+
}
4346
opt.Printer.Logger.Info("Received signal, terminating...")
4447
stop()
4548
}()

‎pkg/driver/distro/distro.go

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ var (
4949
hostRoot = string(os.PathSeparator)
5050
// ErrUnsupported is the error returned when the target distro is not supported.
5151
ErrUnsupported = errors.New("failed to determine distro")
52+
// ErrAlreadyPresent is the error returned when a driver is already present on filesystem.
53+
ErrAlreadyPresent = errors.New("driver already present")
5254
)
5355

5456
// Distro is the common interface used by distro-specific implementations.
@@ -169,8 +171,7 @@ func Build(ctx context.Context,
169171
driverFileName := toFilename(d, &kr, driverName, driverType)
170172
destination := toLocalPath(driverVer, driverFileName, kr.Architecture.ToNonDeb())
171173
if exist, _ := utils.FileExists(destination); exist {
172-
printer.Logger.Info("Skipping build, driver already present.", printer.Logger.Args("path", destination))
173-
return destination, nil
174+
return destination, ErrAlreadyPresent
174175
}
175176

176177
env, err := d.customizeBuild(ctx, printer, driverType, kr)
@@ -189,11 +190,7 @@ func Build(ctx context.Context,
189190
if err != nil {
190191
return "", err
191192
}
192-
err = copyDataToLocalPath(destination, f)
193-
if err == nil {
194-
printer.Logger.Info("Driver built.", printer.Logger.Args("path", destination))
195-
}
196-
return destination, err
193+
return destination, copyDataToLocalPath(destination, f)
197194
}
198195

199196
// Download will try to download drivers for a distro trying specified repos.
@@ -211,8 +208,7 @@ func Download(ctx context.Context,
211208
// Skip if existent
212209
destination := toLocalPath(driverVer, driverFileName, kr.Architecture.ToNonDeb())
213210
if exist, _ := utils.FileExists(destination); exist {
214-
printer.Logger.Info("Skipping download, driver already present.", printer.Logger.Args("path", destination))
215-
return destination, nil
211+
return destination, ErrAlreadyPresent
216212
}
217213

218214
// Try to download from any specified repository,
@@ -236,11 +232,7 @@ func Download(ctx context.Context,
236232
}
237233
continue
238234
}
239-
err = copyDataToLocalPath(destination, resp.Body)
240-
if err == nil {
241-
printer.Logger.Info("Driver downloaded.", printer.Logger.Args("path", destination))
242-
}
243-
return destination, err
235+
return destination, copyDataToLocalPath(destination, resp.Body)
244236
}
245237
return destination, fmt.Errorf("unable to find a prebuilt driver")
246238
}

‎pkg/driver/type/bpf.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,6 @@ func (b *bpf) Build(ctx context.Context,
7878
driverName, driverVersion string,
7979
env map[string]string,
8080
) (string, error) {
81-
printer.Logger.Info("Trying to compile the eBPF probe")
82-
8381
srcPath := fmt.Sprintf("/usr/src/%s-%s/bpf", driverName, driverVersion)
8482

8583
makeCmdArgs := fmt.Sprintf(`make -C %q`, filepath.Clean(srcPath))
@@ -88,12 +86,10 @@ func (b *bpf) Build(ctx context.Context,
8886
for key, val := range env {
8987
makeCmd.Env = append(makeCmd.Env, fmt.Sprintf("%s=%s", key, val))
9088
}
91-
err := runCmdPipingStdout(printer, makeCmd)
92-
outProbe := fmt.Sprintf("%s/probe.o", srcPath)
93-
if err == nil {
94-
printer.Logger.Info("Probe successfully built.", printer.Logger.Args("file", outProbe))
95-
} else {
96-
printer.Logger.Info("Failed to build probe.", printer.Logger.Args("err", err))
89+
out, err := makeCmd.CombinedOutput()
90+
if err != nil {
91+
printer.DefaultText.Print(string(out))
9792
}
93+
outProbe := fmt.Sprintf("%s/probe.o", srcPath)
9894
return outProbe, err
9995
}

‎pkg/driver/type/kmod.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -192,8 +192,6 @@ func (k *kmod) Build(ctx context.Context,
192192
return "", fmt.Errorf("unsupported on uek hosts")
193193
}
194194

195-
printer.Logger.Info("Trying to compile the kernel module")
196-
197195
out, err := exec.Command("which", "gcc").Output()
198196
if err != nil {
199197
return "", err
@@ -224,8 +222,7 @@ func (k *kmod) Build(ctx context.Context,
224222
driverName, driverVersion, kr.String())
225223

226224
// Try the build through dkms
227-
dkmsCmd := exec.CommandContext(ctx, "bash", "-c", dkmsCmdArgs) //nolint:gosec // false positive
228-
err = runCmdPipingStdout(printer, dkmsCmd)
225+
out, err = exec.CommandContext(ctx, "bash", "-c", dkmsCmdArgs).CombinedOutput() //nolint:gosec // false positive
229226
if err == nil {
230227
koGlob := fmt.Sprintf("/var/lib/dkms/%s/%s/%s/%s/module/%s", driverName, driverVersion, kr.String(), kr.Architecture.ToNonDeb(), driverName)
231228
var koFiles []string
@@ -238,6 +235,7 @@ func (k *kmod) Build(ctx context.Context,
238235
printer.Logger.Info("Module installed in dkms.", printer.Logger.Args("file", koFile))
239236
return koFile, nil
240237
}
238+
printer.DefaultText.Print(string(out))
241239
dkmsLogFile := fmt.Sprintf("/var/lib/dkms/$%s/%s/build/make.log", driverName, driverVersion)
242240
logs, err := os.ReadFile(filepath.Clean(dkmsLogFile))
243241
if err != nil {

‎pkg/driver/type/type.go

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,7 @@
1717
package drivertype
1818

1919
import (
20-
"bufio"
2120
"fmt"
22-
"os/exec"
2321

2422
"github.com/falcosecurity/driverkit/pkg/kernelrelease"
2523
"golang.org/x/net/context"
@@ -59,26 +57,3 @@ func Parse(driverType string) (DriverType, error) {
5957
}
6058
return nil, fmt.Errorf("wrong driver type specified: %s", driverType)
6159
}
62-
63-
func runCmdPipingStdout(printer *output.Printer, cmd *exec.Cmd) error {
64-
stdout, err := cmd.StdoutPipe()
65-
if err != nil {
66-
printer.Logger.Warn("Failed to pipe output. Trying without piping.", printer.Logger.Args("err", err))
67-
_, err = cmd.Output()
68-
} else {
69-
defer stdout.Close()
70-
err = cmd.Start()
71-
if err != nil {
72-
printer.Logger.Warn("Failed to execute command.", printer.Logger.Args("err", err))
73-
} else {
74-
// print the output of the subprocess line by line
75-
scanner := bufio.NewScanner(stdout)
76-
for scanner.Scan() {
77-
m := scanner.Text()
78-
printer.DefaultText.Println(m)
79-
}
80-
err = cmd.Wait()
81-
}
82-
}
83-
return err
84-
}

0 commit comments

Comments
 (0)