diff --git a/controller/activities/app.go b/controller/activities/app.go index 8380614..186144c 100644 --- a/controller/activities/app.go +++ b/controller/activities/app.go @@ -90,7 +90,8 @@ type Image struct { Tag string } -func updateImageTags(node *yaml.Node, newImages []Image) error { +func updateImageTags(node *yaml.Node, newImages []Image) (bool, error) { + changed := false var walk func(n *yaml.Node) walk = func(n *yaml.Node) { if n.Kind != yaml.MappingNode { @@ -116,8 +117,9 @@ func updateImageTags(node *yaml.Node, newImages []Image) error { } if repoNode != nil && tagNode != nil { for _, img := range newImages { - if repoNode.Value == img.Repository { + if repoNode.Value == img.Repository && tagNode.Value != img.Tag { tagNode.Value = img.Tag + changed = true } } } @@ -127,38 +129,42 @@ func updateImageTags(node *yaml.Node, newImages []Image) error { } } walk(node) - return nil + return changed, nil } -func UpdateAppVersion(ctx context.Context, appsDir, namespace, app, cluster string, newImages []Image) error { +func UpdateAppVersion(ctx context.Context, appsDir, namespace, app, cluster string, newImages []Image) (bool, error) { path := filepath.Join(appsDir, namespace, app, fmt.Sprintf("%s.yaml", cluster)) data, err := os.ReadFile(path) if err != nil { - return fmt.Errorf("failed to read file: %w", err) + return false, fmt.Errorf("failed to read file: %w", err) } var node yaml.Node if err := yaml.Unmarshal(data, &node); err != nil { - return fmt.Errorf("failed to unmarshal YAML: %w", err) + return false, fmt.Errorf("failed to unmarshal YAML: %w", err) } - if err := updateImageTags(&node, newImages); err != nil { - return fmt.Errorf("failed to update image tags: %w", err) + changed, err := updateImageTags(&node, newImages) + if err != nil { + return false, fmt.Errorf("failed to update image tags: %w", err) } - var buf bytes.Buffer - encoder := yaml.NewEncoder(&buf) - encoder.SetIndent(2) + if changed { + var buf bytes.Buffer + encoder := yaml.NewEncoder(&buf) + encoder.SetIndent(2) - if err := encoder.Encode(&node); err != nil { - return fmt.Errorf("failed to encode YAML: %w", err) - } - encoder.Close() + if err := encoder.Encode(&node); err != nil { + return false, fmt.Errorf("failed to encode YAML: %w", err) + } + encoder.Close() - if err := os.WriteFile(path, buf.Bytes(), 0644); err != nil { - return fmt.Errorf("failed to write YAML file: %w", err) + newData := buf.Bytes() + if err := os.WriteFile(path, newData, 0644); err != nil { + return false, fmt.Errorf("failed to write YAML file: %w", err) + } } - return nil + return changed, nil } diff --git a/controller/activities/app_test.go b/controller/activities/app_test.go index e966d93..9fd35ad 100644 --- a/controller/activities/app_test.go +++ b/controller/activities/app_test.go @@ -204,7 +204,7 @@ service: // Execute UpdateAppVersion ctx := context.Background() - err = UpdateAppVersion(ctx, tempDir, namespace, app, cluster, tt.newImages) + changed, err := UpdateAppVersion(ctx, tempDir, namespace, app, cluster, tt.newImages) if tt.expectError { assert.Error(t, err) @@ -212,6 +212,7 @@ service: } require.NoError(t, err) + assert.Equal(t, tt.expectedUpdate, changed, "Expected change result doesn't match") // Read the updated file updatedContent, err := os.ReadFile(yamlPath) @@ -237,7 +238,7 @@ func TestUpdateAppVersion_FileErrors(t *testing.T) { defer os.RemoveAll(tempDir) t.Run("non-existent file", func(t *testing.T) { - err := UpdateAppVersion(ctx, tempDir, "ns", "app", "cluster", []Image{}) + _, err := UpdateAppVersion(ctx, tempDir, "ns", "app", "cluster", []Image{}) assert.Error(t, err) assert.Contains(t, err.Error(), "failed to read file") }) @@ -255,7 +256,7 @@ func TestUpdateAppVersion_FileErrors(t *testing.T) { err = os.WriteFile(yamlPath, []byte("invalid: yaml: content: ["), 0644) require.NoError(t, err) - err = UpdateAppVersion(ctx, tempDir, namespace, app, cluster, []Image{}) + _, err = UpdateAppVersion(ctx, tempDir, namespace, app, cluster, []Image{}) assert.Error(t, err) assert.Contains(t, err.Error(), "failed to unmarshal YAML") }) @@ -359,7 +360,7 @@ service: err := yaml.Unmarshal([]byte(tt.yamlContent), &node) require.NoError(t, err) - err = updateImageTags(&node, tt.newImages) + _, err = updateImageTags(&node, tt.newImages) require.NoError(t, err) // Marshall back to verify changes @@ -451,7 +452,7 @@ service: } ctx := context.Background() - err = UpdateAppVersion(ctx, tempDir, namespace, app, cluster, newImages) + _, err = UpdateAppVersion(ctx, tempDir, namespace, app, cluster, newImages) require.NoError(t, err) // Read the updated file and check indentation diff --git a/controller/workflows/app_update.go b/controller/workflows/app_update.go index abe5aa8..2dc6816 100644 --- a/controller/workflows/app_update.go +++ b/controller/workflows/app_update.go @@ -48,6 +48,7 @@ func AppUpdate(ctx workflow.Context, input AppUpdateInput) error { }() appsDir := filepath.Join(workspace, "apps") + var changed bool if err := workflow.ExecuteActivity( workflow.WithActivityOptions(ctx, workflow.ActivityOptions{ StartToCloseTimeout: 30 * time.Second, @@ -58,7 +59,7 @@ func AppUpdate(ctx workflow.Context, input AppUpdateInput) error { input.App, input.Cluster, input.NewImages, - ).Get(ctx, nil); err != nil { + ).Get(ctx, &changed); err != nil { logger.Error("failed to update app version", "error", err) return fmt.Errorf("failed to update app version: %w", err) } @@ -66,7 +67,14 @@ func AppUpdate(ctx workflow.Context, input AppUpdateInput) error { logger.Info("App version updated successfully", "namespace", input.Namespace, "app", input.App, - "cluster", input.Cluster) + "cluster", input.Cluster, + "changed", changed) + + // Skip remaining steps if no changes were made + if !changed { + logger.Info("No changes detected, skipping remaining steps") + return nil + } // Step 3: Git add changes appFilePath := filepath.Join(appsDir, input.Namespace, input.App, fmt.Sprintf("%s.yaml", input.Cluster)) diff --git a/controller/workflows/app_update_test.go b/controller/workflows/app_update_test.go index 7080b8a..4929088 100644 --- a/controller/workflows/app_update_test.go +++ b/controller/workflows/app_update_test.go @@ -49,7 +49,7 @@ func (s *AppUpdateWorkflowTestSuite) TestAppUpdate_Success() { s.env.OnActivity(activities.Clone, mock.Anything, input.Url, input.Revision).Return(workspace, nil) s.env.OnActivity(activities.UpdateAppVersion, mock.Anything, - workspace+"/apps", input.Namespace, input.App, input.Cluster, input.NewImages).Return(nil) + workspace+"/apps", input.Namespace, input.App, input.Cluster, input.NewImages).Return(true, nil) s.env.OnActivity(activities.GitAdd, mock.Anything, appFilePath).Return(nil) s.env.OnActivity(activities.GitCommit, mock.Anything, workspace, "chore(khuedoan/blog): update production version").Return(nil) s.env.OnActivity(activities.GitPush, mock.Anything, workspace).Return(nil) @@ -100,7 +100,7 @@ func (s *AppUpdateWorkflowTestSuite) TestAppUpdate_UpdateAppVersionFailure() { s.env.OnActivity(activities.Clone, mock.Anything, input.Url, input.Revision).Return(workspace, nil) s.env.OnActivity(activities.UpdateAppVersion, mock.Anything, - workspace+"/apps", input.Namespace, input.App, input.Cluster, input.NewImages).Return( + workspace+"/apps", input.Namespace, input.App, input.Cluster, input.NewImages).Return(false, errors.New("failed to read file: no such file or directory")) s.env.ExecuteWorkflow(AppUpdate, input) @@ -127,7 +127,7 @@ func (s *AppUpdateWorkflowTestSuite) TestAppUpdate_GitAddFailure() { s.env.OnActivity(activities.Clone, mock.Anything, input.Url, input.Revision).Return(workspace, nil) s.env.OnActivity(activities.UpdateAppVersion, mock.Anything, - workspace+"/apps", input.Namespace, input.App, input.Cluster, input.NewImages).Return(nil) + workspace+"/apps", input.Namespace, input.App, input.Cluster, input.NewImages).Return(true, nil) s.env.OnActivity(activities.GitAdd, mock.Anything, appFilePath).Return( errors.New("git add failed: file not found")) @@ -155,7 +155,7 @@ func (s *AppUpdateWorkflowTestSuite) TestAppUpdate_GitCommitFailure() { s.env.OnActivity(activities.Clone, mock.Anything, input.Url, input.Revision).Return(workspace, nil) s.env.OnActivity(activities.UpdateAppVersion, mock.Anything, - workspace+"/apps", input.Namespace, input.App, input.Cluster, input.NewImages).Return(nil) + workspace+"/apps", input.Namespace, input.App, input.Cluster, input.NewImages).Return(true, nil) s.env.OnActivity(activities.GitAdd, mock.Anything, appFilePath).Return(nil) s.env.OnActivity(activities.GitCommit, mock.Anything, workspace, "chore(khuedoan/notes): update production version").Return( errors.New("git commit failed: nothing to commit")) @@ -184,7 +184,7 @@ func (s *AppUpdateWorkflowTestSuite) TestAppUpdate_GitPushFailure() { s.env.OnActivity(activities.Clone, mock.Anything, input.Url, input.Revision).Return(workspace, nil) s.env.OnActivity(activities.UpdateAppVersion, mock.Anything, - workspace+"/apps", input.Namespace, input.App, input.Cluster, input.NewImages).Return(nil) + workspace+"/apps", input.Namespace, input.App, input.Cluster, input.NewImages).Return(true, nil) s.env.OnActivity(activities.GitAdd, mock.Anything, appFilePath).Return(nil) s.env.OnActivity(activities.GitCommit, mock.Anything, workspace, "chore(khuedoan/notes): update production version").Return(nil) s.env.OnActivity(activities.GitPush, mock.Anything, workspace).Return( @@ -214,7 +214,7 @@ func (s *AppUpdateWorkflowTestSuite) TestAppUpdate_PushRenderedAppFailure() { s.env.OnActivity(activities.Clone, mock.Anything, input.Url, input.Revision).Return(workspace, nil) s.env.OnActivity(activities.UpdateAppVersion, mock.Anything, - workspace+"/apps", input.Namespace, input.App, input.Cluster, input.NewImages).Return(nil) + workspace+"/apps", input.Namespace, input.App, input.Cluster, input.NewImages).Return(true, nil) s.env.OnActivity(activities.GitAdd, mock.Anything, appFilePath).Return(nil) s.env.OnActivity(activities.GitCommit, mock.Anything, workspace, "chore(khuedoan/notes): update production version").Return(nil) s.env.OnActivity(activities.GitPush, mock.Anything, workspace).Return(nil) @@ -251,7 +251,7 @@ func (s *AppUpdateWorkflowTestSuite) TestAppUpdate_MultipleImages() { s.env.OnActivity(activities.Clone, mock.Anything, input.Url, input.Revision).Return(workspace, nil) s.env.OnActivity(activities.UpdateAppVersion, mock.Anything, - workspace+"/apps", input.Namespace, input.App, input.Cluster, input.NewImages).Return(nil) + workspace+"/apps", input.Namespace, input.App, input.Cluster, input.NewImages).Return(true, nil) s.env.OnActivity(activities.GitAdd, mock.Anything, appFilePath).Return(nil) s.env.OnActivity(activities.GitCommit, mock.Anything, workspace, "chore(test/example): update local version").Return(nil) s.env.OnActivity(activities.GitPush, mock.Anything, workspace).Return(nil) @@ -286,7 +286,7 @@ func (s *AppUpdateWorkflowTestSuite) TestAppUpdate_RealWorldExample() { s.env.OnActivity(activities.Clone, mock.Anything, input.Url, input.Revision).Return(workspace, nil) s.env.OnActivity(activities.UpdateAppVersion, mock.Anything, - workspace+"/apps", input.Namespace, input.App, input.Cluster, input.NewImages).Return(nil) + workspace+"/apps", input.Namespace, input.App, input.Cluster, input.NewImages).Return(true, nil) s.env.OnActivity(activities.GitAdd, mock.Anything, appFilePath).Return(nil) s.env.OnActivity(activities.GitCommit, mock.Anything, workspace, "chore(khuedoan/blog): update production version").Return(nil) s.env.OnActivity(activities.GitPush, mock.Anything, workspace).Return(nil) @@ -340,7 +340,7 @@ func (s *AppUpdateWorkflowTestSuite) TestAppUpdate_EmptyImages() { s.env.OnActivity(activities.Clone, mock.Anything, input.Url, input.Revision).Return(workspace, nil) s.env.OnActivity(activities.UpdateAppVersion, mock.Anything, - workspace+"/apps", input.Namespace, input.App, input.Cluster, input.NewImages).Return(nil) + workspace+"/apps", input.Namespace, input.App, input.Cluster, input.NewImages).Return(true, nil) s.env.OnActivity(activities.GitAdd, mock.Anything, appFilePath).Return(nil) s.env.OnActivity(activities.GitCommit, mock.Anything, workspace, "chore(test/app): update local version").Return(nil) s.env.OnActivity(activities.GitPush, mock.Anything, workspace).Return(nil) @@ -353,6 +353,31 @@ func (s *AppUpdateWorkflowTestSuite) TestAppUpdate_EmptyImages() { s.NoError(s.env.GetWorkflowError()) } +func (s *AppUpdateWorkflowTestSuite) TestAppUpdate_NoChanges() { + input := AppUpdateInput{ + Url: "https://github.com/example/cloudlab.git", + Revision: "main", + Namespace: "test", + App: "app", + Cluster: "local", + Registry: "registry.example.com", + NewImages: []activities.Image{ + {Repository: "docker.io/test/app", Tag: "existing-tag"}, // Same tag as already in file + }, + } + workspace := "/tmp/cloudlab-repos/nochange123" + + s.env.OnActivity(activities.Clone, mock.Anything, input.Url, input.Revision).Return(workspace, nil) + s.env.OnActivity(activities.UpdateAppVersion, mock.Anything, + workspace+"/apps", input.Namespace, input.App, input.Cluster, input.NewImages).Return(false, nil) + // Note: No other activities should be called when there are no changes + + s.env.ExecuteWorkflow(AppUpdate, input) + + s.True(s.env.IsWorkflowCompleted()) + s.NoError(s.env.GetWorkflowError()) +} + func (s *AppUpdateWorkflowTestSuite) TestAppUpdate_SpecialCharactersInPath() { input := AppUpdateInput{ Url: "https://github.com/example/cloudlab.git", @@ -374,7 +399,7 @@ func (s *AppUpdateWorkflowTestSuite) TestAppUpdate_SpecialCharactersInPath() { s.env.OnActivity(activities.Clone, mock.Anything, input.Url, input.Revision).Return(workspace, nil) s.env.OnActivity(activities.UpdateAppVersion, mock.Anything, - workspace+"/apps", input.Namespace, input.App, input.Cluster, input.NewImages).Return(nil) + workspace+"/apps", input.Namespace, input.App, input.Cluster, input.NewImages).Return(true, nil) s.env.OnActivity(activities.GitAdd, mock.Anything, appFilePath).Return(nil) s.env.OnActivity(activities.GitCommit, mock.Anything, workspace, "chore(test-namespace/app-with-dashes): update staging-env version").Return(nil) s.env.OnActivity(activities.GitPush, mock.Anything, workspace).Return(nil)