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

Do not error out in port detection if any of the /proc/net/{tc,ud}p{,6} files are missing in the dev container #6831

Conversation

rm3l
Copy link
Member

@rm3l rm3l commented May 22, 2023

What type of PR is this:
/kind bug
/area odo-on-podman

What does this PR do / why we need it:
This PR makes port detection a bit resilient if any of the /proc/net/{tc,ud}p* are missing in the container.

For example, /proc/net/{tc,ud}p6 files might be missing if IPv6 is disabled in the host networking stack, as reported for instance on RHEL (context of #6824). Actually, /proc/net/{tc,ud}p* files might even be totally missing if network stats are disabled in the kernel configuration.

Which issue(s) this PR fixes:
Related to #6824

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:

rm3l added 2 commits May 22, 2023 14:07
… files are missing

/proc/net/{tc,ud}p6 files might be missing if IPv6 is disabled in the host networking stack,
as reported by a user on RHEL.

Actually /proc/net/{tc,ud}p* files might be totally missing if network stats are disabled.
…n the loopback interface

We are already displaying a warning in such case
(saying that port forwarding might not work correctly);
so this should not prevent port-forwarding messages to be displayed.

We are already behaving this way when detecting if endpoints in the Devfile
are actually opened in the container.
A warning is displayed if any error occurs while checking this.
@netlify
Copy link

netlify bot commented May 22, 2023

Deploy Preview for odo-docusaurus-preview ready!

Name Link
🔨 Latest commit e3cb047
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/646c76367d51ac0008e95fba
😎 Deploy Preview https://deploy-preview-6831--odo-docusaurus-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@openshift-ci openshift-ci bot added kind/bug Categorizes issue or PR as related to a bug. area/odo-on-podman Issues or PRs related to running odo against Podman labels May 22, 2023
@openshift-ci openshift-ci bot requested review from anandrkskd and kadel May 22, 2023 12:35
@rm3l rm3l requested review from sbouchet, feloy and valaparthvi and removed request for kadel and anandrkskd May 22, 2023 12:36
@rm3l rm3l added this to the v3.11.0 🚀 milestone May 22, 2023
Copy link
Contributor

@feloy feloy left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label May 22, 2023
@odo-robot
Copy link

odo-robot bot commented May 22, 2023

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

@odo-robot
Copy link

odo-robot bot commented May 22, 2023

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

@odo-robot
Copy link

odo-robot bot commented May 22, 2023

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

@odo-robot
Copy link

odo-robot bot commented May 22, 2023

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

@odo-robot
Copy link

odo-robot bot commented May 22, 2023

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

@odo-robot
Copy link

odo-robot bot commented May 22, 2023

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

@odo-robot
Copy link

odo-robot bot commented May 22, 2023

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

@odo-robot
Copy link

odo-robot bot commented May 22, 2023

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

@sbouchet
Copy link

sbouchet commented May 22, 2023

my tests returns :

sbouchet@sbouchet:~$ podman exec -it go-app-runtime /bin/sh -c "for f in tcp udp tcp6 udp6; do cat /proc/net/$f || true; done"
cat: /proc/net/: Is a directory
cat: /proc/net/: Is a directory
cat: /proc/net/: Is a directory
cat: /proc/net/: Is a directory

if i escape the $ sign :

sbouchet@sbouchet:~$ podman exec -it go-app-runtime /bin/sh -c "for f in tcp udp tcp6 udp6; do cat /proc/net/\$f || true; done"
  sl  local_address rem_address   st tx_queue rx_queue tr tm->when retrnsmt   uid  timeout inode                                                     
   0: 00000000:1F90 00000000:0000 0A 00000000:00000000 00:00000000 00000000  1001        0 58108305 1 00000000620d2fb7 100 0 0 10 0                  
  sl  local_address rem_address   st tx_queue rx_queue tr tm->when retrnsmt   uid  timeout inode ref pointer drops             
cat: /proc/net/tcp6: No such file or directory
cat: /proc/net/udp6: No such file or directory
sbouchet@sbouchet:~$ 

@rm3l
Copy link
Member Author

rm3l commented May 22, 2023

my tests returns :

sbouchet@sbouchet:~$ podman exec -it go-app-runtime /bin/sh -c "for f in tcp udp tcp6 udp6; do cat /proc/net/$f || true; done"
cat: /proc/net/: Is a directory
cat: /proc/net/: Is a directory
cat: /proc/net/: Is a directory
cat: /proc/net/: Is a directory

if i escape the $ sign :

sbouchet@sbouchet:~$ podman exec -it go-app-runtime /bin/sh -c "for f in tcp udp tcp6 udp6; do cat /proc/net/\$f || true; done"
  sl  local_address rem_address   st tx_queue rx_queue tr tm->when retrnsmt   uid  timeout inode                                                     
   0: 00000000:1F90 00000000:0000 0A 00000000:00000000 00:00000000 00000000  1001        0 58108305 1 00000000620d2fb7 100 0 0 10 0                  
  sl  local_address rem_address   st tx_queue rx_queue tr tm->when retrnsmt   uid  timeout inode ref pointer drops             
cat: /proc/net/tcp6: No such file or directory
cat: /proc/net/udp6: No such file or directory
sbouchet@sbouchet:~$ 

Yes, I think it should also work with simple quotes with no need to escape anything:

podman exec -it go-app-runtime /bin/sh -c 'for f in tcp udp tcp6 udp6; do cat /proc/net/$f || true; done'

At least, that's how the command would get passed to the container by odo.

@rm3l
Copy link
Member Author

rm3l commented May 23, 2023

• [FAILED] [127.958 seconds]
odo init interactive command tests label unauth personalizing Devfile configuration [It] should allow for personalizing configurations [unauth]
/go/odo_1/tests/integration/interactive_init_test.go:95
...
  ? What configuration do you want change? Add new environment variable
  ? Enter new environment variable name: DEBUG_PROJECT_PORT
  
  ======================
  Unexpected error:
      <*xpty.errPassthroughTimeout | 0xc00174e010>: {
          error: <*errors.errorString | 0xc00174e000>{
              s: "passthrough i/o timeout",
          },
      }
      passthrough i/o timeout
  occurred
  In [It] at: /go/odo_1/tests/helper/helper_interactive.go:111 @ 05/22/23 12:43:15.823
------------------------------

Summarizing 1 Failure:
  [FAIL] odo init interactive command tests label unauth personalizing Devfile configuration [It] should allow for personalizing configurations [unauth]
  /go/odo_1/tests/helper/helper_interactive.go:111

Ran 111 of 888 Specs in 168.542 seconds
FAIL! -- 110 Passed | 1 Failed | 0 Pending | 777 Skipped

Issue with interactive tests - being addressed in #6830

/override OpenShift-Integration-tests/OpenShift-Unauth-Integration-tests

@openshift-ci
Copy link

openshift-ci bot commented May 23, 2023

@rm3l: Overrode contexts on behalf of rm3l: OpenShift-Integration-tests/OpenShift-Unauth-Integration-tests

In response to this:

• [FAILED] [127.958 seconds]
odo init interactive command tests label unauth personalizing Devfile configuration [It] should allow for personalizing configurations [unauth]
/go/odo_1/tests/integration/interactive_init_test.go:95
...
 ? What configuration do you want change? Add new environment variable
 ? Enter new environment variable name: DEBUG_PROJECT_PORT
 
 ======================
 Unexpected error:
     <*xpty.errPassthroughTimeout | 0xc00174e010>: {
         error: <*errors.errorString | 0xc00174e000>{
             s: "passthrough i/o timeout",
         },
     }
     passthrough i/o timeout
 occurred
 In [It] at: /go/odo_1/tests/helper/helper_interactive.go:111 @ 05/22/23 12:43:15.823
------------------------------

Summarizing 1 Failure:
 [FAIL] odo init interactive command tests label unauth personalizing Devfile configuration [It] should allow for personalizing configurations [unauth]
 /go/odo_1/tests/helper/helper_interactive.go:111

Ran 111 of 888 Specs in 168.542 seconds
FAIL! -- 110 Passed | 1 Failed | 0 Pending | 777 Skipped

Issue with interactive tests - being addressed in #6830

/override OpenShift-Integration-tests/OpenShift-Unauth-Integration-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.

pkg/port/port.go Outdated
"cat /proc/net/tcp /proc/net/udp /proc/net/tcp6 /proc/net/udp6",
// /proc/net/{tc,ud}p6 files might be missing if IPv6 is disabled in the host networking stack.
// Actually /proc/net/{tc,ud}p* files might be totally missing if network stats are disabled.
"for f in tcp udp tcp6 udp6; do cat /proc/net/$f || true; done",
Copy link
Contributor

Choose a reason for hiding this comment

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

You could also keep the previous command, which will display the files found and display an error for not found ones.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we can do that. I didn't want to stop returning the content of the subsequent files if any file in the middle does not exist, but that seems to be already the case here.

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. Required by Prow. label May 23, 2023
@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

@rm3l rm3l requested a review from feloy May 23, 2023 08:28
@rm3l
Copy link
Member Author

rm3l commented May 23, 2023

  [oc] ''Unable to connect to the server: dial tcp 149.81.180.114:30329: 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.
...
Summarizing 3 Failures:
  [FAIL] odo dev command tests when a container component defines a Command or Args - without metadata.name [It] should run odo dev successfully (#5620)
  C:/Users/Administrator.ANSIBLE-TEST-VS/3995/tests/helper/helper_cmd_wrapper.go:101
  [FAIL] odo describe/list binding command tests when creating a component with a binding as files (service in namespace "binding-aws") when Starting a Pg service [BeforeEach] when running dev session should describe the binding
  C:/Users/Administrator.ANSIBLE-TEST-VS/3995/tests/helper/helper_generic.go:58
  [FAIL] odo dev command tests when running odo dev with alternative commands - without metadata.name [It] should execute the custom non-default build and run commands successfully
  C:/Users/Administrator.ANSIBLE-TEST-VS/3995/tests/integration/cmd_dev_test.go:2887

Network issues in those tests - tracked in #6838

/override windows-integration-test/Windows-test

@openshift-ci
Copy link

openshift-ci bot commented May 23, 2023

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

In response to this:

 [oc] ''Unable to connect to the server: dial tcp 149.81.180.114:30329: 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.
...
Summarizing 3 Failures:
 [FAIL] odo dev command tests when a container component defines a Command or Args - without metadata.name [It] should run odo dev successfully (#5620)
 C:/Users/Administrator.ANSIBLE-TEST-VS/3995/tests/helper/helper_cmd_wrapper.go:101
 [FAIL] odo describe/list binding command tests when creating a component with a binding as files (service in namespace "binding-aws") when Starting a Pg service [BeforeEach] when running dev session should describe the binding
 C:/Users/Administrator.ANSIBLE-TEST-VS/3995/tests/helper/helper_generic.go:58
 [FAIL] odo dev command tests when running odo dev with alternative commands - without metadata.name [It] should execute the custom non-default build and run commands successfully
 C:/Users/Administrator.ANSIBLE-TEST-VS/3995/tests/integration/cmd_dev_test.go:2887

Network issues in those tests - tracked in #6838

/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.

@openshift-merge-robot openshift-merge-robot merged commit dcf9009 into redhat-developer:main May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/odo-on-podman Issues or PRs related to running odo against Podman 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