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

CLOUDP-237254: Update remaining commands to append deployment type telemetry #2803

Merged
merged 19 commits into from Mar 26, 2024
8 changes: 8 additions & 0 deletions internal/cli/deployments/connect.go
Expand Up @@ -28,6 +28,11 @@ func Run(ctx context.Context, opts *options.ConnectOpts) error {
return opts.Connect(ctx)
}

func PostRun(opts *options.ConnectOpts) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not

Suggested change
func PostRun(opts *options.ConnectOpts) error {
func (opts *options.ConnectOpts) PostRun() error {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can't define it there in connect.go and I found too weird to define it in the opts file, so I followed same run convention, I tried this approach before but didn't like it so kept this way

Copy link
Collaborator

Choose a reason for hiding this comment

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

it seems to me from a distance we may be falling into the same traps that made the setup command unmaintainable with complicated "inheritance" that's making commands hard to modify

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think what we can do here, is to move everything from connect_opts_atlas.go and connect_opts_local.go into connect.go then, would that make sense? if so, I can do that in a next PR to avoid growing this one, I can start the draft now

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's open a ticket and discuss as a team if worth the effort given we plan to re write a lot of this logic

Copy link
Contributor Author

Choose a reason for hiding this comment

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

opts.DeploymentTelemetry.AppendDeploymentType()
return nil
}

// atlas deployments connect [clusterName].
func ConnectBuilder() *cobra.Command {
opts := &options.ConnectOpts{}
Expand All @@ -53,6 +58,9 @@ func ConnectBuilder() *cobra.Command {
RunE: func(cmd *cobra.Command, _ []string) error {
return Run(cmd.Context(), opts)
},
PostRunE: func(_ *cobra.Command, _ []string) error {
return PostRun(opts)
},
}

cmd.Flags().StringVar(&opts.ConnectWith, flag.ConnectWith, "", usage.ConnectWith)
Expand Down
88 changes: 33 additions & 55 deletions internal/cli/deployments/connect_test.go
Expand Up @@ -24,12 +24,11 @@ import (
"github.com/golang/mock/gomock"
"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/cli"
"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/cli/deployments/options"
"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/config"
"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/cli/deployments/test/fixture"
"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/flag"
"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/mocks"
"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/podman"
"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/pointer"
"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/store"
"github.com/mongodb/mongodb-atlas-cli/atlascli/internal/test"
"github.com/stretchr/testify/assert"
"go.mongodb.org/atlas-sdk/v20231115008/admin"
Expand All @@ -43,43 +42,20 @@ const (
func TestRun_ConnectLocal(t *testing.T) {
ctrl := gomock.NewController(t)
ctx := context.Background()
mockPodman := mocks.NewMockClient(ctrl)
buf := new(bytes.Buffer)

deploymenTest := fixture.NewMockLocalDeploymentOpts(ctrl, expectedLocalDeployment)
connectOpts := &options.ConnectOpts{
ConnectWith: "connectionString",
DeploymentOpts: options.DeploymentOpts{
PodmanClient: mockPodman,
DeploymentName: expectedLocalDeployment,
DeploymentType: "local",
},
ConnectWith: "connectionString",
DeploymentOpts: *deploymenTest.Opts,
OutputOpts: cli.OutputOpts{
OutWriter: buf,
},
}

expectedContainers := []*podman.Container{
{
Names: []string{expectedLocalDeployment},
State: "running",
Labels: map[string]string{"version": "6.0.9"},
ID: expectedLocalDeployment,
},
}

mockPodman.
EXPECT().
Ready(ctx).
Return(nil).
Times(1)
deploymenTest.LocalMockFlow(ctx)

mockPodman.
EXPECT().
ListContainers(ctx, options.MongodHostnamePrefix).
Return(expectedContainers, nil).
Times(1)

mockPodman.
deploymenTest.MockPodman.
EXPECT().
ContainerInspect(ctx, options.MongodHostnamePrefix+"-"+expectedLocalDeployment).
Return([]*podman.InspectContainerData{
Expand Down Expand Up @@ -122,18 +98,12 @@ func TestRun_ConnectAtlas(t *testing.T) {
ctx := context.Background()
buf := new(bytes.Buffer)

mockAtlasClusterListStore := mocks.NewMockClusterLister(ctrl)
mockCredentialsGetter := mocks.NewMockCredentialsGetter(ctrl)
mockAtlasClusterDescriber := mocks.NewMockClusterDescriber(ctrl)
deploymenTest := fixture.NewMockAtlasDeploymentOpts(ctrl, expectedAtlasDeployment)

connectOpts := &options.ConnectOpts{
ConnectWith: "connectionString",
DeploymentOpts: options.DeploymentOpts{
AtlasClusterListStore: mockAtlasClusterListStore,
DeploymentName: expectedAtlasDeployment,
DeploymentType: "atlas",
CredStore: mockCredentialsGetter,
},
ConnectWith: "connectionString",
DeploymentOpts: *deploymenTest.Opts,
ConnectToAtlasOpts: options.ConnectToAtlasOpts{
Store: mockAtlasClusterDescriber,
GlobalOpts: cli.GlobalOpts{
Expand Down Expand Up @@ -161,29 +131,14 @@ func TestRun_ConnectAtlas(t *testing.T) {
},
}

mockAtlasClusterListStore.
EXPECT().
ProjectClusters(connectOpts.ProjectID,
&store.ListOptions{
PageNum: cli.DefaultPage,
ItemsPerPage: options.MaxItemsPerPage,
},
).
Return(expectedAtlasClusters, nil).
Times(1)
deploymenTest.CommonAtlasMocks(connectOpts.ProjectID)

mockAtlasClusterDescriber.
EXPECT().
AtlasCluster(connectOpts.ProjectID, expectedAtlasDeployment).
Return(&expectedAtlasClusters.GetResults()[0], nil).
Times(1)

mockCredentialsGetter.
EXPECT().
AuthType().
Return(config.OAuth).
Times(1)

if err := Run(ctx, connectOpts); err != nil {
t.Fatalf("Run() unexpected error: %v", err)
}
Expand All @@ -192,6 +147,29 @@ func TestRun_ConnectAtlas(t *testing.T) {
`, buf.String())
}

func TestRun_PostRun(t *testing.T) {
blva marked this conversation as resolved.
Show resolved Hide resolved
ctrl := gomock.NewController(t)
deploymentsTest := fixture.NewMockLocalDeploymentOpts(ctrl, "localDeployment")
buf := new(bytes.Buffer)

opts := &options.ConnectOpts{
DeploymentOpts: *deploymentsTest.Opts,
OutputOpts: cli.OutputOpts{
OutWriter: buf,
},
}

deploymentsTest.
MockDeploymentTelemetry.
EXPECT().
AppendDeploymentType().
Times(1)

if err := PostRun(opts); err != nil {
t.Fatalf("PostRun() unexpected error: %v", err)
}
}

func TestConnectBuilder(t *testing.T) {
test.CmdValidator(
t,
Expand Down
8 changes: 8 additions & 0 deletions internal/cli/deployments/logs.go
Expand Up @@ -181,6 +181,11 @@ func (opts *DownloadOpts) validateAtlasFlags() error {
return nil
}

func (opts *DownloadOpts) PostRun() error {
opts.DeploymentTelemetry.AppendDeploymentType()
return nil
}
blva marked this conversation as resolved.
Show resolved Hide resolved

// atlas deployments logs.
func LogsBuilder() *cobra.Command {
opts := &DownloadOpts{
Expand All @@ -204,6 +209,9 @@ func LogsBuilder() *cobra.Command {
RunE: func(cmd *cobra.Command, _ []string) error {
return opts.Run(cmd.Context())
},
PostRunE: func(_ *cobra.Command, _ []string) error {
return opts.PostRun()
},
blva marked this conversation as resolved.
Show resolved Hide resolved
}

cmd.Flags().StringVar(&opts.DeploymentType, flag.TypeFlag, "", usage.DeploymentType)
Expand Down
23 changes: 23 additions & 0 deletions internal/cli/deployments/logs_test.go
Expand Up @@ -112,3 +112,26 @@ func TestLogs_RunAtlas(t *testing.T) {
t.Fatalf("Run() unexpected error: %v", err)
}
}

func TestLogs_PostRun(t *testing.T) {
blva marked this conversation as resolved.
Show resolved Hide resolved
ctrl := gomock.NewController(t)
deploymentTest := fixture.NewMockLocalDeploymentOpts(ctrl, "localDeployment")
buf := new(bytes.Buffer)

opts := &DownloadOpts{
DeploymentOpts: *deploymentTest.Opts,
OutputOpts: cli.OutputOpts{
OutWriter: buf,
},
}

deploymentTest.
MockDeploymentTelemetry.
EXPECT().
AppendDeploymentType().
Times(1)

if err := opts.PostRun(); err != nil {
t.Fatalf("PostRun() unexpected error: %v", err)
}
}
3 changes: 3 additions & 0 deletions internal/cli/deployments/options/deployment_opts.go
Expand Up @@ -261,5 +261,8 @@ func (opts *DeploymentOpts) IsAuthEnabled() bool {
}

func (opts *DeploymentOpts) UpdateDeploymentTelemetry() {
if opts.DeploymentTelemetry == nil {
opts.DeploymentTelemetry = NewDeploymentTypeTelemetry(opts)
}
opts.DeploymentTelemetry.AppendDeploymentType()
}
4 changes: 2 additions & 2 deletions internal/cli/deployments/options/deployment_opts_select.go
Expand Up @@ -104,7 +104,7 @@ func (opts *DeploymentOpts) Select(deployments []Deployment) (Deployment, error)
opts.DeploymentName = deployments[0].Name
opts.DeploymentType = strings.ToLower(deployments[0].Type)

opts.AppendDeploymentType()
opts.UpdateDeploymentTelemetry()
return deployments[0], nil
}

Expand All @@ -131,6 +131,6 @@ func (opts *DeploymentOpts) Select(deployments []Deployment) (Deployment, error)
deployment := deploymentsByDisplayName[displayName]
opts.DeploymentName = deployment.Name
opts.DeploymentType = strings.ToLower(deployment.Type)
opts.AppendDeploymentType()
opts.UpdateDeploymentTelemetry()
return deployment, nil
}
8 changes: 6 additions & 2 deletions internal/cli/deployments/pause.go
Expand Up @@ -122,6 +122,11 @@ func (opts *PauseOpts) StopMongoD(ctx context.Context, names string) error {
return opts.PodmanClient.Exec(ctx, "-d", names, "mongod", "--shutdown")
}

func (opts *PauseOpts) PostRun() error {
opts.DeploymentTelemetry.AppendDeploymentType()
return opts.PostRunMessages()
}

func PauseBuilder() *cobra.Command {
opts := &PauseOpts{}
cmd := &cobra.Command{
Expand Down Expand Up @@ -149,9 +154,8 @@ func PauseBuilder() *cobra.Command {
}
return opts.Run(cmd.Context())
},

PostRunE: func(_ *cobra.Command, _ []string) error {
return opts.PostRunMessages()
return opts.PostRun()
},
}

Expand Down
22 changes: 22 additions & 0 deletions internal/cli/deployments/pause_test.go
Expand Up @@ -121,6 +121,28 @@ func TestPause_RunAtlas(t *testing.T) {
t.Log(buf.String())
}

func Test_PostRun(t *testing.T) {
blva marked this conversation as resolved.
Show resolved Hide resolved
ctrl := gomock.NewController(t)
deploymentTest := fixture.NewMockLocalDeploymentOpts(ctrl, deploymentName)
buf := new(bytes.Buffer)

opts := &PauseOpts{
DeploymentOpts: *deploymentTest.Opts,
OutputOpts: cli.OutputOpts{
OutWriter: buf,
},
}

deploymentTest.MockDeploymentTelemetry.
EXPECT().
AppendDeploymentType().
Times(1)

if err := opts.PostRun(); err != nil {
t.Fatalf("PostRun() unexpected error: %v", err)
}
}

func TestPauseBuilder(t *testing.T) {
test.CmdValidator(
t,
Expand Down
1 change: 1 addition & 0 deletions internal/cli/deployments/search/indexes/create.go
Expand Up @@ -172,6 +172,7 @@ func (opts *CreateOpts) watch(ctx context.Context) (any, bool, error) {
}

func (opts *CreateOpts) PostRun(ctx context.Context) error {
opts.AppendDeploymentType()
if !opts.EnableWatch {
return opts.Print(opts.index)
}
Expand Down
1 change: 0 additions & 1 deletion internal/cli/deployments/search/indexes/create_test.go
Expand Up @@ -159,7 +159,6 @@ func TestCreate_RunLocal(t *testing.T) {
if err := opts.Run(ctx); err != nil {
t.Fatalf("Run() unexpected error: %v", err)
}

if err := opts.PostRun(ctx); err != nil {
t.Fatalf("PostRun() unexpected error: %v", err)
}
Expand Down
8 changes: 8 additions & 0 deletions internal/cli/deployments/search/indexes/delete.go
Expand Up @@ -99,6 +99,11 @@ func (opts *DeleteOpts) initMongoDBClient(ctx context.Context) func() error {
}
}

func (opts *DeleteOpts) PostRun() error {
opts.DeploymentTelemetry.AppendDeploymentType()
return nil
}

func DeleteBuilder() *cobra.Command {
opts := &DeleteOpts{
DeleteOpts: cli.NewDeleteOpts(deletedMessage, deleteMessageFailed),
Expand Down Expand Up @@ -127,6 +132,9 @@ func DeleteBuilder() *cobra.Command {

return opts.Run(cmd.Context())
},
PostRunE: func(_ *cobra.Command, _ []string) error {
return opts.PostRun()
},
}

cmd.Flags().StringVar(&opts.DeploymentName, flag.DeploymentName, "", usage.DeploymentName)
Expand Down
18 changes: 18 additions & 0 deletions internal/cli/deployments/search/indexes/delete_test.go
Expand Up @@ -147,6 +147,24 @@ func TestDelete_RunAtlas(t *testing.T) {
}
}

func TestDelete_PostRun(t *testing.T) {
blva marked this conversation as resolved.
Show resolved Hide resolved
ctrl := gomock.NewController(t)
deploymentTest := fixture.NewMockLocalDeploymentOpts(ctrl, "localDeployment")
opts := &DeleteOpts{
DeploymentOpts: *deploymentTest.Opts,
}

deploymentTest.
MockDeploymentTelemetry.
EXPECT().
AppendDeploymentType().
Times(1)

if err := opts.PostRun(); err != nil {
t.Fatalf("PostRun() unexpected error: %v", err)
}
}

func TestDeleteBuilder(t *testing.T) {
test.CmdValidator(
t,
Expand Down
8 changes: 8 additions & 0 deletions internal/cli/deployments/search/indexes/describe.go
Expand Up @@ -103,6 +103,11 @@ func (opts *DescribeOpts) initStore(ctx context.Context) func() error {
}
}

func (opts *DescribeOpts) PostRun() error {
opts.DeploymentTelemetry.AppendDeploymentType()
return nil
}

func DescribeBuilder() *cobra.Command {
opts := &DescribeOpts{}
cmd := &cobra.Command{
Expand All @@ -129,6 +134,9 @@ func DescribeBuilder() *cobra.Command {
}
return opts.Run(cmd.Context())
},
PostRunE: func(_ *cobra.Command, _ []string) error {
return opts.PostRun()
},
}

cmd.Flags().StringVar(&opts.DeploymentType, flag.TypeFlag, "", usage.DeploymentType)
Expand Down