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

Azure deployment refresh enhancement #21

Merged
merged 5 commits into from
Mar 7, 2017

Conversation

bzwei
Copy link
Contributor

@bzwei bzwei commented Dec 21, 2016

  1. Use the recently added Azure API to directly get deployment template. With old API we can only retrieve deployment template if the URI is known, or if the deployment is provisioned from MangeIQ. As a result many deployments cannot recover their templates. With the newest API we now can directly get the templates without downloading from an URI or loading from ManageIQ DB. But the API does not work with old deployments, so we have to keep existing logic as the fallback method.

  2. Simplify how to identify resources created by a deployment. We now understand the key to identify them is provisioning_operation == 'Create'. This allows us to delete quite some code and simply the logic.

It seems above improvements should be separate PRs; but they all require regenerating the VCR for refresh spec test. In addition, our recent upgrade azure-armrest gem version to 0.5.0 also requires regenerating the VCR. The work to regenerate VCR is non-trivial, so that I would like to put them together with one commit.

This PR has a dependency on ManageIQ/manageiq-gems-pending#30

@bzwei
Copy link
Contributor Author

bzwei commented Dec 22, 2016

/cc @blomquisg @djberg96 @bronaghs
The build failed because we need a newer azure-armrest. ManageIQ/manageiq-gems-pending#30 needs to be merged first.

@bzwei bzwei closed this Jan 3, 2017
@bzwei bzwei reopened this Jan 3, 2017
@bzwei bzwei closed this Jan 3, 2017
@bzwei bzwei reopened this Jan 3, 2017
@bzwei bzwei closed this Jan 4, 2017
@bzwei bzwei reopened this Jan 4, 2017
@bzwei bzwei closed this Jan 9, 2017
@bzwei bzwei reopened this Jan 9, 2017
@miq-bot
Copy link
Member

miq-bot commented Jan 11, 2017

This pull request is not mergeable. Please rebase and repush.

@djberg96
Copy link
Collaborator

@bzwei Sorry for the conflicting files, @durandom had to update the cassettes for one of his PR's.

@djberg96
Copy link
Collaborator

djberg96 commented Feb 3, 2017

@bzwei Should work now with #28 merged.

@@ -1,3 +1,5 @@
require 'azure-armrest'

Copy link
Collaborator

Choose a reason for hiding this comment

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

I've learned the hard way that we shouldn't do this. It causes a load ordering problem that results in the Rails cache getting cleared because of the cache_method gem:

#38

@djberg96
Copy link
Collaborator

You should be able to just use the cassettes and specs that are already there.

@bzwei
Copy link
Contributor Author

bzwei commented Mar 6, 2017

@miq-bot assign @blomquisg
@miq-bot add_label enhancement
@miq-bot add_label euwe/no

@bzwei
Copy link
Contributor Author

bzwei commented Mar 6, 2017

With the new API support we are now able to get templates for many stacks that we used cannot due to the lacking of uri. For this reason we see the number of total orchestration templates in the testing provider increases from 4 to 18.

@miq-bot
Copy link
Member

miq-bot commented Mar 6, 2017

Checked commits bzwei/manageiq-providers-azure@515c9a5~...16f45d7 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks good. 🍰

@blomquisg
Copy link
Member

@djberg96 can you give me a 👍 on this one?

@djberg96
Copy link
Collaborator

djberg96 commented Mar 6, 2017

👍

@blomquisg blomquisg merged commit a47e469 into ManageIQ:master Mar 7, 2017
@blomquisg blomquisg added this to the Sprint 56 Ending Mar 13, 2017 milestone Mar 7, 2017
@bzwei bzwei deleted the deployment_refresh branch March 7, 2017 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants