feat(resource-service): reduce git fetch operations (#9410)

* feat(resource-service): reduce git fetch operations Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com> * added comment Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com> * trigger build Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com> * added more unit tests Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com> Signed-off-by: Florian Bacher <florian.bacher@dynatrace.com>
keptn · Jan 9, 2023 · 4a2b091 · 4a2b091
1 parent 5b96ef7
commit 4a2b091
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 6 deletions.
diff --git a/resource-service/common/git.go b/resource-service/common/git.go
@@ -556,15 +556,11 @@ func (g *Git) CheckoutBranch(gitContext common_models.GitContext, branch string)
 
 func (g *Git) checkoutBranch(gitContext common_models.GitContext, options *git.CheckoutOptions) error {
 	if g.ProjectExists(gitContext) {
-		r, w, err := g.getWorkTree(gitContext)
+		_, w, err := g.getWorkTree(gitContext)
 		if err != nil {
 			logger.Debugf("checkoutBranch(): Could not get worktree for project '%s': %s", gitContext.Project, err.Error())
 			return err
 		}
-		if err = g.fetch(gitContext, r); err != nil {
-			logger.Debugf("checkoutBranch(): Ccould not fetch for project '%s': %s", gitContext.Project, err.Error())
-			return err
-		}
 
 		return w.Checkout(options)
 	}

diff --git a/resource-service/handler/resource_manager.go b/resource-service/handler/resource_manager.go
@@ -15,7 +15,8 @@ import (
 	"github.com/keptn/keptn/resource-service/models"
 )
 
-//IResourceManager provides an interface for resource CRUD operations
+// IResourceManager provides an interface for resource CRUD operations
+//
 //go:generate moq -pkg handler_mock -skip-ensure -out ./fake/resource_manager_mock.go . IResourceManager
 type IResourceManager interface {
 	CreateResources(params models.CreateResourcesParams) (*models.WriteResourceResponse, error)
@@ -63,6 +64,11 @@ func (p ResourceManager) GetResources(params models.GetResourcesParams) (*models
 	if err != nil {
 		return nil, err
 	}
+	// since we do not automatically fetch each time when we check out a branch, we need to pull
+	// here to get the latest state from the upstream
+	if err := p.git.Pull(*gitContext); err != nil {
+		return nil, err
+	}
 	revision, err := p.git.GetCurrentRevision(*gitContext)
 	if err != nil {
 		return nil, err

diff --git a/resource-service/handler/resource_manager_test.go b/resource-service/handler/resource_manager_test.go
@@ -1250,6 +1250,68 @@ func TestResourceManager_GetResources(t *testing.T) {
 	require.Len(t, fields.fileSystem.WalkPathCalls(), 1)
 }
 
+func TestResourceManager_GetResourcesPullFailed(t *testing.T) {
+	fields := getTestResourceManagerFields()
+
+	fields.git.PullFunc = func(gitContext common_models.GitContext) error {
+		return errors.New("oops")
+	}
+
+	rm := NewResourceManager(fields.git, fields.credentialReader, fields.fileSystem, fields.stageContext)
+
+	result, err := rm.GetResources(models.GetResourcesParams{
+		ResourceContext: models.ResourceContext{
+			Project: models.Project{ProjectName: "my-project"},
+		},
+		GetResourcesQuery: models.GetResourcesQuery{
+			PageSize: 10,
+		},
+	})
+
+	require.NotNil(t, err)
+
+	require.Nil(t, result)
+
+	require.Len(t, fields.stageContext.EstablishCalls(), 1)
+
+	require.Len(t, fields.git.PullCalls(), 1)
+
+	require.Empty(t, fields.git.GetCurrentRevisionCalls())
+
+	require.Empty(t, fields.fileSystem.WalkPathCalls())
+}
+
+func TestResourceManager_GetCurrentRevisionFailed(t *testing.T) {
+	fields := getTestResourceManagerFields()
+
+	fields.git.GetCurrentRevisionFunc = func(gitContext common_models.GitContext) (string, error) {
+		return "", errors.New("oops")
+	}
+
+	rm := NewResourceManager(fields.git, fields.credentialReader, fields.fileSystem, fields.stageContext)
+
+	result, err := rm.GetResources(models.GetResourcesParams{
+		ResourceContext: models.ResourceContext{
+			Project: models.Project{ProjectName: "my-project"},
+		},
+		GetResourcesQuery: models.GetResourcesQuery{
+			PageSize: 10,
+		},
+	})
+
+	require.NotNil(t, err)
+
+	require.Nil(t, result)
+
+	require.Len(t, fields.stageContext.EstablishCalls(), 1)
+
+	require.Len(t, fields.git.PullCalls(), 1)
+
+	require.Len(t, fields.git.GetCurrentRevisionCalls(), 1)
+
+	require.Empty(t, fields.fileSystem.WalkPathCalls())
+}
+
 type fakeFileInfo struct {
 	name  string
 	isDir bool