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

Simplify devfile Kubernetes adapter #6762

Conversation

feloy
Copy link
Contributor

@feloy feloy commented Apr 21, 2023

What type of PR is this:

/kind bug

What does this PR do / why we need it:

This PR makes some refactoring on the Dev code, to simplify the legacy code, to prepare the changes for #6671 :

  • Getting the values passed into the context
  • Moving Devfile param to WatchParams insted of being part of the Adapter structure, and build adapter only once

Which issue(s) this PR fixes:

Fixes partially #6671

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:

@feloy feloy changed the title Simplify devfile Kubernetes adapter [WIP] Simplify devfile Kubernetes adapter Apr 21, 2023
@openshift-ci openshift-ci bot added the kind/bug Categorizes issue or PR as related to a bug. label Apr 21, 2023
@netlify
Copy link

netlify bot commented Apr 21, 2023

Deploy Preview for odo-docusaurus-preview canceled.

Name Link
🔨 Latest commit c9c4258
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/6446816e8dd4500008efc588

@openshift-ci openshift-ci bot added 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 Apr 21, 2023
@odo-robot
Copy link

odo-robot bot commented Apr 21, 2023

OpenShift Unauthenticated Tests on commit 8efd414 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Apr 21, 2023

NoCluster Tests on commit 8efd414 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Apr 21, 2023

Unit Tests on commit 8efd414 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Apr 21, 2023

Validate Tests on commit 8efd414 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Apr 21, 2023

Kubernetes Tests on commit 8efd414 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Apr 21, 2023

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

@odo-robot
Copy link

odo-robot bot commented Apr 21, 2023

OpenShift Tests on commit 8efd414 finished successfully.
View logs: TXT HTML

@feloy feloy requested review from rm3l and removed request for kadel April 21, 2023 11:08
@feloy feloy changed the title [WIP] Simplify devfile Kubernetes adapter Simplify devfile Kubernetes adapter Apr 21, 2023
@openshift-ci 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 Apr 21, 2023
@feloy feloy force-pushed the refacto/simplify-devfile-adapter branch from aff78e7 to 6617dba Compare April 21, 2023 11:12
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Apr 24, 2023
@feloy feloy force-pushed the refacto/simplify-devfile-adapter branch from fa96f81 to dc6dbc8 Compare April 24, 2023 09:23
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. Required by Prow. label Apr 24, 2023
@feloy feloy closed this Apr 24, 2023
@feloy feloy reopened this Apr 24, 2023
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.

@feloy I was testing this PR and noticed there might be an issue in the logic for waiting until the application is listening on the expected ports prior to starting port forwarding.
I checked by delaying the start of my run command by 70 seconds (to check the timeout), but the spinner returned too quickly for me:

$ odo dev
  __
 /  \__     Developing using the "my-java-sb" Devfile
 \__/  \    Namespace: default
 /  \__/    odo version: v3.9.0
 \__/

 ⚠  You are using "default" namespace, odo may not work as expected in the default namespace.
 ⚠  You may set a new namespace by running `odo create namespace <name>`, or set an existing one by running `odo set namespace <name>`

↪ Running on the cluster in Dev mode
 •  Waiting for Kubernetes resources  ...
 ✓  Added storage m2 to component
 ⚠  Pod is Pending
 ✓  Pod is Running
 ✓  Syncing files into the container [116ms]
 ✓  Building your application in container (command: build) [45s]
 •  Executing the application (command: run)  ...
 ✓  Waiting for the application to be ready 
 -  Forwarding from 127.0.0.1:20001 -> 8080


↪ Dev mode
 Status:
 Watching for changes in the current directory /tmp/java-quarkus

I remember we discussed setting up port-forwarding as part of the code handling the infra, but that could come later, no?

@feloy
Copy link
Contributor Author

feloy commented Apr 24, 2023

@feloy I was testing this PR and noticed there might be an issue in the logic for waiting until the application is listening on the expected ports prior to starting port forwarding. I checked by delaying the start of my run command by 70 seconds (to check the timeout), but the spinner returned too quickly for me:

$ odo dev
  __
 /  \__     Developing using the "my-java-sb" Devfile
 \__/  \    Namespace: default
 /  \__/    odo version: v3.9.0
 \__/

 ⚠  You are using "default" namespace, odo may not work as expected in the default namespace.
 ⚠  You may set a new namespace by running `odo create namespace <name>`, or set an existing one by running `odo set namespace <name>`

↪ Running on the cluster in Dev mode
 •  Waiting for Kubernetes resources  ...
 ✓  Added storage m2 to component
 ⚠  Pod is Pending
 ✓  Pod is Running
 ✓  Syncing files into the container [116ms]
 ✓  Building your application in container (command: build) [45s]
 •  Executing the application (command: run)  ...
 ✓  Waiting for the application to be ready 
 -  Forwarding from 127.0.0.1:20001 -> 8080


↪ Dev mode
 Status:
 Watching for changes in the current directory /tmp/java-quarkus

I remember we discussed setting up port-forwarding as part of the code handling the infra, but that could come later, no?

This behaviour is not expected to have changed. Let me check

@feloy
Copy link
Contributor Author

feloy commented Apr 24, 2023

@feloy I was testing this PR and noticed there might be an issue in the logic for waiting until the application is listening on the expected ports prior to starting port forwarding. I checked by delaying the start of my run command by 70 seconds (to check the timeout), but the spinner returned too quickly for me:

$ odo dev
  __
 /  \__     Developing using the "my-java-sb" Devfile
 \__/  \    Namespace: default
 /  \__/    odo version: v3.9.0
 \__/

 ⚠  You are using "default" namespace, odo may not work as expected in the default namespace.
 ⚠  You may set a new namespace by running `odo create namespace <name>`, or set an existing one by running `odo set namespace <name>`

↪ Running on the cluster in Dev mode
 •  Waiting for Kubernetes resources  ...
 ✓  Added storage m2 to component
 ⚠  Pod is Pending
 ✓  Pod is Running
 ✓  Syncing files into the container [116ms]
 ✓  Building your application in container (command: build) [45s]
 •  Executing the application (command: run)  ...
 ✓  Waiting for the application to be ready 
 -  Forwarding from 127.0.0.1:20001 -> 8080


↪ Dev mode
 Status:
 Watching for changes in the current directory /tmp/java-quarkus

I remember we discussed setting up port-forwarding as part of the code handling the infra, but that could come later, no?

This behaviour is not expected to have changed. Let me check

I have pushed a fix. thanks for catching the issue :)

@rm3l
Copy link
Member

rm3l commented Apr 24, 2023

@feloy I was testing this PR and noticed there might be an issue in the logic for waiting until the application is listening on the expected ports prior to starting port forwarding. I checked by delaying the start of my run command by 70 seconds (to check the timeout), but the spinner returned too quickly for me:

$ odo dev
  __
 /  \__     Developing using the "my-java-sb" Devfile
 \__/  \    Namespace: default
 /  \__/    odo version: v3.9.0
 \__/

 ⚠  You are using "default" namespace, odo may not work as expected in the default namespace.
 ⚠  You may set a new namespace by running `odo create namespace <name>`, or set an existing one by running `odo set namespace <name>`

↪ Running on the cluster in Dev mode
 •  Waiting for Kubernetes resources  ...
 ✓  Added storage m2 to component
 ⚠  Pod is Pending
 ✓  Pod is Running
 ✓  Syncing files into the container [116ms]
 ✓  Building your application in container (command: build) [45s]
 •  Executing the application (command: run)  ...
 ✓  Waiting for the application to be ready 
 -  Forwarding from 127.0.0.1:20001 -> 8080


↪ Dev mode
 Status:
 Watching for changes in the current directory /tmp/java-quarkus

I remember we discussed setting up port-forwarding as part of the code handling the infra, but that could come later, no?

This behaviour is not expected to have changed. Let me check

I have pushed a fix. thanks for catching the issue :)

Works better - thanks!

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Apr 24, 2023
@valaparthvi valaparthvi reopened this Apr 24, 2023
@valaparthvi
Copy link
Contributor

valaparthvi commented Apr 24, 2023

OpenShift Failure:

Summarizing 7 Failures:
  [FAIL] odo delete command tests when a component is bootstrapped using a devfile.yaml with URI-referenced Kubernetes components when the component is running in both DEV and DEPLOY mode and dev mode is killed [BeforeEach] when the component is deleted using its name and namespace from another directory should delete the appropriate resources
  /go/odo_1/tests/helper/helper_run.go:54
  [FAIL] odo dev command tests when Devfile contains metadata.projectType when running odo dev [BeforeEach] should set the correct value in labels of resources
  /go/odo_1/tests/helper/helper_run.go:54
  [FAIL] odo dev debug command tests when a composite apply command is used as debug command [It] should execute the composite apply commands successfully
  /go/odo_1/tests/helper/helper_generic.go:86
  [FAIL] odo describe/list binding command tests when creating a component with a spec binding and envvars (service in namespace "binding-vso") when Starting a Pg service when running dev session [BeforeEach] when changing the current namespace should list the binding with --namespace flag
  /go/odo_1/tests/helper/helper_run.go:54
  [FAIL] odo describe/list binding command tests when creating a component with a binding as environment variables (service in namespace "binding-pvu") when Starting a Pg service when running dev session [BeforeEach] should list the binding
  /go/odo_1/tests/helper/helper_run.go:54
  [FAIL] odo list with devfile devfile has missing metadata when projectType is missing when the component is pushed in dev mode [BeforeEach] should show the language for 'Type' in odo list
  /go/odo_1/tests/helper/helper_run.go:54
  [FAIL] odo describe/list binding command tests when creating a component with a binding as files (service in namespace "") when Starting a Pg service when running dev session [BeforeEach] should list the binding
  /go/odo_1/tests/helper/helper_run.go:54

Ran 452 of 824 Specs in 2154.637 seconds
FAIL! -- 445 Passed | 7 Failed | 0 Pending | 372 Skipped
Commonly occurring issue:

  [FAIL] odo describe/list binding command tests when creating a component with a binding as files (service in namespace "") when Starting a Pg service when running dev session [BeforeEach] should list the binding
  [odo]  âš   Devfile command "run" exited with an error status in 20 second(s)
  [odo]  âš   Last 100 lines of log:
  [odo] npm WARN saveError ENOENT: no such file or directory, open '/projects/package.json'
  [odo] npm notice created a lockfile as package-lock.json. You should commit this file.
  [odo] npm WARN enoent ENOENT: no such file or directory, open '/projects/package.json'
  [odo] npm WARN projects No description
  [odo] npm WARN projects No repository field.
  [odo] npm WARN projects No README data
  [odo] npm WARN projects No license field.
  [odo] 
  [odo] up to date in 0.407s
  [odo] found 0 vulnerabilities
  [odo] 
  [odo] npm ERR! code ENOENT
  [odo] npm ERR! syscall open
  [odo] npm ERR! path /projects/package.json
  [odo] npm ERR! errno -2
  [odo] npm ERR! enoent ENOENT: no such file or directory, open '/projects/package.json'
  [odo] npm ERR! enoent This is related to npm not being able to find a file.
  [odo] npm ERR! enoent 
  [odo] 
  [odo] npm ERR! A complete log of this run can be found in:
  [odo] npm ERR!     /opt/app-root/src/.npm/_logs/2023-04-24T15_17_21_036Z-debug.log
  [odo]  •  Waiting for the application to be ready  ...
  [odo] I0424 15:17:27.399028   21878 exec.go:33] Executing command [/bin/sh -c cat /proc/net/tcp /proc/net/udp /proc/net/tcp6 /proc/net/udp6] for pod: my-nodejs-app-app-6cb4666d58-6j2lh in container: runtime
----omitted for brevity----
  [odo] I0424 15:18:11.719313   21878 port.go:317] port 3000 not listening in container "runtime"
  [FAILED] in [BeforeEach] - /go/odo_1/tests/helper/helper_run.go:54 @ 04/24/23 15:18:16.226
  [PANICKED] in [AfterEach] - /usr/local/go/src/runtime/panic.go:260 @ 04/24/23 15:18:16.227
  Deleting project: cmd-describe-list-binding-test742pyx
  Running oc with args [oc delete project cmd-describe-list-binding-test742pyx --wait=false] and odo env: []
  [oc] project.project.openshift.io "cmd-describe-list-binding-test742pyx" deleted
  Deleting project: cmd-describe-list-binding-test742abo
  Running oc with args [oc delete project cmd-describe-list-binding-test742abo --wait=false] and odo env: []
  [oc] project.project.openshift.io "cmd-describe-list-binding-test742abo" deleted
  Setting current dir to: /go/odo_1/tests/integration
  Deleting dir: /tmp/1326900379
  Deleting dir: /tmp/2164776991
  << Timeline

  [FAILED] Timed out after 420.000s.
  Expected
      <string>:   __
       /  \__     Developing using the "my-nodejs-app" Devfile
       \__/  \    Namespace: cmd-describe-list-binding-test742abo
       /  \__/    odo version: v3.9.0
       \__/
      
      ↪ Running on the cluster in Dev mode
       •  Waiting for Kubernetes resources  ...
       •  Creating resource ServiceBinding/my-nodejs-app-cluster-sample-k8s  ...
      
 ✓  Creating resource ServiceBinding/my-nodejs-app-cluster-sample-k8s 
       •  Creating resource ServiceBinding/my-nodejs-app-cluster-sample-ocp  ...
      
 ✓  Creating resource ServiceBinding/my-nodejs-app-cluster-sample-ocp 
       âš   Pod is Pending
       âš   Pod is Terminating
       âš   No pod exists
       âš   Pod is Pending
       ✓  Pod is Running
       •  Syncing files into the container  ...
      
 ✓  Syncing files into the container [1s]
       •  Building your application in container (command: install)  ...
      
 ✓  Building your application in container (command: install) [2s]
       •  Executing the application (command: run)  ...
       ✗  Finished executing the application (command: run) [2s]
       •  Waiting for the application to be ready  ...
      
  to contain substring
      <string>: [Ctrl+c] - Exit
  In [BeforeEach] at: /go/odo_1/tests/helper/helper_run.go:54 @ 04/24/23 15:18:16.226

Ref: https://s3.eu-de.cloud-object-storage.appdomain.cloud/odo-tests-openshift-logs/pr-6762-openshift-tests-4579.txt

@feloy feloy closed this Apr 24, 2023
@feloy feloy reopened this Apr 24, 2023
@feloy feloy closed this Apr 25, 2023
@feloy feloy reopened this Apr 25, 2023
@sonarcloud
Copy link

sonarcloud bot commented Apr 25, 2023

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 10 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@odo-robot
Copy link

odo-robot bot commented Apr 25, 2023

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

@feloy
Copy link
Contributor Author

feloy commented Apr 25, 2023

/override windows-integration-test/Windows-test

[FAILED] [23.603 seconds]
odo list with devfile listing non-odo managed components when an operator managed deployment(without instance and managed-by label) is deployed [AfterEach] should not be listed in the odo list output
  [AfterEach] C:/Users/Administrator.ANSIBLE-TEST-VS/3792/tests/integration/cmd_devfile_list_test.go:85
  [It] C:/Users/Administrator.ANSIBLE-TEST-VS/3792/tests/integration/cmd_devfile_list_test.go:88

  [oc] Unable to connect to the server: dial tcp 161.156.12.82:31700: connectex: A connection attempt failed because the connected party did not properly respond after a period of time, or established connection failed because connected host has failed to respond.

@openshift-ci
Copy link

openshift-ci bot commented Apr 25, 2023

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

In response to this:

/override windows-integration-test/Windows-test

[FAILED] [23.603 seconds]
odo list with devfile listing non-odo managed components when an operator managed deployment(without instance and managed-by label) is deployed [AfterEach] should not be listed in the odo list output
 [AfterEach] C:/Users/Administrator.ANSIBLE-TEST-VS/3792/tests/integration/cmd_devfile_list_test.go:85
 [It] C:/Users/Administrator.ANSIBLE-TEST-VS/3792/tests/integration/cmd_devfile_list_test.go:88

 [oc] Unable to connect to the server: dial tcp 161.156.12.82:31700: connectex: A connection attempt failed because the connected party did not properly respond after a period of time, or established connection failed because connected host has failed to respond.

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.

@openshift-merge-robot openshift-merge-robot merged commit e9c5f86 into redhat-developer:main Apr 25, 2023
@feloy feloy mentioned this pull request May 2, 2023
@rm3l rm3l added this to the v3.10.0 🚀 milestone May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. 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.

4 participants