Skip to content

system_use column is not part of disk offering - #6015

Merged
DaanHoogland merged 1 commit into
mainfrom
FixUserListingDiskOffering
Feb 22, 2022
Merged

system_use column is not part of disk offering#6015
DaanHoogland merged 1 commit into
mainfrom
FixUserListingDiskOffering

Conversation

@harikrishna-patnala

@harikrishna-patnalaharikrishna-patnala commented Feb 21, 2022•
edited
Loading

Copy link
Copy Markdown
Member

Description

This PR fixes#6013

system_use is used to be in disk_offering table before 4.17 which is only applicable to the system service offering. This column is now moved from disk_offering table to service_offering table.

There is a leftover usage of system_use when a user tries to list disk offering. Now we deleted that reference to fix the issue when a user tries to list disk offering.

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

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

How Has This Been Tested?

  1. From a user account, tried listing the disk offerings on volume creation wizard and in VM deployment wizard for data disk
  2. User could list the disk offerings.

image

@harikrishna-patnalaharikrishna-patnala added this to the 4.17.0.0 milestone Feb 21, 2022
@harikrishna-patnalaharikrishna-patnala mentioned this pull request Feb 21, 2022
Closed
DaanHoogland
DaanHoogland approved these changes Feb 21, 2022
View reviewed changes

@DaanHooglandDaanHoogland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

if disk offerings are indeed never exclusive for system use, this criterium should not be there indeed.

@harikrishna-patnala
system_use column is now moved from disk_offering table to service_of…
74a15a5
…fering table, because it is applicable only to service offering.
There is a left over usage of system_use when user tries to list disk offering. Now we deleted that reference.
@harikrishna-patnala

harikrishna-patnala commented Feb 21, 2022

Copy link
Copy Markdown
MemberAuthor

@blueorangutan package

@blueorangutan

blueorangutan commented Feb 21, 2022

Copy link
Copy Markdown

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

weizhouapache
weizhouapache approved these changes Feb 21, 2022
View reviewed changes

@weizhouapacheweizhouapache left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

tested ok

sureshanaparti
sureshanaparti reviewed Feb 21, 2022
View reviewed changes
Comment threadserver/src/main/java/com/cloud/api/query/QueryManagerImpl.java
throw new InvalidParameterValueException("Only ROOT admins and Domain admins can list disk offerings with isrecursive=true");
}
} else { // domain + all ancestors
sc.addAnd("systemUse", SearchCriteria.Op.EQ, false); // non-root users should not see system offering at all

@sureshanapartisureshanapartiFeb 21, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is moved to service/compute offering, not needed while listing disk offerings.

@nvazquez

nvazquez commented Feb 21, 2022

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

blueorangutan commented Feb 21, 2022

Copy link
Copy Markdown

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

@blueorangutan

blueorangutan commented Feb 21, 2022

Copy link
Copy Markdown

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 2673

@weizhouapache

weizhouapache commented Feb 21, 2022

Copy link
Copy Markdown
Member

@blueorangutan test

@blueorangutan

blueorangutan commented Feb 21, 2022

Copy link
Copy Markdown

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

@DaanHoogland
DaanHoogland merged commit 4e826d5 into mainFeb 22, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DaanHooglandDaanHooglandDaanHoogland approved these changes
@nvazqueznvazqueznvazquez approved these changes
@sureshanapartisureshanapartisureshanaparti approved these changes
@weizhouapacheweizhouapacheweizhouapache approved these changes

Assignees

No one assigned

Labels

Severity:BLOCKERtype:bug

Projects

None yet

Milestone

4.17.0.0

Development

Successfully merging this pull request may close these issues.

regular user has no access to offferings for creating a new volume

6 participants

@harikrishna-patnala@blueorangutan@nvazquez@weizhouapache@DaanHoogland@sureshanaparti