Skip to content
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.

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

Re-enable support for the sparseCheckoutDir field in devfiles #3933

Closed
johnmcollier opened this issue Sep 11, 2020 · 15 comments · Fixed by #4120
Closed

Re-enable support for the sparseCheckoutDir field in devfiles #3933

johnmcollier opened this issue Sep 11, 2020 · 15 comments · Fixed by #4120
Assignees
Labels
area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)).
Projects

Comments

@johnmcollier
Copy link
Member

johnmcollier commented Sep 11, 2020

/kind bug
/area devfile
/area regression

What versions of software are you using?

Operating System:
macOS

Output of odo version:
master

How did you run odo exactly?

odo create java-openliberty --starter

Actual behavior

When cloning a starter project with the sparseCheckoutDir field set, odo create --starter now clones the entire source code repo specified in the starter project, rather than checking out the directory specified in sparseCheckoutDir.

As the java-openliberty devfile currently uses sparseCheckoutDir for its starterProject, we're currently unable to use odo create java-openliberty --starter and successfully deploy a component from it.

Expected behavior

odo create --starter respects the sparseCheckoutDir field in devfiles

Any logs, error output, etc?

Sample devfile:

schemaVersion: 2.0.0
metadata:
  name: nodejs
  version: 1.0.0
starterProjects:
  - name: nodejs-starter
    git:
      remotes:
        origin: "https://github.com/odo-devfiles/nodejs-ex.git"
      sparseCheckoutDir: /test
components:
  - name: runtime
    container:
      image: registry.access.redhat.com/ubi8/nodejs-12:1-45
      memoryLimit: 1024Mi
      mountSources: true
      sourceMapping: /project
      endpoints:
        - name: http-3000
          targetPort: 3000
commands:
  - id: install
    exec:
      component: runtime
      commandLine: npm install
      workingDir: /project
      group:
        kind: build
        isDefault: true
  - id: run
    exec:
      component: runtime
      commandLine: npm start
      workingDir: /project
      group:
        kind: run
        isDefault: true
@openshift-ci-robot openshift-ci-robot added kind/bug Categorizes issue or PR as related to a bug. area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. labels Sep 11, 2020
@adisky
Copy link
Contributor

adisky commented Sep 11, 2020

it has been discussed here #3867 (comment)

@johnmcollier
Copy link
Member Author

Thanks @adisky. I'll go update the issue title since its disabling was deliberate

In that case I think we need to do two things:

  1. Update the java-openliberty devfile to not rely on the sparseCheckoutDir
  2. (in the future) Re-introduce support for the sparseCheckoutDir

Have we looked at how Che handles the sparseCheckoutDir field? Maybe they have a better approach than what we were doing.

CC @kadel

@johnmcollier johnmcollier changed the title sparseCheckoutDir in devfiles no longer working Re-enable support for the sparseCheckoutDir field in devfiles Sep 11, 2020
@elsony
Copy link

elsony commented Sep 11, 2020

Disabling that will be a problem with the java-openliberty stack. @scottkurz @ajm01 This means that the liberty starter scenario will not work out of the box as before. You may consider to adjust the java-openliberty stack to workaround this problem for the time being.

@elsony elsony added the priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)). label Sep 11, 2020
@johnmcollier
Copy link
Member Author

johnmcollier commented Sep 11, 2020

+1 on updating the stack to workaround it for now. From Tomas' comment on the PR, it sounds like we weren't handling sparseCheckoutDir in a good way, so until we find a good way to get it working, we should make sure the java-openliberty stack is fixed at least

If it may take a while to re-introduce the sparseCheckoutDir functionality, we may also want to consider adding a warning message to odo create --starter when sparseCheckoutDir is present in a devfile

@scottkurz
Copy link
Contributor

scottkurz commented Sep 11, 2020

@johnmcollier @elsony - How would you suggest enabling the ability to have multiple starters?

It would be easy enough to switch to a new repository with a single starter and nothing else, but we intended to offer:

odo create java-openliberty --starter
odo create java-openliberty --starter OtherStarterProj

Can each individual starter get its own relevant config in the devfile?

(I tried it quick on something close to the odo2 beta and it always checks out the repo root no matter what the --starter arg).

@johnmcollier
Copy link
Member Author

johnmcollier commented Sep 14, 2020

@scottkurz So, devfiles can have multiple starter projects and your syntax (--starter <name>) is correct. For example:

starterProjects:
  - name: nodejs-starter
    git:
      remotes:
        origin: "https://github.com/odo-devfiles/nodejs-ex.git"
  - name: second-starter
    git:
      remotes:
        origin: "https://github.com/johnmcollier/nodejs-ex.git"
      sparseCheckoutDir: /test

odo create nodejs --starter nodejs-starter would checkout the entire repo and odo create nodejs --starter second-starter would checkout the test folder from https://github.com/johnmcollier/nodejs-ex.git (if the sparseCheckoutDir field was working)

@elsony
Copy link

elsony commented Sep 14, 2020

@scottkurz Just to be clear, to work around the current problem of sparseCheckoutDir not supported yet. You can still have multiple starter projects. For those different starter projects, you either:

  1. put the starter code under different repos and reference the starter projects using different repo
  2. put them under different branches of the same repo and use the revision in checkoutFrom to control what to checkout for each of the starter project (see Add an ability to configure multiple remotes for git project devfile/api#120 (comment) for example of the syntax)

@kadel
Copy link
Member

kadel commented Sep 15, 2020

Thanks @adisky. I'll go update the issue title since its disabling was deliberate

In that case I think we need to do two things:

  1. Update the java-openliberty devfile to not rely on the sparseCheckoutDir
  2. (in the future) Re-introduce support for the sparseCheckoutDir

Have we looked at how Che handles the sparseCheckoutDir field? Maybe they have a better approach than what we were doing.

CC @kadel

@johnmcollier, As far as I know, they don't handle that, as they don't have v2 support and this was not in v1 :-/
One solution for this could be to just clone the whole repo to a temporary directory and then copy just sparseCheckoutDir. This would work for starterProject but not for project as in that case, the directory should stay as a cloned git repo. And from what I read about sparse checkout it doesn't work like this.

Sparse checkout in git can populate only certain directories, but the directory structure remains.

For example, If I have a repo with two dirs /dir1/file1.txt and /dir2/file2.txt and I want to do sparse checkout if a /dir2 as git does it would end up with a directory with /dir2/file2.txt.
But what we want to do here is to end up with /file2.txt and that is not what sparse checkout does.

I would actually consider completely removing this field from Devfile, It will create a lot of confusion and problems.

Created devfile/api#128

@kadel
Copy link
Member

kadel commented Sep 15, 2020

  • put the starter code under different repos and reference the starter projects using different repo
  • put them under different branches of the same repo and use the revision in checkoutFrom to control what to checkout for each of the starter project (see devfile/api#120 (comment) for example of the syntax)

revision is not fully implemented yet :-(

The only way how we can fix it right now is to go with option 1 (separate repo)

@elsony
Copy link

elsony commented Sep 15, 2020

@scottkurz ^^ We only have option 1 for the time being.

@girishramnani girishramnani added this to For consideration in Sprint 190 via automation Sep 16, 2020
@girishramnani girishramnani moved this from For consideration to To do in Sprint 190 Sep 16, 2020
@devang-gaur
Copy link
Contributor

From what I could infer from the above discussion,

sparseCheckoutDir is being replaced by the subDir property. And the subDir property would try to comply with how git sparse checkout works ( https://github.blog/2020-01-17-bring-your-monorepo-down-to-size-with-sparse-checkout/ )

@kadel @girishramnani @johnmcollier @elsony can you reassure on this? please correct me if I'm wrong.

@kadel
Copy link
Member

kadel commented Sep 17, 2020

sparseCheckoutDir is being replaced by the subDir property. And the subDir property would try to comply with how git sparse checkout works ( https://github.blog/2020-01-17-bring-your-monorepo-down-to-size-with-sparse-checkout/ )

@kadel @girishramnani @johnmcollier @elsony can you reassure on this? please correct me if I'm wrong.

yes to first part no to second part :-)
It will be renamed to subDir because it won't comply with how git sparse checkout works.

@devang-gaur
Copy link
Contributor

Some more clarity now after a discussion with @kadel .. subDir property would simply clone the specified sub-directory on the project's base directory path.

Waiting on devfile/api#129 before moving ahead with this.

@girishramnani girishramnani added kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation and removed kind/bug Categorizes issue or PR as related to a bug. labels Sep 17, 2020
@girishramnani girishramnani moved this from To do to For consideration in Sprint 190 Sep 17, 2020
@girishramnani girishramnani added this to For consideration in Sprint 191 via automation Oct 5, 2020
@girishramnani girishramnani removed this from For consideration in Sprint 190 Oct 5, 2020
@girishramnani girishramnani added the triage/needs-information Indicates an issue needs more information in order to work on it. label Oct 5, 2020
@girishramnani girishramnani moved this from For consideration to To do in Sprint 191 Oct 5, 2020
@girishramnani girishramnani assigned mik-dass and unassigned adisky Oct 5, 2020
@mik-dass
Copy link
Contributor

mik-dass commented Oct 7, 2020

A function handling sparseCheckoutDir is already present in the master branch https://github.com/openshift/odo/blob/4bb02182db17c445ab5762d97486a7ca5aee886c/pkg/odo/cli/component/create.go#L1178 but currently is only used for zip based starterProjects

@mik-dass
Copy link
Contributor

mik-dass commented Oct 8, 2020

For git component, if a subDir is set, we will clone the entire directory to a temporary directory and copy the files inside the subDir directory and place it in the component directory.

@girishramnani girishramnani added points/3 and removed triage/needs-information Indicates an issue needs more information in order to work on it. labels Oct 8, 2020
@mik-dass mik-dass moved this from To do to In progress in Sprint 191 Oct 22, 2020
@mik-dass mik-dass moved this from In progress to For review in Sprint 191 Oct 22, 2020
@girishramnani girishramnani removed this from For review in Sprint 191 Oct 27, 2020
@girishramnani girishramnani added this to For consideration in Sprint 192 via automation Oct 27, 2020
@girishramnani girishramnani moved this from For consideration to For review in Sprint 192 Oct 27, 2020
Sprint 192 automation moved this from For review to Done Nov 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/devfile-spec Issues or PRs related to the Devfile specification and how odo handles and interprets it. kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation priority/High Important issue; should be worked on before any other issues (except priority/Critical issue(s)).
Projects
No open projects
Sprint 192
  
Done
Development

Successfully merging a pull request may close this issue.

9 participants