From 32f14dd0b1c2ff8612071225e72ac2f10f6a52b1 Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Mon, 19 Feb 2024 11:44:35 +0100 Subject: [PATCH 1/7] adds a dummy cleanup command --- cmd/cleanup.go | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 cmd/cleanup.go diff --git a/cmd/cleanup.go b/cmd/cleanup.go new file mode 100644 index 0000000..d4081f4 --- /dev/null +++ b/cmd/cleanup.go @@ -0,0 +1,41 @@ +/* +Copyright © 2024 Juliano Martinez + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ +package cmd + +import ( + "fmt" + + "github.com/spf13/cobra" +) + +// cleanupCmd represents the cleanup command +var cleanupCmd = &cobra.Command{ + Use: "cleanup", + Short: "A brief description of your command", + Long: `A longer description that spans multiple lines and likely contains examples +and usage of using your command. For example: + +Cobra is a CLI library for Go that empowers applications. +This application is a tool to generate the needed files +to quickly create a Cobra application.`, + Run: func(cmd *cobra.Command, args []string) { + fmt.Println("cleanup called") + }, +} + +func init() { + rootCmd.AddCommand(cleanupCmd) +} From 5642fe33b9d348498f65f88626692f9294c70000 Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Mon, 19 Feb 2024 15:28:37 +0100 Subject: [PATCH 2/7] adds cleanup --- cmd/cleanup.go | 8 +--- pkg/tagit/tagit.go | 15 +++++++ pkg/tagit/tagit_test.go | 98 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 114 insertions(+), 7 deletions(-) diff --git a/cmd/cleanup.go b/cmd/cleanup.go index d4081f4..5323922 100644 --- a/cmd/cleanup.go +++ b/cmd/cleanup.go @@ -24,13 +24,7 @@ import ( // cleanupCmd represents the cleanup command var cleanupCmd = &cobra.Command{ Use: "cleanup", - Short: "A brief description of your command", - Long: `A longer description that spans multiple lines and likely contains examples -and usage of using your command. For example: - -Cobra is a CLI library for Go that empowers applications. -This application is a tool to generate the needed files -to quickly create a Cobra application.`, + Short: "cleanup removes all services with the tag prefix from a given consul service", Run: func(cmd *cobra.Command, args []string) { fmt.Println("cleanup called") }, diff --git a/pkg/tagit/tagit.go b/pkg/tagit/tagit.go index e2011c4..6d81dfe 100644 --- a/pkg/tagit/tagit.go +++ b/pkg/tagit/tagit.go @@ -97,6 +97,21 @@ func (t *TagIt) Run(ctx context.Context) { } } +// CleanupTags removes all tags with the given prefix from the service. +func (t *TagIt) CleanupTags() error { + service, err := t.getService() + if err != nil { + return fmt.Errorf("error getting service: %w", err) + } + registration := t.copyServiceToRegistration(service) + updatedTags, tagged := t.excludeTagged(registration.Tags) + if tagged { + registration.Tags = updatedTags + return t.client.Agent().ServiceRegister(registration) + } + return nil +} + // runScript runs a command and returns the output. func (t *TagIt) runScript() ([]byte, error) { log.WithFields(log.Fields{ diff --git a/pkg/tagit/tagit_test.go b/pkg/tagit/tagit_test.go index 11ce87c..1732434 100644 --- a/pkg/tagit/tagit_test.go +++ b/pkg/tagit/tagit_test.go @@ -623,3 +623,101 @@ func TestUpdateServiceTags(t *testing.T) { }) } } + +func TestCleanupTags(t *testing.T) { + tests := []struct { + name string + serviceID string + mockServices map[string]*api.AgentService + tagPrefix string + mockRegisterErr error + expectError bool + expectTags []string + }{ + { + name: "Successful Tag Cleanup", + serviceID: "test-service", + mockServices: map[string]*api.AgentService{ + "test-service": { + ID: "test-service", + Tags: []string{"tag-prefix1", "tag-prefix2", "other-tag"}, + }, + }, + tagPrefix: "tag", + expectError: false, + expectTags: []string{"other-tag"}, + }, + { + name: "No Tag Cleanup needed", + serviceID: "test-service", + mockServices: map[string]*api.AgentService{ + "test-service": { + ID: "test-service", + Tags: []string{"prefix1", "prefix2", "other-tag"}, + }, + }, + tagPrefix: "tag", + expectError: false, + expectTags: []string{"prefix1", "prefix2", "other-tag"}, + }, + { + name: "Service Not Found", + serviceID: "non-existent-service", + mockServices: map[string]*api.AgentService{ + "other-service": { + ID: "other-service", + Tags: []string{"some-tag", "another-tag"}, + }, + }, + tagPrefix: "tag-prefix", + expectError: true, + }, + { + name: "Consul Register Error", + serviceID: "test-service", + mockServices: map[string]*api.AgentService{ + "test-service": { + ID: "test-service", + Tags: []string{"tag-prefix1", "other-tag"}, + }, + }, + tagPrefix: "tag", + mockRegisterErr: fmt.Errorf("consul register error"), + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockConsulClient := &MockConsulClient{ + MockAgent: &MockAgent{ + ServicesFunc: func() (map[string]*api.AgentService, error) { + return tt.mockServices, nil + }, + ServiceRegisterFunc: func(reg *api.AgentServiceRegistration) error { + // Ensure the service exists in the mock data + if service, exists := tt.mockServices[reg.ID]; exists && tt.mockRegisterErr == nil { + // Update the tags of the service + service.Tags = reg.Tags + tt.mockServices[reg.ID] = service // Update the map with the modified service + } + return tt.mockRegisterErr + }, + }, + } + tagit := New(mockConsulClient, nil, tt.serviceID, "", 0, tt.tagPrefix) + + err := tagit.CleanupTags() + if (err != nil) != tt.expectError { + t.Errorf("CleanupTags() error = %v, wantErr %v", err, tt.expectError) + } + + if !tt.expectError { + updatedService := tt.mockServices[tt.serviceID] + if updatedService != nil && !reflect.DeepEqual(updatedService.Tags, tt.expectTags) { + t.Errorf("Expected tags after cleanup: %v, got: %v", tt.expectTags, updatedService.Tags) + } + } + }) + } +} From ce74e07050084e83bbc8f98458e22f86b7c9816b Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Mon, 19 Feb 2024 15:28:54 +0100 Subject: [PATCH 3/7] adds comment to systemd --- cmd/systemd.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/cmd/systemd.go b/cmd/systemd.go index 9019e2b..2982860 100644 --- a/cmd/systemd.go +++ b/cmd/systemd.go @@ -24,13 +24,7 @@ import ( // systemdCmd represents the systemd command var systemdCmd = &cobra.Command{ Use: "systemd", - Short: "A brief description of your command", - Long: `A longer description that spans multiple lines and likely contains examples -and usage of using your command. For example: - -Cobra is a CLI library for Go that empowers applications. -This application is a tool to generate the needed files -to quickly create a Cobra application.`, + Short: "systemd generate a systemd service, that you can use for the tagit service", Run: func(cmd *cobra.Command, args []string) { fields := &systemd.Fields{ User: cmd.PersistentFlags().Lookup("user").Value.String(), From 03ce87eb4dde5687880791759f37d12b30f4dac3 Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Mon, 19 Feb 2024 15:36:49 +0100 Subject: [PATCH 4/7] update cleanup --- cmd/cleanup.go | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/cmd/cleanup.go b/cmd/cleanup.go index 5323922..a19b9e5 100644 --- a/cmd/cleanup.go +++ b/cmd/cleanup.go @@ -17,8 +17,10 @@ package cmd import ( "fmt" - + "github.com/hashicorp/consul/api" + "github.com/ncode/tagit/pkg/tagit" "github.com/spf13/cobra" + "os" ) // cleanupCmd represents the cleanup command @@ -26,7 +28,28 @@ var cleanupCmd = &cobra.Command{ Use: "cleanup", Short: "cleanup removes all services with the tag prefix from a given consul service", Run: func(cmd *cobra.Command, args []string) { - fmt.Println("cleanup called") + config := api.DefaultConfig() + config.Address = cmd.InheritedFlags().Lookup("consul-addr").Value.String() + config.Token = cmd.InheritedFlags().Lookup("token").Value.String() + consulClient, err := api.NewClient(config) + if err != nil { + fmt.Printf("error creating consul client: %s", err.Error()) + os.Exit(1) + } + + t := tagit.New( + tagit.NewConsulAPIWrapper(consulClient), + &tagit.CmdExecutor{}, + cmd.InheritedFlags().Lookup("service-id").Value.String(), + cmd.InheritedFlags().Lookup("script").Value.String(), + 0, + cmd.InheritedFlags().Lookup("tag-prefix").Value.String(), + ) + err = t.CleanupTags() + if err != nil { + fmt.Printf("error cleaning up tags: %s", err.Error()) + os.Exit(1) + } }, } From d86f1ad66730f3726e887cd86c81c4177c3ea45d Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Mon, 19 Feb 2024 15:55:16 +0100 Subject: [PATCH 5/7] adds test for Run --- pkg/tagit/tagit_test.go | 47 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/pkg/tagit/tagit_test.go b/pkg/tagit/tagit_test.go index 1732434..4a64c3c 100644 --- a/pkg/tagit/tagit_test.go +++ b/pkg/tagit/tagit_test.go @@ -1,6 +1,7 @@ package tagit import ( + "context" "fmt" "reflect" "slices" @@ -721,3 +722,49 @@ func TestCleanupTags(t *testing.T) { }) } } + +func TestRun(t *testing.T) { + // Setup + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + updateServiceTagsCalled := 0 + mockExecutor := &MockCommandExecutor{ + MockOutput: []byte("new-tag1 new-tag2"), + MockError: nil, + } + mockConsulClient := &MockConsulClient{ + MockAgent: &MockAgent{ + ServicesFunc: func() (map[string]*api.AgentService, error) { + updateServiceTagsCalled++ + return map[string]*api.AgentService{ + "test-service": { + ID: "test-service", + Tags: []string{"old-tag"}, + }, + }, nil + }, + ServiceRegisterFunc: func(reg *api.AgentServiceRegistration) error { + + return nil + }, + }, + } + + tagit := New(mockConsulClient, mockExecutor, "test-service", "echo test", 100*time.Millisecond, "tag") + + // Start Run in a goroutine + go tagit.Run(ctx) + + // Allow some time to pass and then cancel the context + time.Sleep(250 * time.Millisecond) // Adjust this duration as needed + cancel() + + // Allow some time for the goroutine to react to the context cancellation + time.Sleep(50 * time.Millisecond) + + // Check if updateServiceTags was called as expected + if updateServiceTagsCalled < 2 || updateServiceTagsCalled > 3 { + t.Errorf("Expected updateServiceTags to be called 2 or 3 times, got %d", updateServiceTagsCalled) + } +} From d68059b50258b698954f0891b65dd2aa85240812 Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Mon, 19 Feb 2024 15:59:33 +0100 Subject: [PATCH 6/7] adds atomic to avoid race --- pkg/tagit/tagit_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/tagit/tagit_test.go b/pkg/tagit/tagit_test.go index 4a64c3c..a47dfbe 100644 --- a/pkg/tagit/tagit_test.go +++ b/pkg/tagit/tagit_test.go @@ -6,6 +6,7 @@ import ( "reflect" "slices" "strings" + "sync/atomic" "testing" "time" @@ -728,7 +729,7 @@ func TestRun(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() - updateServiceTagsCalled := 0 + updateServiceTagsCalled := atomic.Int32{} mockExecutor := &MockCommandExecutor{ MockOutput: []byte("new-tag1 new-tag2"), MockError: nil, @@ -736,7 +737,7 @@ func TestRun(t *testing.T) { mockConsulClient := &MockConsulClient{ MockAgent: &MockAgent{ ServicesFunc: func() (map[string]*api.AgentService, error) { - updateServiceTagsCalled++ + updateServiceTagsCalled.Add(1) return map[string]*api.AgentService{ "test-service": { ID: "test-service", @@ -764,7 +765,7 @@ func TestRun(t *testing.T) { time.Sleep(50 * time.Millisecond) // Check if updateServiceTags was called as expected - if updateServiceTagsCalled < 2 || updateServiceTagsCalled > 3 { + if updateServiceTagsCalled.Load() < 2 || updateServiceTagsCalled.Load() > 3 { t.Errorf("Expected updateServiceTags to be called 2 or 3 times, got %d", updateServiceTagsCalled) } } From 1cb543868b3603b85944b6a1e15ef8f95ad8cda6 Mon Sep 17 00:00:00 2001 From: Juliano Martinez Date: Mon, 19 Feb 2024 16:06:03 +0100 Subject: [PATCH 7/7] adds error just to have test coverage also --- pkg/tagit/tagit_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/tagit/tagit_test.go b/pkg/tagit/tagit_test.go index a47dfbe..c3938a7 100644 --- a/pkg/tagit/tagit_test.go +++ b/pkg/tagit/tagit_test.go @@ -738,6 +738,9 @@ func TestRun(t *testing.T) { MockAgent: &MockAgent{ ServicesFunc: func() (map[string]*api.AgentService, error) { updateServiceTagsCalled.Add(1) + if updateServiceTagsCalled.Load() == 2 { + return nil, fmt.Errorf("enter error") + } return map[string]*api.AgentService{ "test-service": { ID: "test-service", @@ -758,7 +761,7 @@ func TestRun(t *testing.T) { go tagit.Run(ctx) // Allow some time to pass and then cancel the context - time.Sleep(250 * time.Millisecond) // Adjust this duration as needed + time.Sleep(350 * time.Millisecond) // Adjust this duration as needed cancel() // Allow some time for the goroutine to react to the context cancellation