feat: add support for zip projects in project-clone init container #460

amisevsk · 2021-06-16T20:36:05Z

What does this PR do?

Add support for zip-type projects in devfiles. Zip files are downloaded and extracted as-is.
Delegate git clone step to standard git binary rather than relying on the go-git library

Using git instead of go-git for cloning projects results in the project-clone image being ~180MB larger (due to requiring git). However, from some brief testing, it also reduces the memory requirement of project clone by around 5x for large projects.

What issues does this PR fix or reference?

Closes #416
Closes #454

Is it tested? How?

Changes are built and pushed to quay.io/amisevsk/project-clone:zip-support and can be tested with the devfile below

cat <<EOF | kubectl apply -f -
kind: DevWorkspace
apiVersion: workspace.devfile.io/v1alpha2
metadata:
  name: projects-testing
spec:
  started: true
  template:
    projects:
      - name: dwo
        zip:
          location: https://github.com/devfile/devworkspace-operator/archive/refs/heads/0.6.x.zip
        clonePath: test-project-zip
      - name: dwo-2
        git:
          checkoutFrom:
            revision: 0.6.x
          remotes:
            origin: https://github.com/devfile/devworkspace-operator.git
        clonePath: "test-project-git"

    components:
      - name: tools
        container:
          image: quay.io/amisevsk/project-clone:debug
          command:
            - "tail"
            - "-f"
            - "/dev/null"
EOF

Alternatively, the project clone image can be tested directly:

TMPDIR=$(mktemp -d)
cat <<EOF > "$TMPDIR/dw.yaml"
projects:
  - name: dwo
    zip:
      location: https://github.com/devfile/devworkspace-operator/archive/refs/heads/0.6.x.zip
    clonePath: test-project-zip
  - name: dwo-2
    git:
      checkoutFrom:
        revision: 0.6.x
      remotes:
        origin: https://github.com/devfile/devworkspace-operator.git
    clonePath: "test-project-git"
EOF

podman run \
  -e DEVWORKSPACE_FLATTENED_DEVFILE=/tmp/dw.yaml \
  -e PROJECTS_ROOT=/projects \
  --mount type=bind,src=$TMPDIR/dw.yaml,dst=/tmp/dw.yaml,z \
  --mount type=tmpfs,tmpfs-size=500M,dst=/projects/ \
  quay.io/amisevsk/project-clone:zip-support -- /bin/bash -c "/usr/local/bin/project-clone; sleep 10d"

(once clone is completed, run podman exec -it <the container> /bin/bash and check /projects manually)

benoitf · 2021-06-17T10:02:47Z

hello, do you think we could use https://github.com/kubernetes/git-sync instead as the utility to clone projects ?

amisevsk · 2021-06-17T14:39:33Z

git-sync may be overkill or targeting a different use-case. We basically only ever want to sync the git repo once -- otherwise we risk overwriting user changes on start.

…unctionality Signed-off-by: Angel Misevski <amisevsk@redhat.com>

Signed-off-by: Angel Misevski <amisevsk@redhat.com>

…chives Signed-off-by: Angel Misevski <amisevsk@redhat.com>

Instead of using PlainClone() from go-git, delegate the cloning step to the standard git binary. This is required as go-git's clone operation takes significant memory when cloning large projects. This commit installs git in the project-clone container, which increases its size by ~180MB. Signed-off-by: Angel Misevski <amisevsk@redhat.com>

Signed-off-by: Angel Misevski <amisevsk@redhat.com>

Change entrypoint used for devworkspace operator and project clone to use /bin/bash (shellcheck warns that some syntax in the devfile is not POSIX sh compatible) and double quotes the `exec $@` line to allow overriding entrypoint with e.g. /bin/bash -c "echo one && echo two" Signed-off-by: Angel Misevski <amisevsk@redhat.com>

project-clone/internal/devfile.go

project-clone/internal/shell/execute.go

project-clone/internal/utils.go

sleshchenko · 2021-06-22T11:37:57Z

project-clone/main.go

 // TODO: Handle sparse checkout
 // TODO: Add support for auth
 func main() {
-	workspace, err := readFlattenedDevWorkspace()
+	workspace, err := internal.ReadFlattenedDevWorkspace()
 	if err != nil {
 		log.Printf("Failed to read current DevWorkspace: %s", err)


seems it can be Failed to read current DevWorkspace: error reading current DevWorkspace YAML which is a bit of duplication

maybe error reading current DevWorkspace YAML -> error reading file %s

sleshchenko · 2021-06-22T11:43:13Z

project-clone/main.go

-		} else {
-			log.Printf("Project '%s' is already cloned and has all remotes configured", project.Name)
+		if err != nil {
+			log.Printf("Encountered error while setting up project %s: %s", project.Name, err)


Suggested change

log.Printf("Encountered error while setting up project %s: %s", project.Name, err)

log.Printf("Encountered error while setting up project %s: %s", project.Name, err)

os.Exit(1)

?

Also, probably git/setup.go is suppose to be adapted since zip return error while git log error and go os.Exit(1).

sleshchenko · 2021-06-22T12:14:57Z

project-clone/internal/zip/setup.go

+// closeSafe is a wrapper on io.Closer.Close() that just prints an error if encountered.
+//
+// Adapted from the Che plugin broker:
+// https://github.com/eclipse/che-plugin-broker/blob/27e7c6953c92633cbe7e8ce746a16ca10d240ea2/utils/ioutil.go#L326


how these references from sources could help us in future? Maybe just drop them?

It feels strange to copy-paste code from another project without referencing its source, even if I helped write both functions :)

sleshchenko · 2021-06-22T13:17:03Z

project-clone/internal/zip/setup.go

+	}
+
+	log.Printf("Extracting project archive to %s", zipFilePath)
+	err = unzip(zipFilePath, projectPath)


I wonder if we should not drop the top level folder.
Like

cat <<EOF | kubectl apply -f - kind: DevWorkspace apiVersion: workspace.devfile.io/v1alpha2 metadata: name: projects-testing spec: started: true template: projects: - name: dwo zip: location: https://github.com/devfile/devworkspace-operator/archive/refs/heads/main.zip clonePath: test-project-zip - name: dwo-2 git: checkoutFrom: revision: 0.6.x remotes: origin: https://github.com/devfile/devworkspace-operator.git clonePath: "test-project-git" components: - name: tools container: image: quay.io/amisevsk/project-clone:debug command: - "tail" - "-f" - "/dev/null" EOF

^ the project is the same but git will be in test-project-git when zip in test-project-zip/devworkspace-operator-main.
I'm not sure about the current Theia behavior

I'll work on it 👍

I was mostly going for compatibility with how Theia currently does it, but there's no real reason for that :)

Co-authored-by: Serhii Leshchenko <sleshche@redhat.com>

Signed-off-by: Angel Misevski <amisevsk@redhat.com>

Fix an issue where if a project does not specify checkoutFrom.Remote when using only one remote, the remote name in logs is empty. Signed-off-by: Angel Misevski <amisevsk@redhat.com>

Change project-clone functionality to extract zip projects while dropping top-level directories in the archive, e.g. change /projects/my-project/my-project-main/[files] to /projects/my-project/[files] when /projects/my-project/ contains only a single directory Signed-off-by: Angel Misevski <amisevsk@redhat.com>

sleshchenko

tested and it works fine

openshift-ci · 2021-06-23T15:11:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amisevsk, JPinkney, sleshchenko

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [JPinkney,amisevsk,sleshchenko]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

sleshchenko · 2021-06-23T16:17:43Z

/test v7-devworkspaces-operator-e2e, v7-devworkspace-happy-path

amisevsk requested review from benoitf, sleshchenko and JPinkney June 16, 2021 20:36

openshift-ci bot added approved needs-rebase labels Jun 16, 2021

amisevsk changed the title ~~Zip project clone~~ feat: add support for zip projects in project-clone init container Jun 16, 2021

amisevsk force-pushed the zip-project-clone branch from ff47006 to 9510936 Compare June 16, 2021 20:41

openshift-ci bot removed the needs-rebase label Jun 16, 2021

openshift-ci bot added the needs-rebase label Jun 21, 2021

amisevsk added 6 commits June 21, 2021 14:58

refactor: Reorganize project-clone internal package to separate git f…

41d01c1

…unctionality Signed-off-by: Angel Misevski <amisevsk@redhat.com>

refactor: Move functionality out of project-clone main.go

4bc2659

Signed-off-by: Angel Misevski <amisevsk@redhat.com>

feat: Add support for downloading and extracting projects from zip ar…

45732c5

…chives Signed-off-by: Angel Misevski <amisevsk@redhat.com>

docs: Add/Update documentation in project-clone package.

30968ce

Signed-off-by: Angel Misevski <amisevsk@redhat.com>

amisevsk force-pushed the zip-project-clone branch from 9510936 to 2896fcf Compare June 21, 2021 18:59

openshift-ci bot removed the needs-rebase label Jun 21, 2021

JPinkney approved these changes Jun 21, 2021

View reviewed changes

openshift-ci bot assigned JPinkney Jun 21, 2021

openshift-ci bot added the lgtm label Jun 21, 2021

sleshchenko reviewed Jun 22, 2021

View reviewed changes

project-clone/internal/devfile.go Show resolved Hide resolved

project-clone/internal/shell/execute.go Outdated Show resolved Hide resolved

sleshchenko reviewed Jun 22, 2021

View reviewed changes

Apply suggestions from code review

5e25ffd

Co-authored-by: Serhii Leshchenko <sleshche@redhat.com>

openshift-ci bot removed lgtm labels Jun 23, 2021

amisevsk added 2 commits June 22, 2021 23:48

Rework git parts of project-clone to return error instead of exiting

f5204a6

Signed-off-by: Angel Misevski <amisevsk@redhat.com>

Fix empty fields in error messages when a single remote is used

1de14c9

Fix an issue where if a project does not specify checkoutFrom.Remote when using only one remote, the remote name in logs is empty. Signed-off-by: Angel Misevski <amisevsk@redhat.com>

sleshchenko approved these changes Jun 23, 2021

View reviewed changes

openshift-ci bot assigned sleshchenko Jun 23, 2021

openshift-ci bot added the lgtm label Jun 23, 2021

amisevsk merged commit cc86aac into devfile:main Jun 24, 2021

amisevsk deleted the zip-project-clone branch June 24, 2021 01:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add support for zip projects in project-clone init container #460

feat: add support for zip projects in project-clone init container #460

amisevsk commented Jun 16, 2021

benoitf commented Jun 17, 2021

amisevsk commented Jun 17, 2021

sleshchenko Jun 22, 2021

sleshchenko Jun 22, 2021

amisevsk Jun 23, 2021

sleshchenko Jun 22, 2021

amisevsk Jun 23, 2021

sleshchenko Jun 22, 2021 •

edited

Loading

amisevsk Jun 23, 2021

sleshchenko left a comment

openshift-ci bot commented Jun 23, 2021

sleshchenko commented Jun 23, 2021

	log.Printf("Encountered error while setting up project %s: %s", project.Name, err)
	log.Printf("Encountered error while setting up project %s: %s", project.Name, err)
	os.Exit(1)

feat: add support for zip projects in project-clone init container #460

feat: add support for zip projects in project-clone init container #460

Conversation

amisevsk commented Jun 16, 2021

What does this PR do?

What issues does this PR fix or reference?

Is it tested? How?

benoitf commented Jun 17, 2021

amisevsk commented Jun 17, 2021

sleshchenko Jun 22, 2021

Choose a reason for hiding this comment

sleshchenko Jun 22, 2021

Choose a reason for hiding this comment

amisevsk Jun 23, 2021

Choose a reason for hiding this comment

sleshchenko Jun 22, 2021

Choose a reason for hiding this comment

amisevsk Jun 23, 2021

Choose a reason for hiding this comment

sleshchenko Jun 22, 2021 • edited Loading

Choose a reason for hiding this comment

amisevsk Jun 23, 2021

Choose a reason for hiding this comment

sleshchenko left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Jun 23, 2021

sleshchenko commented Jun 23, 2021

sleshchenko Jun 22, 2021 •

edited

Loading