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

Test() method now tries the connection token aquired #197

Merged
merged 1 commit into from
Feb 6, 2020

Conversation

rgolangh
Copy link
Contributor

@rgolangh rgolangh commented Feb 6, 2020

When the client has a token acquired by former authentication we should
use it to test the connection. It is not enough just to keep holding the
token state when in fact it is no longer valid.

Description of the Change

The change actually performs a simple request to the ovirt-engine API, using an OPTIONS
https method with the Bearer token. If the token is no longer valid the request fails
and a reauthentication is made, to acquire a new token.

Benefits

Long running application, that needs to to communicate with engine API can survive
events of session expirations or timeouts and tokens being removed, without
creating new connections over and over again, by using the Test() method.
This should save us from constant login attempts, and load on the session management module
in the engine.

Possible Drawbacks

It may be that we want to design the client in a way that it only uses auth token and not to really
perform login with user/pass. In that case we would need to change some of the behaviours.

Verification Process

Use the procedure described in the issue to verify #193 (comment)

Fixes: #193

@imjoey PTAL

When the client has a token accquired by former authentication we should
use it to test the connection. It is not enough just to keep holding the
token state when in fact it is no longer valid.

Signed-off-by: Roy Golan <rgolan@redhat.com>
@imjoey
Copy link
Member

imjoey commented Feb 6, 2020

@rgolangh It makes sense and look good to me. Thanks for this.

@imjoey imjoey merged commit fbd25e3 into oVirt:master Feb 6, 2020
rgolangh added a commit to rgolangh/cluster-api-provider-ovirt that referenced this pull request Feb 17, 2020
Bump the ovirt go sdk to reliably test the connection to the api.

See: oVirt/ovirt-engine-sdk-go#197

Signed-off-by: Roy Golan <rgolan@redhat.com>
openshift-cherrypick-robot pushed a commit to openshift-cherrypick-robot/cluster-api-provider-ovirt that referenced this pull request Feb 18, 2020
Bump the ovirt go sdk to reliably test the connection to the api.

See: oVirt/ovirt-engine-sdk-go#197

Signed-off-by: Roy Golan <rgolan@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

connection Test() doesn't really mean if a connection is authenticated and ready
2 participants