Skip to content

Commit 335e040

Browse files
gssbzngssbzn
committed
task: logs improvements
- Improve logs help text - extract the common downloader methods to a new struct that can be embeded - cleanup failed log file if there's an error
1 parent 5498df1 commit 335e040

File tree

7 files changed

+177
-149
lines changed

7 files changed

+177
-149
lines changed

‎internal/cli/atlas/logs_download.go

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,16 @@
1515
package atlas
1616

1717
import (
18+
"fmt"
1819
"io"
1920
"os"
2021

21-
"github.com/mongodb/mongocli/internal/cli"
22-
2322
atlas "github.com/mongodb/go-client-mongodb-atlas/mongodbatlas"
23+
"github.com/mongodb/mongocli/internal/cli"
2424
"github.com/mongodb/mongocli/internal/config"
2525
"github.com/mongodb/mongocli/internal/description"
2626
"github.com/mongodb/mongocli/internal/flag"
27+
"github.com/mongodb/mongocli/internal/search"
2728
"github.com/mongodb/mongocli/internal/store"
2829
"github.com/mongodb/mongocli/internal/usage"
2930
"github.com/spf13/afero"
@@ -52,13 +53,13 @@ func (opts *LogsDownloadOpts) Run() error {
5253
if err != nil {
5354
return err
5455
}
55-
defer f.Close()
5656

5757
r := opts.newDateRangeOpts()
5858
if err := opts.store.DownloadLog(opts.ConfigProjectID(), opts.host, opts.name, f, r); err != nil {
59+
_ = opts.handleError(f)
5960
return err
6061
}
61-
return nil
62+
return f.Close()
6263
}
6364

6465
func (opts *LogsDownloadOpts) output() string {
@@ -68,6 +69,11 @@ func (opts *LogsDownloadOpts) output() string {
6869
return opts.out
6970
}
7071

72+
func (opts *LogsDownloadOpts) handleError(f io.Closer) error {
73+
_ = f.Close()
74+
return opts.fs.Remove(opts.output())
75+
}
76+
7177
func (opts *LogsDownloadOpts) newWriteCloser() (io.WriteCloser, error) {
7278
// Create file only if is not there already (don't overwrite)
7379
ff := os.O_CREATE | os.O_TRUNC | os.O_WRONLY | os.O_EXCL
@@ -82,22 +88,27 @@ func (opts *LogsDownloadOpts) newDateRangeOpts() *atlas.DateRangetOptions {
8288
}
8389
}
8490

91+
var logNames = []string{"mongodb.gz", "mongos.gz", "mongodb-audit-log.gz", "mongos-audit-log.gz"}
92+
8593
// mongocli atlas logs download <hostname> <logname> [--type type] [--output destination] [--projectId projectId]
8694
func LogsDownloadBuilder() *cobra.Command {
8795
opts := &LogsDownloadOpts{
8896
fs: afero.NewOsFs(),
8997
}
9098
cmd := &cobra.Command{
9199
Use: "download <hostname> <logname>",
92-
Short: description.ListDisks,
100+
Short: description.LogsDownload,
101+
Long: description.LogsDownloadLong,
93102
Args: cobra.ExactArgs(2),
94103
PreRunE: func(cmd *cobra.Command, args []string) error {
95104
return opts.PreRunE(opts.initStore)
96105
},
97106
RunE: func(cmd *cobra.Command, args []string) error {
98107
opts.host = args[0]
99108
opts.name = args[1]
100-
109+
if !search.StringInSlice(logNames, opts.name) {
110+
return fmt.Errorf("<logname> must be one of %s", logNames)
111+
}
101112
return opts.Run()
102113
},
103114
}

‎internal/cli/downloader_opts.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// Copyright 2020 MongoDB Inc
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package cli
16+
17+
import (
18+
"io"
19+
"os"
20+
21+
"github.com/spf13/afero"
22+
)
23+
24+
// DownloaderOpts options required when deleting a resource.
25+
// A command can compose this struct and then safely rely on the methods Prompt, or Delete
26+
// to manage the interactions with the user
27+
type DownloaderOpts struct {
28+
Out string
29+
Fs afero.Fs
30+
}
31+
32+
func (opts *DownloaderOpts) NewWriteCloser() (io.WriteCloser, error) {
33+
// Create file only if is not there already (don't overwrite)
34+
ff := os.O_CREATE | os.O_TRUNC | os.O_WRONLY | os.O_EXCL
35+
f, err := opts.Fs.OpenFile(opts.Out, ff, 0777)
36+
return f, err
37+
}
38+
39+
func (opts *DownloaderOpts) OnError(f io.Closer) error {
40+
_ = f.Close()
41+
return opts.Fs.Remove(opts.Out)
42+
}

‎internal/cli/opsmanager/diagnose_archive_download.go

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,7 @@
1515
package opsmanager
1616

1717
import (
18-
"io"
19-
"os"
20-
2118
"github.com/mongodb/mongocli/internal/cli"
22-
2319
"github.com/mongodb/mongocli/internal/config"
2420
"github.com/mongodb/mongocli/internal/description"
2521
"github.com/mongodb/mongocli/internal/flag"
@@ -32,10 +28,9 @@ import (
3228

3329
type DiagnoseArchiveDownloadOpts struct {
3430
cli.GlobalOpts
35-
out string
31+
cli.DownloaderOpts
3632
limit int64
3733
minutes int64
38-
fs afero.Fs
3934
store store.ArchivesDownloader
4035
}
4136

@@ -46,24 +41,17 @@ func (opts *DiagnoseArchiveDownloadOpts) initStore() error {
4641
}
4742

4843
func (opts *DiagnoseArchiveDownloadOpts) Run() error {
49-
out, err := opts.newWriteCloser()
44+
out, err := opts.NewWriteCloser()
5045
if err != nil {
5146
return err
5247
}
53-
defer out.Close()
5448

5549
if err := opts.store.DownloadArchive(opts.ConfigProjectID(), opts.newDiagnosticsListOpts(), out); err != nil {
50+
_ = opts.OnError(out)
5651
return err
5752
}
5853

59-
return nil
60-
}
61-
62-
func (opts *DiagnoseArchiveDownloadOpts) newWriteCloser() (io.WriteCloser, error) {
63-
// Create file only if is not there already (don't overwrite)
64-
ff := os.O_CREATE | os.O_TRUNC | os.O_WRONLY | os.O_EXCL
65-
f, err := opts.fs.OpenFile(opts.out, ff, 0777)
66-
return f, err
54+
return out.Close()
6755
}
6856

6957
func (opts *DiagnoseArchiveDownloadOpts) newDiagnosticsListOpts() *opsmngr.DiagnosticsListOpts {
@@ -75,9 +63,8 @@ func (opts *DiagnoseArchiveDownloadOpts) newDiagnosticsListOpts() *opsmngr.Diagn
7563

7664
// mongocli om diagnose-archive download [--out out] [--projectId projectId]
7765
func DiagnoseArchiveDownloadBuilder() *cobra.Command {
78-
opts := &DiagnoseArchiveDownloadOpts{
79-
fs: afero.NewOsFs(),
80-
}
66+
opts := &DiagnoseArchiveDownloadOpts{}
67+
opts.Fs = afero.NewOsFs()
8168
cmd := &cobra.Command{
8269
Use: "download",
8370
Aliases: []string{"get"},
@@ -90,7 +77,7 @@ func DiagnoseArchiveDownloadBuilder() *cobra.Command {
9077
},
9178
}
9279

93-
cmd.Flags().StringVarP(&opts.out, flag.Out, flag.OutShort, "diagnose-archive.tar.gz", usage.DiagnoseOut)
80+
cmd.Flags().StringVarP(&opts.Out, flag.Out, flag.OutShort, "diagnose-archive.tar.gz", usage.DiagnoseOut)
9481
cmd.Flags().Int64Var(&opts.limit, flag.Limit, 0, usage.ArchiveLimit)
9582
cmd.Flags().Int64Var(&opts.minutes, flag.Minutes, 0, usage.ArchiveMinutes)
9683

‎internal/cli/opsmanager/diagnose_archive_download_test.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,12 @@ func TestDiagnoseArchiveDownloadOpts_Run(t *testing.T) {
2626
ctrl := gomock.NewController(t)
2727
mockStore := mocks.NewMockArchivesDownloader(ctrl)
2828

29-
appFS := afero.NewMemMapFs()
30-
3129
opts := &DiagnoseArchiveDownloadOpts{
32-
fs: appFS,
3330
store: mockStore,
3431
}
32+
opts.Fs = afero.NewMemMapFs()
3533

36-
f, err := opts.newWriteCloser()
34+
f, err := opts.NewWriteCloser()
3735
if err != nil {
3836
t.Fatalf("newWriteCloser() unexpected error: %v", err)
3937
}

‎internal/cli/opsmanager/logs_jobs_download.go

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,7 @@
1515
package opsmanager
1616

1717
import (
18-
"io"
19-
"os"
20-
2118
"github.com/mongodb/mongocli/internal/cli"
22-
2319
"github.com/mongodb/mongocli/internal/config"
2420
"github.com/mongodb/mongocli/internal/description"
2521
"github.com/mongodb/mongocli/internal/flag"
@@ -31,9 +27,8 @@ import (
3127

3228
type LogsJobsDownloadOpts struct {
3329
cli.GlobalOpts
30+
cli.DownloaderOpts
3431
id string
35-
out string
36-
fs afero.Fs
3732
store store.LogJobsDownloader
3833
}
3934

@@ -44,34 +39,26 @@ func (opts *LogsJobsDownloadOpts) initStore() error {
4439
}
4540

4641
func (opts *LogsJobsDownloadOpts) Run() error {
47-
out, err := opts.newWriteCloser()
42+
out, err := opts.NewWriteCloser()
4843
if err != nil {
4944
return err
5045
}
51-
defer out.Close()
5246

5347
if err := opts.store.DownloadLogJob(opts.ConfigProjectID(), opts.id, out); err != nil {
48+
_ = opts.OnError(out)
5449
return err
5550
}
5651

57-
return nil
58-
}
59-
60-
func (opts *LogsJobsDownloadOpts) newWriteCloser() (io.WriteCloser, error) {
61-
// Create file only if is not there already (don't overwrite)
62-
ff := os.O_CREATE | os.O_TRUNC | os.O_WRONLY | os.O_EXCL
63-
f, err := opts.fs.OpenFile(opts.out, ff, 0777)
64-
return f, err
52+
return out.Close()
6553
}
6654

6755
// mongocli om logs jobs download <ID> [--out out] [--projectId projectId]
6856
func LogsJobsDownloadOptsBuilder() *cobra.Command {
69-
opts := &LogsJobsDownloadOpts{
70-
fs: afero.NewOsFs(),
71-
}
57+
opts := &LogsJobsDownloadOpts{}
58+
opts.Fs = afero.NewOsFs()
7259
cmd := &cobra.Command{
7360
Use: "download <ID>",
74-
Short: description.DownloadLogs,
61+
Short: description.DownloadLogCollectionJob,
7562
Args: cobra.ExactArgs(1),
7663
PreRunE: func(cmd *cobra.Command, args []string) error {
7764
return opts.PreRunE(opts.initStore)
@@ -82,7 +69,7 @@ func LogsJobsDownloadOptsBuilder() *cobra.Command {
8269
},
8370
}
8471

85-
cmd.Flags().StringVarP(&opts.out, flag.Out, flag.OutShort, "", usage.LogOut)
72+
cmd.Flags().StringVarP(&opts.Out, flag.Out, flag.OutShort, "", usage.LogOut)
8673

8774
cmd.Flags().StringVar(&opts.ProjectID, flag.ProjectID, "", usage.ProjectID)
8875

‎internal/cli/opsmanager/logs_jobs_download_test.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,13 @@ func TestLogsDownloadOpts_Run(t *testing.T) {
2626
ctrl := gomock.NewController(t)
2727
mockStore := mocks.NewMockLogJobsDownloader(ctrl)
2828

29-
appFS := afero.NewMemMapFs()
30-
3129
opts := &LogsJobsDownloadOpts{
3230
id: "1",
33-
fs: appFS,
3431
store: mockStore,
3532
}
33+
opts.Fs = afero.NewMemMapFs()
34+
f, err := opts.NewWriteCloser()
3635

37-
f, err := opts.newWriteCloser()
3836
if err != nil {
3937
t.Fatalf("newWriteCloser() unexpected error: %v", err)
4038
}

0 commit comments

Comments
 (0)