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

Adds odo dev --no-watch #5675

Merged
merged 7 commits into from
Apr 22, 2022
Merged

Adds odo dev --no-watch #5675

merged 7 commits into from
Apr 22, 2022

Conversation

dharmit
Copy link
Member

@dharmit dharmit commented Apr 19, 2022

What type of PR is this:

/kind feature

What does this PR do / why we need it:
Need this to allow users to avoid syncing the code too fast

Which issue(s) this PR fixes:
Fixes #5631

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:

  1. Execute odo dev --no-watch; modify a file in the component directory.
  2. odo should not trigger a sync for the file change
  3. Hitting Ctrl+c should correctly delete the resources from the cluster

@openshift-ci openshift-ci bot added the kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation label Apr 19, 2022
@netlify
Copy link

netlify bot commented Apr 19, 2022

Deploy Preview for odo-docusaurus-preview canceled.

Name Link
🔨 Latest commit 1fbef81
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/6261189ad21aab00085ad042

@odo-robot
Copy link

odo-robot bot commented Apr 19, 2022

OpenShift Tests on commit 124d27d finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Apr 19, 2022

Validate Tests on commit 124d27d finished successfully.
View logs: TXT HTML

@dharmit dharmit requested review from feloy, valaparthvi, rm3l and cdrage and removed request for mohammedzee1000, rnapoles-rh and feloy April 19, 2022 15:43
@dharmit
Copy link
Member Author

dharmit commented Apr 20, 2022

/test unit

Looks like network issue

 go mod verify
go: github.com/Netflix/[email protected]: Get "https://proxy.golang.org/github.com/%21netflix/go-expect/@v/v0.0.0-20201125194554-85d881c3777e.mod": dial tcp: i/o timeout
make[1]: *** [check-vendor] Error 1
make[1]: Leaving directory `/go/src/github.com/redhat-developer/odo'
make: *** [openshiftci-presubmit-unittests] Error 2 

pkg/watch/interface.go Outdated Show resolved Hide resolved
tests/integration/devfile/cmd_dev_test.go Outdated Show resolved Hide resolved
pkg/watch/interface.go Outdated Show resolved Hide resolved
Copy link
Member

@rm3l rm3l left a comment

Choose a reason for hiding this comment

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

One last thing I noticed after testing your branch. The color of the last message (Press Ctrl+c to exit `odo dev` and delete resources from the cluster) remains green, which might cause visual confusion with the list of ports forwarded.

image

@odo-robot
Copy link

odo-robot bot commented Apr 20, 2022

Unit Tests on commit 124d27d finished successfully.
View logs: TXT HTML

@rm3l
Copy link
Member

rm3l commented Apr 20, 2022

One last thing I noticed after testing your branch. The color of the last message (Press Ctrl+c to exit `odo dev` and delete resources from the cluster) remains green, which might cause visual confusion with the list of ports forwarded.

Weird, I launched the same command again, and now the message shows up as white 🤔

image

@dharmit dharmit requested review from valaparthvi and rm3l April 20, 2022 09:35
@odo-robot
Copy link

odo-robot bot commented Apr 20, 2022

Kubernetes Tests on commit 124d27d finished successfully.
View logs: TXT HTML

pkg/odo/cli/dev/dev.go Outdated Show resolved Hide resolved
@odo-robot
Copy link

odo-robot bot commented Apr 20, 2022

Windows Tests (OCP) on commit 124d27d finished with errors.
View logs: TXT HTML

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Apr 20, 2022
@dharmit
Copy link
Member Author

dharmit commented Apr 20, 2022

Seeing a panic in integration tests running on Windows.

 V  Syncing files into the container [285ms]
[5] [odo] I0420 07:08:40.197473    8304 exec.go:26] Executing command [/opt/odo/bin/supervisord ctl status] for pod: gnlyol-app-f99d96ccb-7rg69 in container: runtime
[5] [odo] devrun                           RUNNING   pid 16, uptime 0:00:01
[5] [odo] debugrun                         STOPPED   
[5] [odo] I0420 07:08:40.478338    8304 pods.go:29] Waiting for component=gnlyol pod
[5] [odo] I0420 07:08:40.482489    8304 pods.go:59] Status of gnlyol-app-f99d96ccb-7rg69 pod is Running
[5] [odo] I0420 07:08:40.482576    8304 pods.go:63] Pod Conditions: {"type":"Initialized","status":"True","lastProbeTime":null,"lastTransitionTime":"2022-04-20T12:08:38Z"}
[5] [odo] I0420 07:08:40.482576    8304 pods.go:63] Pod Conditions: {"type":"Ready","status":"True","lastProbeTime":null,"lastTransitionTime":"2022-04-20T12:08:39Z"}
[5] [odo] I0420 07:08:40.482576    8304 pods.go:63] Pod Conditions: {"type":"ContainersReady","status":"True","lastProbeTime":null,"lastTransitionTime":"2022-04-20T12:08:39Z"}
[5] [odo] I0420 07:08:40.482576    8304 pods.go:63] Pod Conditions: {"type":"PodScheduled","status":"True","lastProbeTime":null,"lastTransitionTime":"2022-04-20T12:08:36Z"}
[5] [odo] I0420 07:08:40.482745    8304 pods.go:68] Container Status: {"name":"runtime","state":{"running":{"startedAt":"2022-04-20T12:08:39Z"}},"lastState":{},"ready":true,"restartCount":0,"image":"registry.access.redhat.com/ubi8/nodejs-12:1-36","imageID":"registry.access.redhat.com/ubi8/nodejs-12@sha256:c691d5de6ddefe45bec53a35f6bc54a0fe420960196ddb12656c6f9331a39c03","containerID":"cri-o://e6d83d9e0999927863a3b5cc6a9519ac3cec4af3844a72e0d41f45d1b7771ed3","started":true}
[5] [odo] I0420 07:08:40.482861    8304 pods.go:72] Pod gnlyol-app-f99d96ccb-7rg69 is Running
[5] [odo] I0420 07:08:40.482861    8304 generic.go:130] supervisord stop command to restart or start other command
[5] [odo] I0420 07:08:40.482861    8304 exec.go:26] Executing command [/bin/sh -c cd ${PROJECTS_ROOT} && npm install] for pod: gnlyol-app-f99d96ccb-7rg69 in container: runtime
[5] [odo]  -  Building your application in container on cluster  ...
[5] [odo] npm notice created a lockfile as package-lock.json. You should commit this file.
[5] [odo] added 66 packages from 58 contributors and audited 66 packages in 4.41s
[5] [odo] panic: runtime error: invalid memory address or nil pointer dereference
[5] [odo] [signal 0xc0000005 code=0x0 addr=0x0 pc=0x8cfd47]
[5] [odo] 
[5] [odo] goroutine 436 [running]:
[5] [odo] github.com/redhat-developer/odo/pkg/devfile/adapters/common.startReaderGoroutine.func1(0x2bf5200, 0xc000242700, 0x0, 0xc0007a5c70, 0x0, 0xc0009b2180)
[5] [odo] 	C:/Users/Administrator.ANSIBLE-TEST-VS/odo/pkg/devfile/adapters/common/exec.go:69 +0x205
[5] [odo] created by github.com/redhat-developer/odo/pkg/devfile/adapters/common.startReaderGoroutine
[5] [odo] 	C:/Users/Administrator.ANSIBLE-TEST-VS/odo/pkg/devfile/adapters/common/exec.go:57 +0x9d
[5] Deleting project: cmd-dev-test1208eip
[5] Running odo.exe with args [odo project delete cmd-dev-test1208eip -f]
[5] [odo] I0420 07:12:37.520159    1992 util.go:767] Cached response used.
[5] [odo] I0420 07:12:37.622220    1992 kclient.go:272] Checking if "projects" resource is supported
[5] [odo] I0420 07:12:37.702657    1992 kclient.go:272] Checking if "projects" resource is supported
[5] [odo]  V  Deleted project : cmd-dev-test1208eip
[5] [odo]  !  Warning! Projects are asynchronously deleted from the cluster. odo does its best to delete the project. Due to multi-tenant clusters, the project may still exist on a different node.
[5] [odo] I0420 07:12:37.766171    1992 odo.go:75] Could not get the latest release information in time. Never mind, exiting gracefully :)
[5] Setting current dir to: C:\Users\Administrator.ANSIBLE-TEST-VS\637\tests\integration\devfile
[5] Deleting dir: C:\Users\Administrator.ANSIBLE-TEST-VS\AppData\Local\Temp\761339394
[5] Deleting dir: C:\Users\Administrator.ANSIBLE-TEST-VS\AppData\Local\Temp\087345529

@dharmit
Copy link
Member Author

dharmit commented Apr 21, 2022

Seeing a panic in integration tests running on Windows.

Tried odo dev --no-watch manually on Windows, it works fine. Not sure what's the problem. 😕

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Apr 21, 2022
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@dharmit
Copy link
Member Author

dharmit commented Apr 21, 2022

Weirder than before. It can't find --no-watch flag! 😕

[14] [odo] Your new component 'qaymxq' is ready in the current directory.
[14] [odo] To start editing your component, use 'odo dev' and open this folder in your favorite IDE.
[14] [odo] Changes will be directly reflected on the cluster.
[14] Running odo.exe with args [odo dev --random-ports --no-watch]
[14] [odo] Error: unknown flag: --no-watch
[14] [odo] Usage:
[14] [odo]   odo dev [flags]
[14] [odo] 
[14] [odo] Examples:
[14] [odo]   # Deploy component to the development cluster
[14] [odo]   odo dev
[14] [odo] 
[14] [odo] Flags:
[14] [odo]   -h, --help           Help for dev
[14] [odo]   -f, --random-ports   Assign random ports to redirected ports
[14] [odo] 
[14] [odo] Additional Flags:
[14] [odo]       --kubeconfig string    Paths to a kubeconfig. Only required if out-of-cluster.
[14] [odo]   -v, --v Level              Number for the log level verbosity. Level varies from 0 to 9 (default 0).
[14] [odo]       --vmodule moduleSpec   Comma-separated list of pattern=N settings for file-filtered logging
[14] [odo] 
[14] [odo]  X  unknown flag: --no-watch

@dharmit
Copy link
Member Author

dharmit commented Apr 21, 2022

@rm3l @kadel all three of us have played with this PR on the Windows, and it's working fine/as expected. Can you folks help merge this PR?

@rm3l
Copy link
Member

rm3l commented Apr 21, 2022

Weirder than before. It can't find --no-watch flag! confused

[14] [odo] Your new component 'qaymxq' is ready in the current directory.
[14] [odo] To start editing your component, use 'odo dev' and open this folder in your favorite IDE.
[14] [odo] Changes will be directly reflected on the cluster.
[14] Running odo.exe with args [odo dev --random-ports --no-watch]
[14] [odo] Error: unknown flag: --no-watch
[14] [odo] Usage:
[14] [odo]   odo dev [flags]
[14] [odo] 
[14] [odo] Examples:
[14] [odo]   # Deploy component to the development cluster
[14] [odo]   odo dev
[14] [odo] 
[14] [odo] Flags:
[14] [odo]   -h, --help           Help for dev
[14] [odo]   -f, --random-ports   Assign random ports to redirected ports
[14] [odo] 
[14] [odo] Additional Flags:
[14] [odo]       --kubeconfig string    Paths to a kubeconfig. Only required if out-of-cluster.
[14] [odo]   -v, --v Level              Number for the log level verbosity. Level varies from 0 to 9 (default 0).
[14] [odo]       --vmodule moduleSpec   Comma-separated list of pattern=N settings for file-filtered logging
[14] [odo] 
[14] [odo]  X  unknown flag: --no-watch

Indeed, this is strange. As you said, it looks like a different binary was used. I can see the version is v2.5.0 in the CI job logs, while it should now be v3.0.0-alpha1 (since 946385b and #5677, and given that you rebased your branch onto main):

[14] [odo] I0421 03:47:24.159538   10020 implem.go:103] The path for preference file is C:\Users\Administrator.ANSIBLE-TEST-VS\AppData\Local\Temp\238738076\preference.yaml
[14] [odo]   __
[14] [odo]  /  \__     Initializing new component
[14] [odo]  \__/  \    
[14] [odo]  /  \__/    odo version: v2.5.0
[14] [odo]  \__/
[14] [odo] 
[14] [odo] Interactive mode enabled, please answer the following questions:
[14] [odo]  -  Copying devfile from "C:\\Users\\Administrator.ANSIBLE-TEST-VS\\651\\tests\\examples\\source\\devfiles\\nodejs\\devfile.yaml"  ...
[14] [odo]

LGTM after retesting your changes, but maybe we should create a separate issue for this.

/lgtm
/approve

EDIT: I created #5683 to keep track of the binary issue with the Windows tests.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Apr 21, 2022
@openshift-ci
Copy link

openshift-ci bot commented Apr 21, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rm3l

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Apr 21, 2022
@rm3l
Copy link
Member

rm3l commented Apr 21, 2022

Don't know why, but a lot of unit tests CI jobs are currently Queued in IBM Cloud. And this is blocking this PR from getting merged. Is there any way to dequeue them?

@dharmit
Copy link
Member Author

dharmit commented Apr 22, 2022

Overriding because unit tests already succeeded using prow

/override Unit Tests

@openshift-ci
Copy link

openshift-ci bot commented Apr 22, 2022

@dharmit: /override requires a failed status context or a job name to operate on.
The following unknown contexts were given:

  • Unit Tests

Only the following contexts were expected:

  • Kubernetes Integration Tests/Kubernetes Integration Tests
  • OpenShift Integration tests/OpenShift Integration tests
  • Unit Tests/Unit Tests
  • ci/prow/unit
  • ci/prow/v4.10-images
  • ci/prow/v4.10-integration-e2e
  • ci/prow/v4.11-images
  • netlify/odo-docusaurus-preview/deploy-preview
  • pull-ci-redhat-developer-odo-main-unit
  • pull-ci-redhat-developer-odo-main-v4.10-images
  • pull-ci-redhat-developer-odo-main-v4.10-integration-e2e
  • pull-ci-redhat-developer-odo-main-v4.11-images
  • tide
  • validator/Validate
  • windows-integration-test/Windows-test

In response to this:

Overriding because unit tests already succeeded using prow

/override Unit Tests

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.

@dharmit
Copy link
Member Author

dharmit commented Apr 22, 2022

/override Unit Tests/Unit Tests

@openshift-ci
Copy link

openshift-ci bot commented Apr 22, 2022

@dharmit: Overrode contexts on behalf of dharmit: Unit Tests/Unit Tests

In response to this:

/override Unit Tests/Unit Tests

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.

@dharmit
Copy link
Member Author

dharmit commented Apr 22, 2022

/override windows-integration-test/Windows-test

@openshift-ci
Copy link

openshift-ci bot commented Apr 22, 2022

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

In response to this:

/override windows-integration-test/Windows-test

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.

@dharmit
Copy link
Member Author

dharmit commented Apr 22, 2022

Don't know why, but a lot of unit tests CI jobs are currently Queued in IBM Cloud. And this is blocking this PR from getting merged. Is there any way to dequeue them?

I have overridden unit tests since the counterparts are already passing on prow. And for Windows, well, overrode that as well. Let's see.

@openshift-merge-robot openshift-merge-robot merged commit 58a0481 into redhat-developer:main Apr 22, 2022
@dharmit dharmit deleted the fix-5631 branch April 22, 2022 06:17
cdrage pushed a commit to cdrage/odo that referenced this pull request Aug 31, 2022
* Adds `odo dev --no-watch`

* Remove select for a single case

More details on - dominikh/go-tools#503 (comment)

* Update mocks

* Update tests/integration/devfile/cmd_dev_test.go

Co-authored-by: Armel Soro <[email protected]>

* Rename CleanupFunc to Cleanup

* Add usage doc for --no-watch

* Remove infinite for loop

Co-authored-by: Armel Soro <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

odo dev --no-watch
4 participants