Skip to content

Add project clone retries functionality #1613

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.

Sign up for GitHub

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

Open
dkwon17 wants to merge 3 commits into
base: main
Choose a base branch
from
+47 −5
Open

Add project clone retries functionality #1613

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f8e5d42
Add project clone retries functionality
dkwon17 Apr 9, 2026
d312f58
Update based on PR feedback
dkwon17 Apr 10, 2026
f20a64c
Run make fmt
dkwon17 Apr 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 26 additions & 4 deletions project-clone/internal/git/setup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//
// Copyright (c) 2019-2025 Red Hat, Inc.
// Copyright (c) 2019-2026 Red Hat, Inc.
// 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
Expand All @@ -20,6 +20,7 @@ import (
"log"
"os"
"path"
"time"

dw "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
projectslib "github.com/devfile/devworkspace-operator/pkg/library/projects"
Expand All @@ -46,9 +47,24 @@ func doInitialGitClone(project *dw.Project) error {
// Clone into a temp dir and then move set up project to PROJECTS_ROOT to try and make clone atomic in case
// project-clone container is terminated
tmpClonePath := path.Join(internal.CloneTmpDir, projectslib.GetClonePath(project))
err := CloneProject(project, tmpClonePath)
if err != nil {
return fmt.Errorf("failed to clone project: %s", err)
var cloneErr error
for attempt := 0; attempt <= internal.CloneRetries; attempt++ {
if attempt > 0 {
delayBeforeRetry(project.Name, attempt)
if err := os.RemoveAll(tmpClonePath); err != nil {
log.Printf("Warning: cleanup before retry failed: %s", err)
}
}
cloneErr = CloneProject(project, tmpClonePath)
if cloneErr == nil {
break
}
if attempt < internal.CloneRetries {
log.Printf("Failed git clone for project %s (attempt %d/%d): %s", project.Name, attempt+1, internal.CloneRetries+1, cloneErr)
}
}
if cloneErr != nil {
return fmt.Errorf("failed to clone project: %w", cloneErr)
}
Comment on lines +66 to 68
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 10, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Final clone-failure path skips temp directory cleanup.

After the last failed attempt, Line 67 returns immediately and can leave partial repo data in tmpClonePath. Over repeated init-container failures, stale temp directories may accumulate under the mounted projects volume.

Proposed fix 
 	if cloneErr != nil {
+		if err := os.RemoveAll(tmpClonePath); err != nil {
+			log.Printf("Warning: cleanup after final clone failure failed: %s", err)
+		}
 		return fmt.Errorf("failed to clone project: %w", cloneErr)
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if cloneErr != nil {
return fmt.Errorf("failed to clone project: %w", cloneErr)
}
if cloneErr != nil {
if err := os.RemoveAll(tmpClonePath); err != nil {
log.Printf("Warning: cleanup after final clone failure failed: %s", err)
}
return fmt.Errorf("failed to clone project: %w", cloneErr)
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@project-clone/internal/git/setup.go` around lines 66 - 68, The clone error
return currently exits without removing the temporary clone directory
(tmpClonePath), leaving stale data; ensure tmpClonePath is cleaned on failure by
calling os.RemoveAll(tmpClonePath) before returning (or by registering a defer
cleanup immediately after creating tmpClonePath) in the function that performs
the clone (refer to the tmpClonePath variable and the clone error handling block
that returns fmt.Errorf("failed to clone project: %w", cloneErr)).

Copy link
Copy Markdown
Collaborator Author

@dkwon17 dkwon17 Apr 10, 2026

Choose a reason for hiding this comment

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

Even if project-clone failed to clone, the workspace will still start. Having partial data in the tmp folder be useful for debugging.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 10, 2026

Choose a reason for hiding this comment

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

@dkwon17, that's a good point — keeping the partial clone data in the temp directory after failure aids debugging. I'll withdraw this suggestion.

✏️ Learnings added 
Learnt from: dkwon17
Repo: devfile/devworkspace-operator PR: 1613
File: project-clone/internal/git/setup.go:66-68
Timestamp: 2026-04-10T19:50:55.397Z
Learning: In `project-clone/internal/git/setup.go`, leaving the partial/stale clone data in `tmpClonePath` after a final clone failure is intentional: the workspace still starts even when project-clone fails, and the leftover data in the temp directory is useful for post-failure debugging. Do not flag the absence of `os.RemoveAll(tmpClonePath)` cleanup on the final error return path as an issue.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

🧠 Learnings used 
Learnt from: CR
Repo: devfile/devworkspace-operator PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-27T15:35:18.698Z
Learning: Applies to test/e2e/**/*_test.go : In e2e tests creating DevWorkspace, MUST use 'DeleteDevWorkspaceAndWait' in cleanup (AfterAll or AfterEach) to delete workspace and wait for PVC cleanup. Using 'DeleteDevWorkspace' alone may leave PVC and cause conflicts.

Learnt from: CR
Repo: devfile/devworkspace-operator PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-27T15:35:18.698Z
Learning: Applies to controllers/workspace/**/*.go : When workspace fails, call 'failWorkspace()' with appropriate 'FailureReason' (ReasonBadRequest, ReasonInfrastructureFailure, ReasonWorkspaceEngineFailure, or ReasonUnknown)

Learnt from: CR
Repo: devfile/devworkspace-operator PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-27T15:35:18.698Z
Learning: Workspace bootstrapping is supported via 'controller.devfile.io/bootstrap-devworkspace: true' attribute when a devfile cannot be resolved but the project can be cloned. Flow: workspace starts with generic deployment, project-clone clones projects, finds devfile, DevWorkspace is updated with found devfile, workspace restarts with new definition.


if project.Attributes.Exists(internal.ProjectSparseCheckout) {
Expand Down Expand Up @@ -83,6 +99,12 @@ func doInitialGitClone(project *dw.Project) error {
return nil
}

func delayBeforeRetry(projectName string, attempt int) {
delay := internal.BaseRetryDelay * (1 << (attempt - 1))
log.Printf("Retrying git clone for project %s (attempt %d/%d) after %s", projectName, attempt+1, internal.CloneRetries+1, delay)
time.Sleep(delay)
}

func setupRemotesForExistingProject(project *dw.Project) error {
projectPath := path.Join(internal.ProjectsRoot, projectslib.GetClonePath(project))
repo, err := internal.OpenRepo(projectPath)
Expand Down
22 changes: 21 additions & 1 deletion project-clone/internal/global.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//
// Copyright (c) 2019-2025 Red Hat, Inc.
// Copyright (c) 2019-2026 Red Hat, Inc.
// 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
Expand All @@ -21,6 +21,8 @@ import (
"log"
"os"
"regexp"
"strconv"
"time"

"github.com/devfile/devworkspace-operator/pkg/library/constants"
gittransport "github.com/go-git/go-git/v5/plumbing/transport"
Expand All @@ -33,11 +35,16 @@ const (
credentialsMountPath = "/.git-credentials/credentials"
sshConfigMountPath = "/etc/ssh/ssh_config"
publicCertsDir = "/public-certs"
cloneRetriesEnvVar = "PROJECT_CLONE_RETRIES"
defaultCloneRetries = 3
maxCloneRetries = 10
BaseRetryDelay = 1 * time.Second
)

var (
ProjectsRoot string
CloneTmpDir string
CloneRetries int
tokenAuthMethod map[string]*githttp.BasicAuth
credentialsRegex = regexp.MustCompile(`https://(.+):(.+)@(.+)`)
)
Expand All @@ -59,6 +66,19 @@ func init() {
log.Printf("Using temporary directory %s", tmpDir)
CloneTmpDir = tmpDir

CloneRetries = defaultCloneRetries
if val := os.Getenv(cloneRetriesEnvVar); val != "" {
parsed, err := strconv.Atoi(val)
if err != nil || parsed < 0 {
log.Printf("Invalid value for %s: %q, using default (%d)", cloneRetriesEnvVar, val, defaultCloneRetries)
} else if parsed > maxCloneRetries {
log.Printf("Value for %s (%d) exceeds maximum (%d), using maximum", cloneRetriesEnvVar, parsed, maxCloneRetries)
CloneRetries = maxCloneRetries
} else {
CloneRetries = parsed
}
}

setupAuth()
}

Expand Down
Loading