Fix: Starterproject not replace existing devfile stack files of the context dir #6633

valaparthvi · 2023-03-06T10:21:22Z

What type of PR is this:
/kind bug

What does this PR do / why we need it:
This PR ensures that downloading a starter project does not replace an existing stack files downloading as a part of the Devfile.
Which issue(s) this PR fixes:

Fixes #6489

PR acceptance criteria:

Unit test
Integration test
Documentation

How to test changes / Special notes to the reviewer:
0. Setup a custom devfile registry by using quay.io/pvala18/devfile-index:latest and go Devfile.

Running the odo init --devfile go --devfile-version latest --starter go-starter --name my-go-app should show the following result:

➜  ls
README.md    devfile.yaml docker       go.mod       kubernetes   main.go

Here docker and kubernetes are a part of the Devfile stack.
2. Run odo init --devfile go --devfile-version latest --starter conflicting-starter-project-demo --name my-go-app. A CONFLICT_STARTER_PROJECT should have been created containing the starter project and a warning must be printed listing the conflicting files.
3. Run odo init --devfile go --devfile-version latest --starter starterproject-with-devfile-demo --name my-go-app. Downloading the starter project should replace devfile.yaml and the stack files.

netlify · 2023-03-06T10:21:54Z

✅ Deploy Preview for odo-docusaurus-preview canceled.

Name	Link
🔨 Latest commit	`d9f25dc`
🔍 Latest deploy log	https://app.netlify.com/sites/odo-docusaurus-preview/deploys/641996fb3ff90500081a7171

odo-robot · 2023-03-06T10:27:46Z

OpenShift Unauthenticated Tests on commit 9c1a3f4 finished successfully.
View logs: TXT HTML

odo-robot · 2023-03-06T10:28:04Z

NoCluster Tests on commit 9c1a3f4 finished successfully.
View logs: TXT HTML

odo-robot · 2023-03-06T10:30:09Z

Unit Tests on commit 9c1a3f4 finished successfully.
View logs: TXT HTML

odo-robot · 2023-03-06T11:18:50Z

Validate Tests on commit 9c1a3f4 finished successfully.
View logs: TXT HTML

odo-robot · 2023-03-06T11:27:14Z

Windows Tests (OCP) on commit 9c1a3f4 finished successfully.
View logs: TXT HTML

odo-robot · 2023-03-06T12:03:06Z

Kubernetes Tests on commit 9c1a3f4 finished successfully.
View logs: TXT HTML

odo-robot · 2023-03-06T12:13:26Z

Kubernetes Docs Tests on commit f543afa finished successfully.
View logs: TXT HTML

odo-robot · 2023-03-06T12:42:37Z

OpenShift Tests on commit 9c1a3f4 finished successfully.
View logs: TXT HTML

valaparthvi · 2023-03-13T11:54:38Z

/override Kubernetes-Integration-Tests/Kubernetes-Integration-Tests

  Expected
      <string>:  âš   Pod is Terminating
       âœ—  Finished executing the application (command: devrun) [43s]
       âš   No pod exists
       âš   Pod is Pending
       âœ“  Pod is Running
       â€¢  Syncing files into the container  ...
      
 âœ“  Syncing files into the container [501ms]
       â€¢  Building your application in container (command: devbuild)  ...
      
 âœ“  Building your application in container (command: devbuild) [3s]
       â€¢  Executing the application (command: devrun)  ...
       -  Forwarding from 127.0.0.1:39429 -> 3000
      
      
      â†ª Dev mode
       Keyboard Commands:
      [Ctrl+c] - Exit and delete resources from the cluster
           [p] - Manually apply local changes to the application on the cluster
      
  To satisfy at least one of these matchers: [%!s(*matchers.ContainSubstringMatcher=&{Pushing files... []}) %!s(*matchers.ContainSubstringMatcher=&{Updating Component... []})]
  In [BeforeEach] at: /go/odo_1/tests/helper/helper_run.go:64
------------------------------
â€¢â€¢â€¢â€¢â€¢â€¢â€¢


Summarizing 1 Failure:
  [FAIL] odo dev command tests port-forwarding for the component when devfile has single endpoint when running odo dev when modifying memoryLimit for container in Devfile [BeforeEach] should react on the Devfile modification
  /go/odo_1/tests/helper/helper_run.go:64

openshift-ci · 2023-03-13T11:54:43Z

@valaparthvi: Overrode contexts on behalf of valaparthvi: Kubernetes-Integration-Tests/Kubernetes-Integration-Tests

In response to this:

/override Kubernetes-Integration-Tests/Kubernetes-Integration-Tests

 Expected
     <string>:  âš   Pod is Terminating
      âœ—  Finished executing the application (command: devrun) [43s]
      âš   No pod exists
      âš   Pod is Pending
      âœ“  Pod is Running
      â€¢  Syncing files into the container  ...
     
âœ“  Syncing files into the container [501ms]
      â€¢  Building your application in container (command: devbuild)  ...
     
âœ“  Building your application in container (command: devbuild) [3s]
      â€¢  Executing the application (command: devrun)  ...
      -  Forwarding from 127.0.0.1:39429 -> 3000
     
     
     â†ª Dev mode
      Keyboard Commands:
     [Ctrl+c] - Exit and delete resources from the cluster
          [p] - Manually apply local changes to the application on the cluster
     
 To satisfy at least one of these matchers: [%!s(*matchers.ContainSubstringMatcher=&{Pushing files... []}) %!s(*matchers.ContainSubstringMatcher=&{Updating Component... []})]
 In [BeforeEach] at: /go/odo_1/tests/helper/helper_run.go:64
------------------------------
â€¢â€¢â€¢â€¢â€¢â€¢â€¢


Summarizing 1 Failure:
 [FAIL] odo dev command tests port-forwarding for the component when devfile has single endpoint when running odo dev when modifying memoryLimit for container in Devfile [BeforeEach] should react on the Devfile modification
 /go/odo_1/tests/helper/helper_run.go:64

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

valaparthvi · 2023-03-13T11:55:34Z

/override windows-integration-test/Windows-test

  Expected
      <*url.Error | 0xc0002ec8a0>: {
          Op: "Post",
          URL: "http://127.0.0.1:64534/api/newuser",
          Err: <*errors.errorString | 0xc000088130>{s: "EOF"},
      }
  to be nil
  In [It] at: C:/Users/Administrator.ANSIBLE-TEST-VS/3416/tests/e2escenarios/e2e_test.go:306

  There were additional failures detected after the initial failure.  Here's a summary - for full details run Ginkgo in verbose mode:
    [FAILED] in [AfterEach] at C:/Users/Administrator.ANSIBLE-TEST-VS/3416/tests/helper/helper_filesystem.go:48
------------------------------


Summarizing 1 Failure:
  [FAIL] E2E Test starting with non-empty Directory add Binding [It] should verify developer workflow of using binding as env in innerloop
  C:/Users/Administrator.ANSIBLE-TEST-VS/3416/tests/e2escenarios/e2e_test.go:306

openshift-ci · 2023-03-13T11:55:38Z

@valaparthvi: Overrode contexts on behalf of valaparthvi: windows-integration-test/Windows-test

In response to this:

/override windows-integration-test/Windows-test

 Expected
     <*url.Error | 0xc0002ec8a0>: {
         Op: "Post",
         URL: "http://127.0.0.1:64534/api/newuser",
         Err: <*errors.errorString | 0xc000088130>{s: "EOF"},
     }
 to be nil
 In [It] at: C:/Users/Administrator.ANSIBLE-TEST-VS/3416/tests/e2escenarios/e2e_test.go:306

 There were additional failures detected after the initial failure.  Here's a summary - for full details run Ginkgo in verbose mode:
   [FAILED] in [AfterEach] at C:/Users/Administrator.ANSIBLE-TEST-VS/3416/tests/helper/helper_filesystem.go:48
------------------------------


Summarizing 1 Failure:
 [FAIL] E2E Test starting with non-empty Directory add Binding [It] should verify developer workflow of using binding as env in innerloop
 C:/Users/Administrator.ANSIBLE-TEST-VS/3416/tests/e2escenarios/e2e_test.go:306

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

tests/integration/cmd_devfile_init_test.go

pkg/registry/registry.go

rm3l · 2023-03-16T17:39:39Z

pkg/util/util.go

@@ -228,7 +224,7 @@ func IsValidProjectDir(path string, devfilePath string) error {
 // takes an absolute path prefixed with file:// and extracts it to a destination.
 // pathToUnzip specifies the path within the zip folder to extract
 // TODO(feloy) sync with devfile library?
-func GetAndExtractZip(zipURL string, destination string, pathToUnzip string, starterToken string) error {
+func GetAndExtractZip(zipURL string, destination string, pathToUnzip string, starterToken string, fsys filesystem.Filesystem) error {


Can fsys be the first parameter of this function, just to be consistent with the other functions like IsValidProjectDir?

I've changed the position of fsys in IsValidProjectDir just to be more consistent with other functions.

pkg/util/util.go

Signed-off-by: Parthvi Vala <pvala@redhat.com>

valaparthvi · 2023-03-20T10:35:49Z

/override Kubernetes-Integration-Tests/Kubernetes-Docs-Integration-Tests

  user-guides/quickstart/docs-mdx/dotnet/dotnet_odo_dev_output.mdx
  Expected
      <string>:   (
        	"""
        	... // 9 identical lines
        	â€¢  Waiting for Kubernetes resources  ...
        	âš   Pod is Pending
      - 	âœ“  Pod is Running
        	âœ“  Syncing files into the container [1s]
        	âœ“  Building your application in container (command: build) [1s]
        	... // 9 identical lines
        	[Ctrl+c] - Exit and delete resources from the cluster
        	[p] - Manually apply local changes to the application on the cluster
      + 	âœ“  Pod is Running
        	```
        	"""
        )
      
  to be empty
  In [It] at: /go/odo_1/tests/documentation/user-guides/doc_user_guides_quickstart_test.go:208
------------------------------
Spawning '/go/bin/odo init' from /tmp/2337855403
â€¢â€¢Spawning '/go/bin/odo init' from /tmp/4264151494
â€¢Spawning '/go/bin/odo init' from /tmp/1110626714
â€¢


Summarizing 1 Failure:
  [FAIL] User guides: Quickstart test .NET quickstart guide [It] should test the complete dotnet quickstart output in order
  /go/odo_1/tests/documentation/user-guides/doc_user_guides_quickstart_test.go:208

openshift-ci · 2023-03-20T10:36:05Z

@valaparthvi: Overrode contexts on behalf of valaparthvi: Kubernetes-Integration-Tests/Kubernetes-Docs-Integration-Tests

In response to this:

/override Kubernetes-Integration-Tests/Kubernetes-Docs-Integration-Tests

 user-guides/quickstart/docs-mdx/dotnet/dotnet_odo_dev_output.mdx
 Expected
     <string>:   (
       	"""
       	... // 9 identical lines
       	â€¢  Waiting for Kubernetes resources  ...
       	âš   Pod is Pending
     - 	âœ“  Pod is Running
       	âœ“  Syncing files into the container [1s]
       	âœ“  Building your application in container (command: build) [1s]
       	... // 9 identical lines
       	[Ctrl+c] - Exit and delete resources from the cluster
       	[p] - Manually apply local changes to the application on the cluster
     + 	âœ“  Pod is Running
       	```
       	"""
       )
     
 to be empty
 In [It] at: /go/odo_1/tests/documentation/user-guides/doc_user_guides_quickstart_test.go:208
------------------------------
Spawning '/go/bin/odo init' from /tmp/2337855403
â€¢â€¢Spawning '/go/bin/odo init' from /tmp/4264151494
â€¢Spawning '/go/bin/odo init' from /tmp/1110626714
â€¢


Summarizing 1 Failure:
 [FAIL] User guides: Quickstart test .NET quickstart guide [It] should test the complete dotnet quickstart output in order
 /go/odo_1/tests/documentation/user-guides/doc_user_guides_quickstart_test.go:208

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

valaparthvi · 2023-03-20T10:37:47Z

/override windows-integration-test/Windows-test

  Expected
      <*url.Error | 0xc0005dac00>: {
          Op: "Post",
          URL: "http://127.0.0.1:61953/api/newuser",
          Err: <*errors.errorString | 0xc000088130>{s: "EOF"},
      }
  to be nil
  In [It] at: C:/Users/Administrator.ANSIBLE-TEST-VS/3429/tests/e2escenarios/e2e_test.go:306

  There were additional failures detected after the initial failure.  Here's a summary - for full details run Ginkgo in verbose mode:
    [FAILED] in [AfterEach] at C:/Users/Administrator.ANSIBLE-TEST-VS/3429/tests/helper/helper_filesystem.go:48
------------------------------


Summarizing 1 Failure:
  [FAIL] E2E Test starting with non-empty Directory add Binding [It] should verify developer workflow of using binding as env in innerloop
  C:/Users/Administrator.ANSIBLE-TEST-VS/3429/tests/e2escenarios/e2e_test.go:306

openshift-ci · 2023-03-20T10:37:51Z

@valaparthvi: Overrode contexts on behalf of valaparthvi: windows-integration-test/Windows-test

In response to this:

/override windows-integration-test/Windows-test

 Expected
     <*url.Error | 0xc0005dac00>: {
         Op: "Post",
         URL: "http://127.0.0.1:61953/api/newuser",
         Err: <*errors.errorString | 0xc000088130>{s: "EOF"},
     }
 to be nil
 In [It] at: C:/Users/Administrator.ANSIBLE-TEST-VS/3429/tests/e2escenarios/e2e_test.go:306

 There were additional failures detected after the initial failure.  Here's a summary - for full details run Ginkgo in verbose mode:
   [FAILED] in [AfterEach] at C:/Users/Administrator.ANSIBLE-TEST-VS/3429/tests/helper/helper_filesystem.go:48
------------------------------


Summarizing 1 Failure:
 [FAIL] E2E Test starting with non-empty Directory add Binding [It] should verify developer workflow of using binding as env in innerloop
 C:/Users/Administrator.ANSIBLE-TEST-VS/3429/tests/e2escenarios/e2e_test.go:306

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

pkg/registry/registry.go

Signed-off-by: Parthvi Vala <pvala@redhat.com>

…icting and non-conflicting cases Signed-off-by: Parthvi Vala <pvala@redhat.com>

rm3l

Thanks for the changes. LGTM overall. I had one last comment on error handling when walking the directory tree.

pkg/registry/registry.go

Signed-off-by: Parthvi Vala <pvala@redhat.com>

rm3l

Thanks for the changes.

rm3l

Whoops, sorry. I just approved this PR feature-wise, but noticed I had a lot of odostarterproject* and odo* non-empty folders filling up my temporary filesystem.

pkg/registry/registry.go

Signed-off-by: Parthvi Vala <pvala@redhat.com>

sonarqubecloud · 2023-03-21T11:38:41Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
3 Code Smells

No Coverage information
0.3% Duplication

rm3l

Thanks for your work on this!

openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. kind/bug Categorizes issue or PR as related to a bug. labels Mar 6, 2023

openshift-ci bot requested review from anandrkskd and feloy March 6, 2023 10:23

valaparthvi force-pushed the 6489_download_starter_project_replaces_existing_files branch from 3199f13 to ac36ffc Compare March 13, 2023 05:15

valaparthvi changed the title ~~[WIP] Fix: Starterproject not replace existing devfile stack files of the context dir~~ Fix: Starterproject not replace existing devfile stack files of the context dir Mar 13, 2023

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Mar 13, 2023

valaparthvi requested review from rm3l and removed request for anandrkskd March 13, 2023 11:55

rm3l requested changes Mar 16, 2023

View reviewed changes

openshift-ci bot assigned rm3l Mar 16, 2023

valaparthvi added 6 commits March 17, 2023 00:13

Starterproject does not replace existing files of the context dir

b7a4c1f

Signed-off-by: Parthvi Vala <pvala@redhat.com>

Fix issue with os.RemoveAll

b643c62

Signed-off-by: Parthvi Vala <pvala@redhat.com>

Fix errors, it works now

4eea9c7

Signed-off-by: Parthvi Vala <pvala@redhat.com>

Fix test failures

e8f9349

Signed-off-by: Parthvi Vala <pvala@redhat.com>

Use filesystem to perform file operations wherever possible

03d2cbb

Signed-off-by: Parthvi Vala <pvala@redhat.com>

Use filesystem to perform file operations wherever possible (2)

79f12e4

Signed-off-by: Parthvi Vala <pvala@redhat.com>

Change arg position in IsValidProjectDir

30bb75c

Signed-off-by: Parthvi Vala <pvala@redhat.com>

valaparthvi force-pushed the 6489_download_starter_project_replaces_existing_files branch from efc25e4 to 30bb75c Compare March 16, 2023 18:44

valaparthvi requested a review from rm3l March 20, 2023 10:57

rm3l requested changes Mar 20, 2023

View reviewed changes

pkg/registry/registry.go Outdated Show resolved Hide resolved

pkg/registry/registry.go Outdated Show resolved Hide resolved

pkg/registry/registry.go Outdated Show resolved Hide resolved

pkg/registry/registry.go Outdated Show resolved Hide resolved

valaparthvi added 3 commits March 20, 2023 18:18

rename isConflicting to getConflictingFiles

ce672eb

Signed-off-by: Parthvi Vala <pvala@redhat.com>

Add zip files for starterproject with empty dir

9a4748e

Signed-off-by: Parthvi Vala <pvala@redhat.com>

Add unit tests covering starterprojects with empty dir for both confl…

eeb059e

…icting and non-conflicting cases Signed-off-by: Parthvi Vala <pvala@redhat.com>

valaparthvi requested a review from rm3l March 20, 2023 14:57

rm3l requested changes Mar 20, 2023

View reviewed changes

pkg/registry/registry.go Show resolved Hide resolved

Handle error while walking

2330428

Signed-off-by: Parthvi Vala <pvala@redhat.com>

valaparthvi requested a review from rm3l March 20, 2023 18:35

rm3l approved these changes Mar 20, 2023

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Mar 20, 2023

rm3l requested changes Mar 20, 2023

View reviewed changes

pkg/registry/registry.go Show resolved Hide resolved

pkg/registry/registry.go Show resolved Hide resolved

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Mar 20, 2023

valaparthvi force-pushed the 6489_download_starter_project_replaces_existing_files branch from c826e28 to 101b895 Compare March 21, 2023 10:39

valaparthvi closed this Mar 21, 2023

valaparthvi reopened this Mar 21, 2023

valaparthvi closed this Mar 21, 2023

valaparthvi reopened this Mar 21, 2023

Delete temp dirs

d9f25dc

Signed-off-by: Parthvi Vala <pvala@redhat.com>

valaparthvi force-pushed the 6489_download_starter_project_replaces_existing_files branch from 101b895 to d9f25dc Compare March 21, 2023 11:37

rm3l approved these changes Mar 21, 2023

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Mar 21, 2023

openshift-merge-robot merged commit 47234b9 into redhat-developer:main Mar 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: Starterproject not replace existing devfile stack files of the context dir #6633

Fix: Starterproject not replace existing devfile stack files of the context dir #6633

valaparthvi commented Mar 6, 2023 •

edited

Loading

netlify bot commented Mar 6, 2023 •

edited

Loading

odo-robot bot commented Mar 6, 2023 •

edited

Loading

odo-robot bot commented Mar 6, 2023 •

edited

Loading

odo-robot bot commented Mar 6, 2023 •

edited

Loading

odo-robot bot commented Mar 6, 2023 •

edited

Loading

odo-robot bot commented Mar 6, 2023 •

edited

Loading

odo-robot bot commented Mar 6, 2023 •

edited

Loading

odo-robot bot commented Mar 6, 2023 •

edited

Loading

odo-robot bot commented Mar 6, 2023 •

edited

Loading

valaparthvi commented Mar 13, 2023

openshift-ci bot commented Mar 13, 2023

valaparthvi commented Mar 13, 2023

openshift-ci bot commented Mar 13, 2023

rm3l Mar 16, 2023

valaparthvi Mar 20, 2023

valaparthvi commented Mar 20, 2023

openshift-ci bot commented Mar 20, 2023

valaparthvi commented Mar 20, 2023

openshift-ci bot commented Mar 20, 2023

rm3l left a comment

rm3l left a comment

rm3l left a comment •

edited

Loading

sonarqubecloud bot commented Mar 21, 2023

rm3l left a comment

Fix: Starterproject not replace existing devfile stack files of the context dir #6633

Fix: Starterproject not replace existing devfile stack files of the context dir #6633

Conversation

valaparthvi commented Mar 6, 2023 • edited Loading

netlify bot commented Mar 6, 2023 • edited Loading

✅ Deploy Preview for odo-docusaurus-preview canceled.

odo-robot bot commented Mar 6, 2023 • edited Loading

odo-robot bot commented Mar 6, 2023 • edited Loading

odo-robot bot commented Mar 6, 2023 • edited Loading

odo-robot bot commented Mar 6, 2023 • edited Loading

odo-robot bot commented Mar 6, 2023 • edited Loading

odo-robot bot commented Mar 6, 2023 • edited Loading

odo-robot bot commented Mar 6, 2023 • edited Loading

odo-robot bot commented Mar 6, 2023 • edited Loading

valaparthvi commented Mar 13, 2023

openshift-ci bot commented Mar 13, 2023

valaparthvi commented Mar 13, 2023

openshift-ci bot commented Mar 13, 2023

rm3l Mar 16, 2023

Choose a reason for hiding this comment

valaparthvi Mar 20, 2023

Choose a reason for hiding this comment

valaparthvi commented Mar 20, 2023

openshift-ci bot commented Mar 20, 2023

valaparthvi commented Mar 20, 2023

openshift-ci bot commented Mar 20, 2023

rm3l left a comment

Choose a reason for hiding this comment

rm3l left a comment

Choose a reason for hiding this comment

rm3l left a comment • edited Loading

Choose a reason for hiding this comment

sonarqubecloud bot commented Mar 21, 2023

rm3l left a comment

Choose a reason for hiding this comment

valaparthvi commented Mar 6, 2023 •

edited

Loading

netlify bot commented Mar 6, 2023 •

edited

Loading

odo-robot bot commented Mar 6, 2023 •

edited

Loading

odo-robot bot commented Mar 6, 2023 •

edited

Loading

odo-robot bot commented Mar 6, 2023 •

edited

Loading

odo-robot bot commented Mar 6, 2023 •

edited

Loading

odo-robot bot commented Mar 6, 2023 •

edited

Loading

odo-robot bot commented Mar 6, 2023 •

edited

Loading

odo-robot bot commented Mar 6, 2023 •

edited

Loading

odo-robot bot commented Mar 6, 2023 •

edited

Loading

rm3l left a comment •

edited

Loading