Skip to content

Commit e3fcfd5

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 62d9da1 commit e3fcfd5

File tree

8 files changed

+109
-66
lines changed

8 files changed

+109
-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: 23 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

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

‎cmd/driver/install/install.go

Lines changed: 68 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package driverinstall
1717

1818
import (
19+
"bytes"
1920
"crypto/tls"
2021
"errors"
2122
"fmt"
@@ -25,6 +26,7 @@ import (
2526
"time"
2627

2728
"github.com/blang/semver"
29+
"github.com/pterm/pterm"
2830
"github.com/spf13/cobra"
2931
"golang.org/x/net/context"
3032

@@ -172,30 +174,88 @@ func (o *driverInstallOptions) RunDriverInstall(ctx context.Context, driver *con
172174
}
173175
o.Printer.Logger.Info("found distro", o.Printer.Logger.Args("target", d))
174176

175-
err = driver.Type.Cleanup(o.Printer, driver.Name)
177+
var (
178+
dest string
179+
buf bytes.Buffer
180+
)
181+
182+
if !o.Printer.DisableStyling {
183+
o.Printer.Spinner, _ = o.Printer.Spinner.Start("Cleaning up existing drivers")
184+
}
185+
err = driver.Type.Cleanup(o.Printer.WithWriter(&buf), driver.Name)
186+
if o.Printer.Spinner != nil {
187+
_ = o.Printer.Spinner.Stop()
188+
}
189+
if o.Printer.Logger.Formatter == pterm.LogFormatterJSON {
190+
// Only print formatted text if we are formatting to json
191+
out := strings.ReplaceAll(buf.String(), "\n", ";")
192+
o.Printer.Logger.Info("Driver build", o.Printer.Logger.Args("output", out))
193+
} else {
194+
// Print much more readable output as-is
195+
o.Printer.DefaultText.Print(buf.String())
196+
}
176197
if err != nil {
177198
return "", err
178199
}
179200

180-
setDefaultHTTPClientOpts(o.driverDownloadOptions)
181-
182-
var dest string
183201
if o.Download {
184-
dest, err = driverdistro.Download(ctx, d, o.Printer, kr, driver.Name, driver.Type, driver.Version, driver.Repos)
202+
setDefaultHTTPClientOpts(o.driverDownloadOptions)
203+
if !o.Printer.DisableStyling {
204+
o.Printer.Spinner, _ = o.Printer.Spinner.Start("Trying to download the driver")
205+
}
206+
dest, err = driverdistro.Download(ctx, d, o.Printer.WithWriter(&buf), kr, driver.Name, driver.Type, driver.Version, driver.Repos)
207+
if o.Printer.Spinner != nil {
208+
_ = o.Printer.Spinner.Stop()
209+
}
210+
if o.Printer.Logger.Formatter == pterm.LogFormatterJSON {
211+
// Only print formatted text if we are formatting to json
212+
out := strings.ReplaceAll(buf.String(), "\n", ";")
213+
o.Printer.Logger.Info("Driver build", o.Printer.Logger.Args("output", out))
214+
} else {
215+
// Print much more readable output as-is
216+
o.Printer.DefaultText.Print(buf.String())
217+
}
218+
buf.Reset()
185219
if err == nil {
220+
o.Printer.Logger.Info("Driver downloaded.", o.Printer.Logger.Args("path", dest))
221+
return dest, nil
222+
}
223+
if errors.Is(err, driverdistro.ErrAlreadyPresent) {
224+
o.Printer.Logger.Info("Skipping download, driver already present.", o.Printer.Logger.Args("path", dest))
186225
return dest, nil
187226
}
188227
// Print the error but go on
189228
// attempting a build if requested
190-
o.Printer.Logger.Warn(err.Error())
229+
if o.Compile {
230+
o.Printer.Logger.Warn(err.Error())
231+
}
191232
}
192233

193234
if o.Compile {
194-
dest, err = driverdistro.Build(ctx, d, o.Printer, kr, driver.Name, driver.Type, driver.Version)
235+
if !o.Printer.DisableStyling {
236+
o.Printer.Spinner, _ = o.Printer.Spinner.Start("Trying to build the driver")
237+
}
238+
dest, err = driverdistro.Build(ctx, d, o.Printer.WithWriter(&buf), kr, driver.Name, driver.Type, driver.Version)
239+
if o.Printer.Spinner != nil {
240+
_ = o.Printer.Spinner.Stop()
241+
}
242+
if o.Printer.Logger.Formatter == pterm.LogFormatterJSON {
243+
// Only print formatted text if we are formatting to json
244+
out := strings.ReplaceAll(buf.String(), "\n", ";")
245+
o.Printer.Logger.Info("Driver build", o.Printer.Logger.Args("output", out))
246+
} else {
247+
// Print much more readable output as-is
248+
o.Printer.DefaultText.Print(buf.String())
249+
}
250+
buf.Reset()
195251
if err == nil {
252+
o.Printer.Logger.Info("Driver built.", o.Printer.Logger.Args("path", dest))
253+
return dest, nil
254+
}
255+
if errors.Is(err, driverdistro.ErrAlreadyPresent) {
256+
o.Printer.Logger.Info("Skipping build, driver already present.", o.Printer.Logger.Args("path", dest))
196257
return dest, nil
197258
}
198-
o.Printer.Logger.Warn(err.Error())
199259
}
200260

201261
return 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)