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

Try to reload config when disconnected from the cluster #6130

Merged

Conversation

feloy
Copy link
Contributor

@feloy feloy commented Sep 16, 2022

What type of PR is this:

/kind bug

What does this PR do / why we need it:

  1. When disconnected from the cluster and odo dev is interrupted with Ctrl-c, this message is displayed:
Error connecting to the cluster, the resources were not cleaned up.
Please log in again and cleanup the resource with `odo delete component
  1. When disconnected from the cluster, odo dev tries to reload the configuration from kubeconfig file.
  • If a change of namespace or cluster in the current context is detected, the configuration is not updated. The user can still update the current namespace / cluster
  • if the namespace and cluster are the same, the new configuration is loaded, and odo dev continues with it
Keyboard Commands:
[Ctrl+c] - Exit and delete resources from the cluster
     [p] - Manually apply local changes to the application on the cluster
pPushing files...

Error connecting to the cluster. Please log in again

Error updating Kubernetes config: namespace changed ("prj3" -> "prj4"), won't refresh the configuration
Error connecting to the cluster. Please log in again

[...]
 Keyboard Commands:
[Ctrl+c] - Exit and delete resources from the cluster
     [p] - Manually apply local changes to the application on the cluster
pPushing files...

Error connecting to the cluster. Please log in again

Error updating Kubernetes config: cluster changed ("api-crc-testing:6443" -> "kubernetes"), won't refresh the configuration
Error connecting to the cluster. Please log in again

Error updating Kubernetes config: cluster changed ("api-crc-testing:6443" -> "kubernetes"), won't refresh the configuration
Error connecting to the cluster. Please log in again

[...]

Error connecting to the cluster. Please log in again

Updated Kubernetes config
 •  Waiting for Kubernetes resources  ...
 ✓  Syncing files into the container [1ms]

↪ Dev mode
 Status:
 Watching for changes in the current directory /home/phmartin/Documents/tests/go

 Keyboard Commands:
[Ctrl+c] - Exit and delete resources from the cluster
     [p] - Manually apply local changes to the application on the cluster

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

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:

[terminal 1]
$ crc start
$ oc login -u developer https://api.crc.testing:6443
[terminal 2]
$ odo dev
[...]
↪ Dev mode
 Status:
 Watching for changes in the current directory /home/phmartin/Documents/tests/go

 Keyboard Commands:
[Ctrl+c] - Exit and delete resources from the cluster
     [p] - Manually apply local changes to the application on the cluster
[terminal 1]
$ oc logout
[terminal 2]
(repeat press [p] until the token is expired and error occurs)
pPushing files...

Error connecting to the cluster. Please log in again

Updated Kubernetes config
Error connecting to the cluster. Please log in again
[...]
[terminal 1]
$ oc login -u developer https://api.crc.testing:6443
[terminal 2]
Error connecting to the cluster. Please log in again

Updated Kubernetes config
 •  Waiting for Kubernetes resources  ...
 ✓  Syncing files into the container [1ms]

↪ Dev mode
 Status:
 Watching for changes in the current directory /home/phmartin/Documents/tests/go

 Keyboard Commands:
[Ctrl+c] - Exit and delete resources from the cluster
     [p] - Manually apply local changes to the application on the cluster

@netlify
Copy link

netlify bot commented Sep 16, 2022

Deploy Preview for odo-docusaurus-preview ready!

Name Link
🔨 Latest commit d0147ca
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/632843d454b29d000883b04e
😎 Deploy Preview https://deploy-preview-6130--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 the kind/bug Categorizes issue or PR as related to a bug. label Sep 16, 2022
@openshift-ci openshift-ci bot requested review from dharmit and rm3l September 16, 2022 12:32
@odo-robot
Copy link

odo-robot bot commented Sep 16, 2022

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

@odo-robot
Copy link

odo-robot bot commented Sep 16, 2022

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

@odo-robot
Copy link

odo-robot bot commented Sep 16, 2022

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

@odo-robot
Copy link

odo-robot bot commented Sep 16, 2022

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

@odo-robot
Copy link

odo-robot bot commented Sep 16, 2022

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

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.

Thanks for your work on this. LGTM feature-wise.

/approve

@openshift-ci
Copy link

openshift-ci bot commented Sep 19, 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 Sep 19, 2022
@feloy
Copy link
Contributor Author

feloy commented Sep 19, 2022

Restart Windows tests

@feloy feloy closed this Sep 19, 2022
@feloy feloy reopened this Sep 19, 2022
@feloy
Copy link
Contributor Author

feloy commented Sep 19, 2022

Problem resolving registry.npmjs.org on cluster dedicated to Windows tests.

Retesting

@feloy feloy closed this Sep 19, 2022
@feloy feloy reopened this Sep 19, 2022
@sonarcloud
Copy link

sonarcloud bot commented Sep 19, 2022

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 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@dharmit
Copy link
Member

dharmit commented Sep 19, 2022

@feloy I'm seeing a weird behaviour in that, when I do odo logout, odo manages to push the files just fine! I tried this multiple times to be sure before writing it here. It sounds/seems super absurd to me.

However, if I change the namespaces using odo set project, it complains about a changed namespace as well as it complains about not being able to connect to the cluster which I expected the first time around as well.

@feloy
Copy link
Contributor Author

feloy commented Sep 19, 2022

@feloy I'm seeing a weird behaviour in that, when I do odo logout, odo manages to push the files just fine! I tried this multiple times to be sure before writing it here. It sounds/seems super absurd to me.

Could you please share the exact commands and output

However, if I change the namespaces using odo set project, it complains about a changed namespace as well as it complains about not being able to connect to the cluster which I expected the first time around as well.

@feloy
Copy link
Contributor Author

feloy commented Sep 19, 2022

@feloy I'm seeing a weird behaviour in that, when I do odo logout, odo manages to push the files just fine! I tried this multiple times to be sure before writing it here. It sounds/seems super absurd to me.

Could you please share the exact commands and output

Sorry, I didn't see the video you shared.

As said in the Steps to reproduce:

(repeat press [p] until the token is expired and error occurs)

When you execute oc logout, the token is removed from your kubeconfig, and so next kubectl commands fail because it does not have access anymore to the token.

On the other side, odo dev has saved the token and will still use it until an error occurs. Errors occur only when the token is de-activated on the cluster side, which can happen long after oc logout.

I experimented very different duration for the token to be deactivated on the cluster, from 1 minute to ~ 10 minutes.

@dharmit
Copy link
Member

dharmit commented Sep 20, 2022

/lgtm

Looked at 4 failed Windows jobs for this PR. A lot of failures are related to network issue:

npm ERR! network This is a problem related to network connectivity.

Hence, overriding Windows tests here because:

  1. All other tests are passing
  2. We need this feature in the next release.

/override windows-integration-test/Windows-test

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

openshift-ci bot commented Sep 20, 2022

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

In response to this:

/lgtm

Looked at 4 failed Windows jobs for this PR. A lot of failures are related to network issue:

npm ERR! network This is a problem related to network connectivity.

Hence, overriding Windows tests here because:

  1. All other tests are passing
  2. We need this feature in the next release.

/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 1d78b85 into redhat-developer:main Sep 20, 2022
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/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.

odo dev - improve error msg when exiting interactive mode and user was not authenticated to cluster
4 participants