Skip to content

Misc fixes around API permissions, global settings and template UX#3534

Merged
yadvr merged 4 commits into
apache:masterfrom
shapeblue:misc-fixes
Aug 1, 2019
Merged

Misc fixes around API permissions, global settings and template UX#3534
yadvr merged 4 commits into
apache:masterfrom
shapeblue:misc-fixes

Conversation

@yadvr
Copy link
Copy Markdown
Member

@yadvr yadvr commented Jul 31, 2019 •
edited
Loading

  • This allows getUploadParamsForIso for all user role types, also fixes
    authorised field for getUploadParamsForTemplate API.
  • Fix global setting description to say what is used when value is empty/blank.
  • For VM running/allocated usage description, use parenthesis to return the instance name and ID.
  • Display template download progress when template is added to a project

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)

@yadvr
api: allow getUploadParamsForIso for all user role types
665d270 
This allows getUploadParamsForIso for all user role types, also fixes
authorised field for getUploadParamsForTemplate API.

Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
@yadvr yadvr added this to the 4.13.0.0 milestone Jul 31, 2019
yadvr added 3 commits July 31, 2019 13:38
@yadvr
fix config key setting description
787d075 
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
@yadvr
fix usage description to not use comma, put ID in another parenthesis
b4b718b 
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
@yadvr
show template progress when template belong to a project
23b2e01 
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
@yadvr
Copy link
Copy Markdown
Member Author

yadvr commented Jul 31, 2019

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jul 31, 2019

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jul 31, 2019

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-203

@yadvr
Copy link
Copy Markdown
Member Author

yadvr commented Jul 31, 2019

@blueorangutan test

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jul 31, 2019

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

@yadvr
Copy link
Copy Markdown
Member Author

yadvr commented Jul 31, 2019

Pinging for review @shwstppr @anuragaw

anuragaw
anuragaw approved these changes Jul 31, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@anuragaw anuragaw left a comment

Choose a reason for hiding this comment

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

Code LGTM. But a minor suggestion.

Will test and get back unless @andrijapanicsb or @borisstoyanov want to verify/review it ?

Comment thread api/src/main/java/org/apache/cloudstack/query/QueryService.java
static final ConfigKey<String> UserVMBlacklistedDetails = new ConfigKey<String>("Advanced", String.class,
"user.vm.blacklisted.details", "rootdisksize, cpuOvercommitRatio, memoryOvercommitRatio, Message.ReservedCapacityFreed.Flag",
"Determines whether users can view certain VM settings", true);
"Determines whether users can view certain VM settings. When set to empty, default value used is: rootdisksize, cpuOvercommitRatio, memoryOvercommitRatio, Message.ReservedCapacityFreed.Flag.", true);
Copy link
Copy Markdown
Contributor

@anuragaw anuragaw Jul 31, 2019

Choose a reason for hiding this comment

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

Slight addition could be there optinally - "To allow all settings to be visible set this to none"?

Copy link
Copy Markdown
Member Author

@yadvr yadvr Jul 31, 2019

Choose a reason for hiding this comment

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

The main issue with setting to a fake value is that, what is the none is used as a valid detail (most likely that's never going to happen but for sanity-sake). Which is why I chose to simply state that behaviour.

Copy link
Copy Markdown
Contributor

@anuragaw anuragaw Jul 31, 2019

Choose a reason for hiding this comment

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

Sounds good.

@yadvr yadvr added the type:bug label Jul 31, 2019
@yadvr yadvr changed the title [WIP] Misc fixes Misc fixes around API permissions, global settings and template UX Jul 31, 2019
shwstppr
shwstppr approved these changes Jul 31, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@shwstppr shwstppr left a comment

Choose a reason for hiding this comment

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

LGTM based on changes

@borisstoyanov
Copy link
Copy Markdown
Contributor

borisstoyanov commented Jul 31, 2019

These changes seem fine, there's one problem related to uploading template and iso from local, which I'm not sure if it will be fixed in this PR?
cc: @rhtyd

@yadvr
Copy link
Copy Markdown
Member Author

yadvr commented Jul 31, 2019

I'll check/test @borisstoyanov

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jul 31, 2019

Trillian test result (tid-247)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35650 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr3534-t247-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 77 look OK, 0 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File

@anuragaw
Copy link
Copy Markdown
Contributor

anuragaw commented Aug 1, 2019

@borisstoyanov, @rhtyd - I also see a format not matching the selected format when I upload a local macchina qcow2 template on KVM based monkey box. Investigating bit more.

@yadvr
Copy link
Copy Markdown
Member Author

yadvr commented Aug 1, 2019

For the upload iso/template fix, I'll send another PR Bobby. Thanks.
Merging based on test results and lgtms.

@yadvr yadvr merged commit d3b292a into apache:master Aug 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shwstppr shwstppr shwstppr approved these changes

@svenvogel svenvogel svenvogel approved these changes

@borisstoyanov borisstoyanov Awaiting requested review from borisstoyanov

@andrijapanicsb andrijapanicsb Awaiting requested review from andrijapanicsb

+1 more reviewer

@anuragaw anuragaw anuragaw approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

component:api type:bug

Projects

None yet

Milestone

4.13.0.0

Development

Successfully merging this pull request may close these issues.

6 participants

@yadvr @blueorangutan @borisstoyanov @anuragaw @shwstppr @svenvogel