Skip to content

api,server: encryptformat in volume response - #9106

Merged
kiranchavala merged 5 commits into
apache:4.19from
shapeblue:add-volumeresponse-encryptformat
Jun 11, 2024
Merged

api,server: encryptformat in volume response#9106
kiranchavala merged 5 commits into
apache:4.19from
shapeblue:add-volumeresponse-encryptformat

Conversation

@shwstppr

Copy link
Copy Markdown
Contributor

Description

Fixes#9014

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?

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

Fixesapache#9014
Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
@codecov

codecovBot commented May 22, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 4.30%. Comparing base (87e7c57) to head (96d8e6b).
Report is 29 commits behind head on 4.19.

Additional details and impacted files
@@ Coverage Diff @@## 4.19 #9106 +/- ##
============================================
- Coverage 14.96% 4.30% -10.67% 
============================================
Files 5373 363 -5010 Lines 469005 29302 -439703 Branches 58953 5116 -53837 ============================================
- Hits 70198 1261 -68937 + Misses 391036 27898 -363138 + Partials 7771 143 -7628 
FlagCoverage Δ
uitests4.30% <ø> (-0.02%)⬇️
unittests?

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.

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

@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.

clgtm

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@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.

@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.

code lgtm

@blueorangutan

Copy link
Copy Markdown

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

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

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

TestResultTime (s)Test File
test_02_trigger_shutdownFailure341.85test_safe_shutdown.py

@sureshanapartisureshanaparti added this to the 4.19.1.0 milestone May 23, 2024

@sureshanapartisureshanaparti 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.

clgtm

Comment threadserver/src/main/java/com/cloud/api/query/vo/VolumeJoinVO.java Outdated
Comment threadserver/src/main/java/com/cloud/api/query/vo/VolumeJoinVO.java Outdated
Comment threadserver/src/main/java/com/cloud/api/query/vo/VolumeJoinVO.java Outdated
@weizhouapache

Copy link
Copy Markdown
Member

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@weizhouapache 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.

`volumes`.`path` AS `path`,
`volumes`.`chain_info` AS `chain_info`,
`volumes`.`external_uuid` AS `external_uuid`,
`volumes`.`passphrase_id` AS `passphrase_id`,

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.

Suggested change
`volumes`.`passphrase_id` AS `passphrase_id`,

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.

@JoaoJandre , are your concerns met?

@DaanHoogland no, we should apply @weizhouapache's suggestion here, no need to have this view changed for now.

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.

sorry @JoaoJandre , I think we crossed messages (with Kiran in the mix) here. I don't see passphrase_id in the PR. so that means it is applied, right?

If not we'll need to create a new PR.

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.

Yes, @DaanHoogland, it was applied in the last commit, before merging.

@blueorangutan

Copy link
Copy Markdown

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

@DaanHoogland

Copy link
Copy Markdown
Contributor

@JoaoJandre , are your concerns met?

@kiranchavalakiranchavala 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.

LGTM, In the listVolumes api the response contains

"encryptformat": "luks",

(cmk) > list volumes id=68151b4c-5967-4ce8-b747-911ad7f33c73
{
"count": 1,
"volume": [
{
"account": "admin",
"clusterid": "f9bf37c6-50a0-4cb5-ae72-5e64baa32cd2",
"clustername": "p1-c1",
"created": "2024-06-10T13:00:47+0000",
"destroyed": false,
"deviceid": 1,
"diskioread": 0,
"diskiowrite": 0,
"diskkbsread": 0,
"diskkbswrite": 0,
"diskofferingdisplaytext": "encrypted-offering",
"diskofferingid": "c401faa2-6ab7-479c-aa07-46b5786cbbe0",
"diskofferingname": "encrypted-offering",
"displayvolume": true,
"domain": "ROOT",
"domainid": "6917ae26-2724-11ef-9e8f-1e008b00026c",
"encryptformat": "luks",
"hasannotations": false,
"hypervisor": "KVM",
"id": "68151b4c-5967-4ce8-b747-911ad7f33c73",
"isextractable": true,
"name": "DATA-3",
"path": "68151b4c-5967-4ce8-b747-911ad7f33c73",
"physicalsize": 2359296,
"podid": "c46f480b-210f-455d-a31b-93eb6b8da5ed",
"podname": "Pod1",
"provisioningtype": "thin",
"quiescevm": false,
"size": 10737418240,
"state": "Ready",
"storage": "ref-trl-6784-k-Mr8-kiran-chavala-kvm-pri1",
"storageid": "299c9a7d-241c-3f54-b546-12a6bae2c537",
"storagetype": "shared",
"supportsstoragesnapshot": false,
"tags": [],
"type": "DATADISK",
"utilization": "0.0%",
"virtualmachineid": "f3ceea8a-376a-4132-a64c-57f31008eda3",
"virtualsize": 10737418240,
"vmdisplayname": "vm1",
"vmname": "vm1",
"vmstate": "Running",
"vmtype": "User",
"zoneid": "7f79d7fa-4ad3-4c00-87a7-2e7e58180757",
"zonename": "ref-trl-6784-k-Mr8-kiran-chavala"
}
]
}

Comment threadengine/schema/src/main/resources/META-INF/db/views/cloud.volume_view.sql Outdated
@shwstppr

Copy link
Copy Markdown
ContributorAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@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.

@kiranchavala
kiranchavala merged commit fcca3e8 into apache:4.19Jun 11, 2024
kiranchavala added a commit that referenced this pull request Jun 11, 2024
@blueorangutan

Copy link
Copy Markdown

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

@DaanHoogland
DaanHoogland deleted the add-volumeresponse-encryptformat branch June 11, 2024 11:55
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Jun 17, 2024
Fixesapache#9014
* api,server: encryptformat in volume response
* Apply suggestions from code review
* remove passphrase db change
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Abhishek's Bugfix List
Status: Done

Development

Successfully merging this pull request may close these issues.

add encryptionformat to volume reponse for admins

7 participants

@shwstppr@DaanHoogland@blueorangutan@weizhouapache@kiranchavala@sureshanaparti@JoaoJandre