Improve template download error message by oscaralvaro · Pull Request #8029 · apache/cloudstack

oscaralvaro · 2023-10-03T18:12:00Z

Description

This PR...
Improves the error messages to more accurately describe the reasons for failure to download a template. This is done via surfacing the underlying reason for the failure in the error message. This will facilitate troubleshooting.

This improved error messages are only shown to the admin role, for the rest of the users there is no change in error message details (tested with non-admin account).

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

Manual testing via setting up a development environment and triggering failures. Two scenarios were tested with below resulting error messages. Several other failure reasons will be surfaced as well.

-Example: Template checksum mismatch
Before:
Unable to orchestrate start VM instance {id: "3112", name: "i-2-3112-VM", uuid: "db19b8af-00e0-4f7a-803b-a6c3b022ba5d", type="User"} due to [Template 203 could not be downloaded on pool 1, failing after trying on several hosts].

Now:
Unable to orchestrate start VM instance {id: "3112", name: "i-2-3112-VM", uuid: "db19b8af-00e0-4f7a-803b-a6c3b022ba5d", type="User"} due to [Template 203 could not be downloaded on pool 1, failing after trying on several hosts Details: Checksum validation failed].

UI:

-Example: Wrong template url

Before:
Unable to orchestrate start VM instance {id:
"3110", name: "¡-2-3110-VM", uuid:
"40478bcf-5ffa-4a18-9d22-
739d149df76", type="User") due to [Template 203 could not be downloaded on pool 1, failing after trying on several hosts].

Now:
Unable to orchestrate start VM instance {id:
"3110", name: "¡-2-3110-VM", uuid:
"40478bcf-5ffa-4a18-9d22-
739d149df76", type="User") due to [Template 203 could not be downloaded on pool 1, failing after trying on several hosts Details: Unable to download template: Error on HTTPS response].

UI:

yadvr

LGTM

yadvr · 2023-10-04T06:56:10Z

@blueorangutan package

blueorangutan · 2023-10-04T06:58:05Z

@rohityadavcloud a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2023-10-04T07:59:24Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 7219

codecov · 2023-10-04T08:11:37Z

Codecov Report

Merging #8029 (01a5ca1) into 4.18 (3d8cc63) will decrease coverage by 0.01%.
Report is 3 commits behind head on 4.18.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##               4.18    #8029      +/-   ##
============================================
- Coverage     13.07%   13.07%   -0.01%     
  Complexity     9107     9107              
============================================
  Files          2720     2720              
  Lines        257513   257520       +7     
  Branches      40151    40153       +2     
============================================
  Hits          33658    33658              
- Misses       219624   219630       +6     
- Partials       4231     4232       +1

Files	Coverage Δ
...ack/direct/download/DirectDownloadManagerImpl.java	`7.94% <0.00%> (-0.15%)`	⬇️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

DaanHoogland · 2023-10-04T08:34:27Z

thanks @oscaralvaro , I can imagine this would be an improvement in some cases but as @rohityadavcloud mentions, it might be exposing info to unprivileged users as well. Can you make an assessment of that?

yadvr · 2023-10-04T09:02:50Z

@blueorangutan test

blueorangutan · 2023-10-04T09:04:03Z

@rohityadavcloud a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

mlsorensen · 2023-10-04T17:33:29Z

thanks @oscaralvaro , I can imagine this would be an improvement in some cases but as @rohityadavcloud mentions, it might be exposing info to unprivileged users as well. Can you make an assessment of that?

Yes, this may depend on the error. Certainly if users are able to register templates, they would also like the error feedback if they have a bad checksum configured, but there may be other errors leaked, like being able to indirectly probe what servers can be reached by the hosts. One could infer that just from failure to download templates as it is today, but this may give more detail.

oscaralvaro · 2023-10-04T18:23:13Z

thanks for the feedback @rohityadavcloud @DaanHoogland @mlsorensen. I will filter the details to be shown just to admin users

blueorangutan · 2023-10-04T20:39:50Z

[SF] Trillian test result (tid-7836)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 40250 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8029-t7836-kvm-centos7.zip
Smoke tests completed. 108 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

oscaralvaro · 2023-10-04T23:09:09Z

@rohityadavcloud @DaanHoogland @mlsorensen I've made changes to show error details only to admin role

I was wondering if other roles as DOMAIN_ADMIN, RESOURCE_DOMAIN_ADMIN would benefit from this information as well

Also, it feels that a middle ground in the error message detail is missing, because currently either the errors are too generic or too internal. A more granular error messaging (without leaking internals) would allow for a faster troubleshooting and better self-support

DaanHoogland · 2023-10-05T08:36:54Z

@blueorangutan package

blueorangutan · 2023-10-05T08:38:03Z

@DaanHoogland a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2023-10-05T08:48:01Z

Packaging result [SF]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 7237

DaanHoogland · 2023-10-05T08:59:25Z

you've got a compile error @oscaralvaro:

10:45:48 [ERROR] /jenkins/workspace/acs-centos7-pkg-builder/dist/rpmbuild/BUILD/cloudstack-4.18.2.0-SNAPSHOT/server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java:[371,65] error: cannot find symbol
10:45:48   symbol:   variable ACCOUNT_TYPE_ADMIN
10:45:48   location: interface Account

Did you forget to commit something?

…ectDownloadManagerImpl.java

oscaralvaro · 2023-10-05T17:42:56Z

thanks for the commit @DaanHoogland , yes, there is a difference for Account type between 4.16 and 4.18, thanks for the fix

DaanHoogland

clgtm

DaanHoogland · 2023-10-06T07:50:52Z

@blueorangutan package

blueorangutan · 2023-10-06T07:52:03Z

@DaanHoogland a [SF] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2023-10-06T08:54:51Z

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 7251

DaanHoogland · 2023-10-06T11:39:34Z

@blueorangutan test

blueorangutan · 2023-10-06T11:40:04Z

@DaanHoogland a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

stephankruggg

CLGTM, not manually tested

blueorangutan · 2023-10-07T00:00:55Z

[SF] Trillian test result (tid-7870)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 42955 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8029-t7870-kvm-centos7.zip
Smoke tests completed. 108 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

DaanHoogland · 2023-10-09T08:19:44Z

@rohityadavcloud @DaanHoogland @mlsorensen I've made changes to show error details only to admin role

I was wondering if other roles as DOMAIN_ADMIN, RESOURCE_DOMAIN_ADMIN would benefit from this information as well

Also, it feels that a middle ground in the error message detail is missing, because currently either the errors are too generic or too internal. A more granular error messaging (without leaking internals) would allow for a faster troubleshooting and better self-support

i agree @oscaralvaro , but would leave it to a next iteration.

(cherry picked from commit c0128e2) Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

enrich error message for faster troubleshooting

c364f66

yadvr added this to the 4.18.2.0 milestone Oct 4, 2023

yadvr approved these changes Oct 4, 2023

View reviewed changes

yadvr reviewed Oct 4, 2023

View reviewed changes

Comment thread server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java Outdated

show filter error details only for admin

dfa329e

yadvr requested review from DaanHoogland and harikrishna-patnala October 5, 2023 08:01

DaanHoogland closed this Oct 5, 2023

DaanHoogland reopened this Oct 5, 2023

DaanHoogland reviewed Oct 5, 2023

View reviewed changes

Comment thread server/src/main/java/org/apache/cloudstack/direct/download/DirectDownloadManagerImpl.java Outdated

Update server/src/main/java/org/apache/cloudstack/direct/download/Dir…

01a5ca1

…ectDownloadManagerImpl.java

DaanHoogland added type:improvement Severity:Minor labels Oct 5, 2023

DaanHoogland closed this Oct 6, 2023

DaanHoogland reopened this Oct 6, 2023

DaanHoogland approved these changes Oct 6, 2023

View reviewed changes

stephankruggg approved these changes Oct 6, 2023

View reviewed changes

DaanHoogland merged commit c0128e2 into apache:4.18 Oct 9, 2023

shwstppr pushed a commit to shapeblue/cloudstack that referenced this pull request Oct 12, 2023

Improve template download error message (apache#8029)

832995a

(cherry picked from commit c0128e2) Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

Conversation

oscaralvaro commented Oct 3, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

Uh oh!

yadvr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yadvr commented Oct 4, 2023

Uh oh!

blueorangutan commented Oct 4, 2023

Uh oh!

blueorangutan commented Oct 4, 2023

Uh oh!

codecov bot commented Oct 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

DaanHoogland commented Oct 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yadvr commented Oct 4, 2023

Uh oh!

blueorangutan commented Oct 4, 2023

Uh oh!

mlsorensen commented Oct 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oscaralvaro commented Oct 4, 2023

Uh oh!

blueorangutan commented Oct 4, 2023

Uh oh!

oscaralvaro commented Oct 4, 2023

Uh oh!

DaanHoogland commented Oct 5, 2023

Uh oh!

blueorangutan commented Oct 5, 2023

Uh oh!

blueorangutan commented Oct 5, 2023

Uh oh!

DaanHoogland commented Oct 5, 2023

Uh oh!

Uh oh!

oscaralvaro commented Oct 5, 2023

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Oct 6, 2023

Uh oh!

blueorangutan commented Oct 6, 2023

Uh oh!

blueorangutan commented Oct 6, 2023

Uh oh!

DaanHoogland commented Oct 6, 2023

Uh oh!

blueorangutan commented Oct 6, 2023

Uh oh!

stephankruggg left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Oct 7, 2023

Uh oh!

DaanHoogland commented Oct 9, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

oscaralvaro commented Oct 3, 2023 •

edited

Loading

codecov bot commented Oct 4, 2023 •

edited

Loading

DaanHoogland commented Oct 4, 2023 •

edited

Loading

mlsorensen commented Oct 4, 2023 •

edited

Loading