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

Refresh container providers via api fails #7256

Closed
dkorn opened this issue Mar 13, 2016 · 12 comments
Closed

Refresh container providers via api fails #7256

dkorn opened this issue Mar 13, 2016 · 12 comments

Comments

@dkorn
Copy link
Contributor

dkorn commented Mar 13, 2016

According to the docs [1] performing a refresh of providers using api request is done via the refresh action. While it works well for cloud providers, when attempting that on container provider the operation fails with the following response (provider id is 7):

{
"success": false
"message": "Cloud/Infrastructure Provider failed last authentication check"
"href": "http://localhost:3000/api/providers/7"
} 

Looking into it a little, I was able to trace the failure into two methods, that are being called from def refresh_ems from app/models/ext_management_system.rb: missing_credentials and authentication_status_ok.
The first isn't implemented in container_manager.rb, while its implementation in cloud_manager.rb seems lacking some logic (returns false). The second requires a bit more research.

[1] https://github.com/ManageIQ/manageiq_docs/blob/master/api/reference/providers.adoc#refresh-providers

@dkorn
Copy link
Contributor Author

dkorn commented Mar 13, 2016

@alongoldboim @simon3z we are missing the refresh operation as the last part of the deployment.
@simon3z - let me know if you want me to further deep dive into it.

@simon3z
Copy link
Contributor

simon3z commented Mar 14, 2016

@dkorn if this is specific for containers then yes please continue researching (and fix).

@simon3z
Copy link
Contributor

simon3z commented Jul 14, 2016

@zakiva can you take a look at this? (I think @yaacov can help you here).

@miq-bot assign zakiva

@miq-bot miq-bot assigned zakiva and unassigned simon3z Jul 14, 2016
@zakiva
Copy link
Contributor

zakiva commented Jul 20, 2016

Refreshing container providers via the api is currently working fine for me, the problem was probably related to issue #8643 which was already solved.
@dkorn Can you verify that it is working for you now?
cc @simon3z @yaacov

@simon3z
Copy link
Contributor

simon3z commented Jul 20, 2016

@dkorn you've been working on darga recently (for another bug), can you verify this on darga as well?

@yaacov
Copy link
Member

yaacov commented Jul 20, 2016

@simon3z @dkorn to clarify:

  1. The refresh is done using a worker, the worker will not start if the authentications are not valid.
    Before running the refresh the authentications must be valid.

  2. The way it was solved, is to not allow the creation of a provider that did not pass the validation checks.
    A provider created not from the ui (e.g. using console) without checking for validation will also fail to do refresh.

@simon3z
Copy link
Contributor

simon3z commented Sep 2, 2016

@dkorn @yaacov what's the current status of this?

@yaacov
Copy link
Member

yaacov commented Sep 4, 2016

@simon3z @dkorn the code for API refresh [1] does two checks:

  1. Check for missing_credentials?
    This check is buggy for container providers, and may fail if no userid is given.
    We have a PR to fix that, Refactor default required credential fields in authentication mixin #10690.
  2. Check for authentication_status_ok?
    A newly created or edited provider may be with nil or bad authentication_status. To solve this in the UI a validate button was added [3], In the API we have no way to force a new validation proccess.

~~I think of this two fixes (I think they are easy to implement), but both has problems :-(

  1. Add a force validate API call.~~
  2. Do a validation on each create/edit from the API.

@simon3z @dkorn @abellotti what do you think the best way to solve this , do we need to solve this in the API ?

UPDATE A: The API forces authentication validation on each create/edit call, it does not work when using multiple endpoints.
UPDATE B: #10999 is a PR that fixes the regression that prevent multiple endpoints validating on each create/edit from the API.

[1] https://github.com/ManageIQ/manageiq/blob/master/app/models/ext_management_system.rb#L382
[2] #10690
[3] #8912

yaacov pushed a commit to yaacov/manageiq that referenced this issue Sep 4, 2016
Add a force validation in the API, to complete the actions available in the UI

Issue:
ManageIQ#7256
yaacov pushed a commit to yaacov/manageiq that referenced this issue Sep 9, 2016
A regression was created when introducing multiple provider endpoints.
The function for updating authentications was not updated to update when changing
the endpoints and authentications.
This commit updates the authentication is this cases.

Issue:
ManageIQ#7256
@miq-bot
Copy link
Member

miq-bot commented Sep 9, 2017

This issue has been automatically marked as stale because it has not been updated for at least 6 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions!

@miq-bot miq-bot added the stale label Sep 9, 2017
@yaacov
Copy link
Member

yaacov commented Sep 9, 2017

Fixed by:
#10999

@JPrause
Copy link
Member

JPrause commented Nov 30, 2018

@dkorn it appears this issue is now resolved. Please close.

@JPrause
Copy link
Member

JPrause commented Jan 22, 2019

@miq-bot close_issue

@miq-bot miq-bot closed this as completed Jan 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants