Add support for starter projects in DevWorkspaces #1130
Conversation
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## main #1130 +/- ##
==========================================
+ Coverage 52.33% 52.37% +0.04%
==========================================
Files 81 81
Lines 7380 7380
==========================================
+ Hits 3862 3865 +3
+ Misses 3237 3234 -3
Partials 281 281 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Minor comments (mainly typo's) but looks good to me and works as expected.
I tested using subDir with the following devworkspace, based on the dotnet 5.0 sample (with slight modifications to the container image being used to include git, and the project checkoutFrom revision):
kind: DevWorkspace
apiVersion: workspace.devfile.io/v1alpha2
metadata:
name: dotnet50
spec:
started: true
routingClass: 'basic'
template:
attributes:
controller.devfile.io/storage-type: ephemeral
controller.devfile.io/use-starter-project: dotnet50-example
starterProjects:
- name: dotnet50-example
git:
checkoutFrom:
remote: origin
revision: dotnet-5.0
remotes:
origin: https://github.com/redhat-developer/s2i-dotnetcore-ex
subDir: app
components:
- name: dotnet
container:
image: quay.io/wto/web-terminal-tooling:next
args: ["tail", "-f", "/dev/null"]
mountSources: true
env:
- name: CONFIGURATION
value: Debug
- name: STARTUP_PROJECT
value: app.csproj
- name: ASPNETCORE_ENVIRONMENT
value: Development
- name: ASPNETCORE_URLS
value: http://*:8080
endpoints:
- name: http-dotnet50
targetPort: 8080
commands:
- id: build
exec:
workingDir: ${PROJECT_SOURCE}
commandLine: kill $(pidof dotnet); dotnet build -c $CONFIGURATION $STARTUP_PROJECT /p:UseSharedCompilation=false
component: dotnet
group:
isDefault: true
kind: build
- id: run
exec:
workingDir: ${PROJECT_SOURCE}
commandLine: dotnet run -c $CONFIGURATION --no-build --project $STARTUP_PROJECT --no-launch-profile
component: dotnet
group:
isDefault: true
kind: runThe workspace started up correctly. When I navigated to the project directory and did a git status, it showed that a sparse checkout was being used:
bash-4.4 /projects/dotnet50-example $ git status
On branch dotnet-5.0
Your branch is up to date with 'origin/dotnet-5.0'.
You are in a sparse checkout with 11% of tracked files present.
nothing to commit, working tree cleanThe project clone container logs were as follows:
│ project-clone 2023/06/22 00:22:17 Using temporary directory /projects/project-clone-1697882419 │
│ project-clone 2023/06/22 00:22:17 Read DevWorkspace at /devworkspace-metadata/flattened.devworkspace.yaml │
│ project-clone 2023/06/22 00:22:17 Processing project dotnet50-example │
│ project-clone 2023/06/22 00:22:17 Cloning project dotnet50-example to /projects/project-clone-1697882419/dotnet50-example │
│ project-clone 2023/06/22 00:22:17 DOING A GIT SPARSE CHECKOUT SHELL COMMAND │
│ project-clone Cloning into '/projects/project-clone-1697882419/dotnet50-example'... │
│ project-clone 2023/06/22 00:22:19 Cloned project dotnet50-example to /projects/project-clone-1697882419/dotnet50-example │
│ project-clone 2023/06/22 00:22:19 Setting up sparse checkout for project dotnet50-example │
│ project-clone 2023/06/22 00:22:19 Setting up remotes for project dotnet50-example │
│ project-clone 2023/06/22 00:22:20 Fetched remote origin at https://github.com/redhat-developer/s2i-dotnetcore-ex │
│ project-clone 2023/06/22 00:22:20 Checking out remote branch dotnet-5.0 │
│ project-clone Switched to a new branch 'dotnet-5.0' │
│ project-clone branch 'dotnet-5.0' set up to track 'origin/dotnet-5.0'. │
│ project-clone 2023/06/22 00:22:20 Moving cloned project dotnet50-example from temporary dir /projects/project-clone-1697882419/dotnet50-example to /projects/dotnet50-example
One note that could be for a future improvement, or as part of this PR (though solving this seems like it'd take more effort than it's worth): if you specify an invalid subDir for the starterProject, the project will still be cloned and no errors/warnings will be logged.
However, any directories belonging to the git repo won't be cloned.
For example, setting subDir to some non-existing directory name with the dotnet50 devworkspace that I provided will result in app folder from the repository being missing, as shown:
bash-4.4 /projects/dotnet50-example $ ls -la
total 32
drwxr-sr-x. 1 user user 148 Jun 22 00:23 .
drwxrwsrwx. 1 root user 32 Jun 22 00:23 ..
-rw-r--r--. 1 user user 328 Jun 22 00:23 .editorconfig
drwxr-sr-x. 1 user user 188 Jun 22 00:24 .git
-rw-r--r--. 1 user user 102 Jun 22 00:23 .gitattributes
-rw-r--r--. 1 user user 83 Jun 22 00:23 .mailmap
-rw-r--r--. 1 user user 2568 Jun 22 00:23 Contributing.adoc
-rw-r--r--. 1 user user 11357 Jun 22 00:23 LICENSE
-rw-r--r--. 1 user user 1918 Jun 22 00:23 README.adoc
bash-4.4 /projects/dotnet50-example $ git status
On branch dotnet-5.0
Your branch is up to date with 'origin/dotnet-5.0'.
You are in a sparse checkout with 11% of tracked files present.
nothing to commit, working tree cleanSolving this would probably require validating that subDir exisits as a directory within the repo, and then falling back to a normal (non-sparse) git clone if the subDir doesn't exist in the repo.
| return fmt.Errorf("failed to sparsely git clone from %s: %s", defaultRemoteURL, err) | ||
| } | ||
| } else { | ||
| // Delegate to standard git binary because git.PlainClone takes a lot of memory for large repos |
There was a problem hiding this comment.
I don't think git.PlainClone is being used anywhere in the DWO repo since f281fd6#diff-d1d2dba91d4290537953cc71ad75775637541a2eff4062bf6fedc2632095d34dL67, so it might be worth just removing this comment
There was a problem hiding this comment.
I think the comment is still useful as it warns of a major pitfall in switching to something like PlainClone (namely, dramatically higher memory usage). While git.PlainClone is no longer used, the go-git library is still in use within project-clone.
|
Sidenote: I was a bit confused about how |
amisevsk
force-pushed
the
starter-projects-support
branch
from
efbb9fe to
cee3fda
Compare
amisevsk
force-pushed
the
starter-projects-support
branch
2 times, most recently
from
1f1cd20 to
c2cd57f
Compare
|
Re-reading the subDir documentation after @AObuchow pointed at that issue, I realized that the approach in this PR for subDir was actually incorrect. I've pushed the following changes:
@AObuchow please verify the updated behaviour for subDir when you have a moment. The new behavior should be
This new subDir behaviour is roughly equivalent to "download just one directory from a git repository" |
There was a problem hiding this comment.
Looks good to me, see minor comment 👍
Tested on OpenShift 4.12. During normal behaviour of subDir, the correct directory was put into the project folder and the directory wasn't a git repo, as expected.
I also tried giving an invalid/non-existent subdirectory, which resulted in the following logs (which were also placed into /projects/project-clone-errors.log:
2023/06/26 22:19:02 Using temporary directory /projects/project-clone-1166241122
2023/06/26 22:19:02 Read DevWorkspace at /devworkspace-metadata/flattened.devworkspace.yaml
2023/06/26 22:19:02 Processing project dotnet50-example
2023/06/26 22:19:02 Cloning project dotnet50-example to /projects/project-clone-1166241122/dotnet50-example
Cloning into '/projects/project-clone-1166241122/dotnet50-example'...
2023/06/26 22:19:02 Cloned project dotnet50-example to /projects/project-clone-1166241122/dotnet50-example
2023/06/26 22:19:02 Setting up remotes for project dotnet50-example
2023/06/26 22:19:03 Fetched remote origin at https://github.com/redhat-developer/s2i-dotnetcore-ex
2023/06/26 22:19:03 Checking out remote branch dotnet-5.0
Switched to a new branch 'dotnet-5.0'
branch 'dotnet-5.0' set up to track 'origin/dotnet-5.0'.
2023/06/26 22:19:03 Moving subdirectory invalid in project dotnet50-example from temporary directory to /projects/dotnet50-example
2023/06/26 22:19:03 Encountered error while setting up project dotnet50-example: failed to move subdirectory of cloned project to /projects: rename /projects/project-clone-1166241122/dotnet50-example/invalid /projects/dotnet50-example: no such file or directory
project-clone/internal/git/setup.go
Outdated
| var err error | ||
| subDirSubPath := project.Attributes.GetString(internal.ProjectSubDir, &err) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to projects subDir on project: %w", err) |
There was a problem hiding this comment.
typo projects -> process
There was a problem hiding this comment.
Definitely a melted brain moment :D
Thanks!
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: amisevsk, AObuchow 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:
Approvers can indicate their approval by writing |
|
I also tested using sparseCheckout as a project attribute, which worked as expected |
Add attribute 'controller.devfile.io/use-starter-project' to allow selecting which starterProject should be cloned and update project clone library to validate attribute. Signed-off-by: Angel Misevski <[email protected]>
Configure the project-clone init container to process the 'controller.devfile.io/use-starter-project' attribute and clone the selected starter project if applicable. Signed-off-by: Angel Misevski <[email protected]>
Simplify git operations in project clone to use the '-C' argument to run commands in another directory rather than switching working directories before running each command. Signed-off-by: Angel Misevski <[email protected]>
Add support for using sparse-checkouts in project-clone in order to
support the subDir field on starterProjects.
Since starterProjects are converted into projects within project clone
(to ease processing), the subDir field is processed as an attribute
('subDir') on projects.
Signed-off-by: Angel Misevski <[email protected]>
Signed-off-by: Angel Misevski <[email protected]>
Signed-off-by: Angel Misevski <[email protected]>
The subDir field on starterProjects is intended to take one directory out of a starterProject rather than perform a sparse checkout. The previous subDir handling as been reworked into a regular sparse-checkout by renaming the attribute 'sparseCheckout'. The 'subDir' attribute now configures the project-clone container to 1. Clone the project to a temporary directory, set up remotes, check out a branch, etc. 2. Copy _the subdirectory specified by subDir_ from that project into $PROJECTS_ROOT This results in a folder within $PROJECTS_ROOT that is not a git repository and only contains the contents of that subfolder. Signed-off-by: Angel Misevski <[email protected]>
Signed-off-by: Angel Misevski <[email protected]>
Signed-off-by: Angel Misevski <[email protected]>
amisevsk
force-pushed
the
starter-projects-support
branch
from
c2cd57f to
fecea96
Compare
|
New changes are detected. LGTM label has been removed. |
What does this PR do?
See #1130 (comment) for additional information; initially starterProject's
subDirfield was handled incorrectly.Add attribute
controller.devfile.io/use-starter-project(naming suggestions welcome) which can be applied in the top-level attributes of a devfile/DevWorkspace to select a starter project from the.spec.template.starterProjectsarray. If a starterProject is selected in this way, it is converted internally (within project-clone) to a regular project and processed alongside any other projects defined in the DevWorkspace.In order to support the
subDirfield on starterProjects, project-clone can now also perform sparse checkouts (though this is still not user-facing for regular projects at the moment).I also simplified the git operations in the project clone directory -- we don't need to change dirs before running commands and can instead use the
-Ccommandline option togit.A future improvement would be to expose
selectedStarterProjectas a DevWorkspace template field, rather than relying on the attribute.What issues does this PR fix or reference?
Closes #1084
Is it tested? How?
Changes to the project-clone container are pushed to
quay.io/amisevsk/project-clone:starter-projectsPR contains a devfile useful for basic smoke testing of starter projects support:
project-clone/test/starter-project-test.devworkspace.yamlFor manual verification, various devfiles in https://registry.devfile.io/viewer contain starter projects (e.g. Go, which is a basic example, or .NET 5.0 which includes a subDir. To make testing devfiles from the registry a little simpler, you can use the
yqscript:(you will need to add the
controller.devfile.io/use-starter-projectattribute to actually select a project)PR Checklist
/test v8-devworkspace-operator-e2e, v8-che-happy-pathto trigger)v8-devworkspace-operator-e2e: DevWorkspace e2e testv8-che-happy-path: Happy path for verification integration with Che