Skip to content

api,server: allow updating hypervisor capabilities with hypervisor and version#8475

Merged
sureshanaparti merged 7 commits intoapache:4.19from
shapeblue:add-updhycapapi-param
Jun 24, 2024
Merged

api,server: allow updating hypervisor capabilities with hypervisor and version#8475
sureshanaparti merged 7 commits intoapache:4.19from
shapeblue:add-updhycapapi-param

Conversation

@shwstppr
Copy link
Contributor

@shwstppr shwstppr commented Jan 9, 2024

Description

hypervisor and hypervisorversion parameters are added to the updateHypervisorCapabilities API.
This would allow updating capabilities by specifying the hypervisor type and its version. This would also allow creating new hypervisor capabilities entries in the database when they doesn't exist for a given hypervisor version.

Related to #8412

New parameters as their description suggests are not meant to change the value of hypervisor and hypervisorversion of an existing capability. Rather they will be used to find the capabilities entry in the database and update other params such as maxdatavolumeslimit, maxguestslimit, maxhostspercluster, securitygroupenabled, storagemotionenabled, vmsnapshotenabled.

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)
  • build/CI

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

(localcloud) 🐱 > list hypervisorcapabilities hypervisor=VMware filter=hypervisorversion,maxdatavolumeslimit
{
  "count": 21,
  "hypervisorCapabilities": [
    {
      "hypervisorversion": "default",
      "maxdatavolumeslimit": 13
    },
    {
      "hypervisorversion": "4.0",
      "maxdatavolumeslimit": 13
    },
    {
      "hypervisorversion": "4.1",
      "maxdatavolumeslimit": 13
    },
    {
      "hypervisorversion": "5.0",
      "maxdatavolumeslimit": 13
    },
    {
      "hypervisorversion": "5.1",
      "maxdatavolumeslimit": 13
    },
    {
      "hypervisorversion": "5.5",
      "maxdatavolumeslimit": 13
    },
    {
      "hypervisorversion": "6.0",
      "maxdatavolumeslimit": 59
    },
    {
      "hypervisorversion": "6.5",
      "maxdatavolumeslimit": 59
    },
    {
      "hypervisorversion": "6.7",
      "maxdatavolumeslimit": 59
    },
    {
      "hypervisorversion": "6.7.1",
      "maxdatavolumeslimit": 59
    },
    {
      "hypervisorversion": "6.7.2",
      "maxdatavolumeslimit": 59
    },
    {
      "hypervisorversion": "6.7.3",
      "maxdatavolumeslimit": 59
    },
    {
      "hypervisorversion": "7.0",
      "maxdatavolumeslimit": 59
    },
    {
      "hypervisorversion": "7.0.1.0",
      "maxdatavolumeslimit": 59
    },
    {
      "hypervisorversion": "7.0.2.0",
      "maxdatavolumeslimit": 59
    },
    {
      "hypervisorversion": "7.0.3.0",
      "maxdatavolumeslimit": 59
    },
    {
      "hypervisorversion": "8.0",
      "maxdatavolumeslimit": 59
    },
    {
      "hypervisorversion": "8.0.0.1",
      "maxdatavolumeslimit": 59
    },
    {
      "hypervisorversion": "8.0.1",
      "maxdatavolumeslimit": 59
    },
    {
      "hypervisorversion": "8.0.0.2",
      "maxdatavolumeslimit": 59
    },
    {
      "hypervisorversion": "8.0.0.3",
      "maxdatavolumeslimit": 59
    }
  ]
}

  1. Update a hypervisorcapability by using hypervisor and hypervisorversion
(localcloud) 🐱 > update hypervisorcapabilities hypervisor=VMware hypervisorversion="8.0.0.3" maxdatavolumeslimit=100
{
  "hypervisorcapabilities": {
    "hypervisor": "VMware",
    "hypervisorversion": "8.0.0.3",
    "id": "761b1e90-22af-11ef-b7e0-a02942fcdd70",
    "maxdatavolumeslimit": 100,
    "maxguestslimit": 1024,
    "maxhostspercluster": 64,
    "securitygroupenabled": false,
    "storagemotionenabled": true,
    "vmsnapshotenabled": true
  }
}
  1. Add capabilities for a patch version which does not exist (specifying one capability, other will be copied from the parent version)
(localcloud) 🐱 > update hypervisorcapabilities hypervisor=VMware hypervisorversion="8.0.0.4" maxdatavolumeslimit=64
{
  "hypervisorcapabilities": {
    "hypervisor": "VMware",
    "hypervisorversion": "8.0.0.4",
    "id": "9cca3a28-da41-4a9b-9dbc-86db434ebbf8",
    "maxdatavolumeslimit": 64,
    "maxguestslimit": 1024,
    "maxhostspercluster": 64,
    "securitygroupenabled": false,
    "storagemotionenabled": true,
    "vmsnapshotenabled": true
  }
}
(localcloud) 🐱 > list hypervisorcapabilities id=9cca3a28-da41-4a9b-9dbc-86db434ebbf8
{
  "count": 1,
  "hypervisorCapabilities": [
    {
      "hypervisor": "VMware",
      "hypervisorversion": "8.0.0.4",
      "id": "9cca3a28-da41-4a9b-9dbc-86db434ebbf8",
      "maxdatavolumeslimit": 64,
      "maxguestslimit": 1024,
      "maxhostspercluster": 64,
      "securitygroupenabled": false,
      "storagemotionenabled": true,
      "vmsnapshotenabled": true
    }
  ]
}

How did you try to break this feature and the system with this change?

@codecov
Copy link

codecov bot commented Jan 9, 2024

Codecov Report

Attention: Patch coverage is 0% with 49 lines in your changes missing coverage. Please review.

Project coverage is 14.95%. Comparing base (ca59720) to head (ff9a5f5).
Report is 69 commits behind head on 4.19.

Files Patch % Lines
...in/java/com/cloud/server/ManagementServerImpl.java 0.00% 32 Missing ⚠️
...com/cloud/hypervisor/HypervisorCapabilitiesVO.java 0.00% 11 Missing ⚠️
.../admin/config/UpdateHypervisorCapabilitiesCmd.java 0.00% 6 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##              4.19    #8475       +/-   ##
============================================
+ Coverage     4.30%   14.95%   +10.65%     
- Complexity       0    11016    +11016     
============================================
  Files          363     5382     +5019     
  Lines        29312   470175   +440863     
  Branches      5118    60220    +55102     
============================================
+ Hits          1261    70321    +69060     
- Misses       27908   392065   +364157     
- Partials       143     7789     +7646     
Flag Coverage Δ
uitests 4.28% <ø> (-0.02%) ⬇️
unittests 15.66% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sureshanaparti
Copy link
Contributor

Hi @shwstppr Is this PR targeted for 4.19.1? If so, please rebase and update target branch.

version

hypervisor and hypervisorversion parameter added to the
updateHypervisorCapabilities API.

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
@shwstppr shwstppr changed the base branch from main to 4.19 April 18, 2024 11:57
@shwstppr shwstppr force-pushed the add-updhycapapi-param branch from 9503ce3 to 308819c Compare April 18, 2024 11:59
@shwstppr
Copy link
Contributor Author

@sureshanaparti done

@blueorangutan package

@blueorangutan
Copy link

@shwstppr a [SL] 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
Copy link

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

@shwstppr shwstppr marked this pull request as ready for review June 12, 2024 07:29
@shwstppr
Copy link
Contributor Author

@blueorangutan package

@blueorangutan
Copy link

@shwstppr a [SL] 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.

Copy link
Contributor

@sureshanaparti sureshanaparti left a comment

Choose a reason for hiding this comment

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

clgtm

@blueorangutan
Copy link

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

@kiranchavala kiranchavala self-assigned this Jun 12, 2024
@kiranchavala
Copy link
Member

@blueorangutan package

@blueorangutan
Copy link

@kiranchavala a [SL] 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
Copy link

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

Copy link
Contributor

@JoaoJandre JoaoJandre left a comment

Choose a reason for hiding this comment

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

CLGTM, did not test it

Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

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

just one remark about the parameter description, looks good otherwise

Co-authored-by: dahn <daan.hoogland@gmail.com>
Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

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

tlgtm

@shwstppr
Copy link
Contributor Author

@kiranchavala added some tests I did with the API in the PR description

Co-authored-by: Henrique Sato <henriquesato2003@gmail.com>
@DaanHoogland
Copy link
Contributor

@blueorangutan package

@blueorangutan
Copy link

@DaanHoogland a [SL] 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
Copy link

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

@DaanHoogland
Copy link
Contributor

@blueorangutan test

@blueorangutan
Copy link

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

@blueorangutan
Copy link

[SF] Trillian Build Failed (tid-10471)

@vishesh92
Copy link
Member

@blueorangutan test

@blueorangutan
Copy link

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

@blueorangutan
Copy link

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

Test Result Time (s) Test File
test_02_trigger_shutdown Failure 341.67 test_safe_shutdown.py

}

hpvCapabilities = _hypervisorCapabilitiesDao.createForUpdate(id);
hpvCapabilities = _hypervisorCapabilitiesDao.createForUpdate(hpvCapabilities.getId());
Copy link
Member

@vishesh92 vishesh92 Jun 21, 2024

Choose a reason for hiding this comment

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

Need to set id = hpvCapabilities.getId().
id is being used in L5156 to update the hypervisor capabilities. If id is not passed in the request, update is not getting triggered if the value already exists because id is null. This is also resulting in not getting any output.

(localcloud) __ > update hypervisorcapabilities hypervisor=VMware hypervisorversion="8.0.0.15" maxdatavolumeslimit=640
__ Error: (HTTP 530, error code 9999) Failed to update hypervisor capabilities

Executing this for the first time creates an entry which is a duplicate of the parent hypervisor. And subsequent command just fails.

@shwstppr
Copy link
Contributor Author

@blueorangutan package

@blueorangutan
Copy link

@shwstppr a [SL] 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.

@vishesh92
Copy link
Member

@blueorangutan package

@blueorangutan
Copy link

@vishesh92 a [SL] 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.

Copy link
Member

@vishesh92 vishesh92 left a comment

Choose a reason for hiding this comment

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

code looks good to me.
Tested locally with Simulator. API is working as expected.

@blueorangutan
Copy link

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

@sureshanaparti sureshanaparti merged commit de266a4 into apache:4.19 Jun 24, 2024
@sureshanaparti sureshanaparti deleted the add-updhycapapi-param branch June 24, 2024 09:11
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Jul 2, 2024
…d version (apache#8475)

* api,server: allow updating hypervisor capabilities with hypervisor and
version

hypervisor and hypervisorversion parameter added to the
updateHypervisorCapabilities API.

* param description

* Update server/src/main/java/com/cloud/server/ManagementServerImpl.java

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
Co-authored-by: dahn <daan.hoogland@gmail.com>
Co-authored-by: Henrique Sato <henriquesato2003@gmail.com>
Co-authored-by: Vishesh <vishesh92@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

8 participants