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

Find Azure orchestration stack failure from its operations #12064

Merged
merged 1 commit into from
Nov 7, 2016

Conversation

bzwei
Copy link
Contributor

@bzwei bzwei commented Oct 19, 2016

Azure template deployment, aka orchestration stack, does not have an attribute for status message. For failed deployments, user needs to go through its operations, aka resources, to find out the causes.

Because of this reason, manageiq automate service currently simply shows provision failure without detailed error message. In the log the user is reminded to look into stack resources for reasons. QE often opened manageiq bugs when an Azure orchestration provisioning failed, but the actual failure happened on the provider side.

With this enhancement we scan through the operations and delegate the error message to the stack. So that the user will see clearly how the stack creation is failing.

The enhancement work is placed in a module. The code is meant to be reused by the refresh logic. A follow-up PR will be created to use this module in refresh.

@bzwei
Copy link
Contributor Author

bzwei commented Oct 19, 2016

@gmcculloug @djberg96 please review

else
operation.properties.status_message.to_s
end
end
Copy link

@bronaghs bronaghs Oct 20, 2016

Choose a reason for hiding this comment

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

@bzwei - Looks repetitive. I have not tested this, but maybe something along these lines?
status_message = properties.try(:status_message)
return nil unless status_message
error_message = status_message.try(:error).try(:message)
error_message.nil? ? status_message.to_s : error_message

Also , maybe pass operation.properties into the method instead of just operation

before do
bad_raw_stack = Azure::Armrest::TemplateDeployment.new('properties' => {'provisioningState' => 'Failed'})
operations = [Azure::Armrest::TemplateDeploymentOperation.new('properties' => { 'statusMessage' => 'reason'})]
allow(orchestration_service).to receive(:get).and_return(bad_raw_stack)

Choose a reason for hiding this comment

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

@bzwei - A bit of a nick pick but are you testing the scenario where operation.properties.status_message.error.message is populated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bronaghs This block is testing #raw_status. We just need to setup a failed stack with any type of error message. The test for different types of error messages is in another block.

@miq-bot
Copy link
Member

miq-bot commented Oct 20, 2016

Checked commit bzwei@00e24f4 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
3 files checked, 0 offenses detected
Everything looks good. 👍

@bronaghs
Copy link

@bzwei - Looks like you added the test, is that correct? ( commit is squashed so I am not sure)

@bzwei
Copy link
Contributor Author

bzwei commented Oct 24, 2016

@bronaghs Yes. The test is added in this PR, but to the context of testing status. Before the existing test for status only covers the successful case. This one covers a failed status with error message.

There is another test deployment_failure_reason for different error formats.

@bronaghs
Copy link

@bzwei - 👍

@blomquisg - review/merge?

@djberg96
Copy link
Contributor

👍

@bronaghs
Copy link

bronaghs commented Nov 7, 2016

@gmcculloug or @agrare - final review/ merge?

@bronaghs
Copy link

bronaghs commented Nov 7, 2016

@bzwei - euwe, darga?

@agrare agrare self-assigned this Nov 7, 2016
@agrare agrare merged commit 47390ef into ManageIQ:master Nov 7, 2016
@agrare agrare added this to the Sprint 49 Ending Nov 14, 2016 milestone Nov 7, 2016
@bzwei
Copy link
Contributor Author

bzwei commented Nov 7, 2016

@miq-bot add_label euwe/yes

@simaishi
Copy link
Contributor

simaishi commented Jan 5, 2017

@bzwei is there a BZ for this?

@bzwei
Copy link
Contributor Author

bzwei commented Jan 5, 2017

@simaishi No BZ. This is an enhancement. Do we need one?

@bzwei
Copy link
Contributor Author

bzwei commented Jan 6, 2017

simaishi pushed a commit that referenced this pull request Jan 6, 2017
Find Azure orchestration stack failure from its operations
(cherry picked from commit 47390ef)

https://bugzilla.redhat.com/show_bug.cgi?id=1410828
@simaishi
Copy link
Contributor

simaishi commented Jan 6, 2017

Euwe backport details:

$ git log -1
commit 909493937a794b4f3b9474df1c1a6c2924eeefe2
Author: Adam Grare <agrare@redhat.com>
Date:   Mon Nov 7 10:53:10 2016 -0500

    Merge pull request #12064 from bzwei/azure_orchestration_error
    
    Find Azure orchestration stack failure from its operations
    (cherry picked from commit 47390ef61906fc8d02a06f8c45e80bedc267c0b3)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1410828

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.

8 participants