Skip to content

linstor: Fix template volume missing on copy node #8082

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
shwstppr merged 1 commit into from
Oct 17, 2023
Merged

linstor: Fix template volume missing on copy node #8082

shwstppr merged 1 commit into from
Oct 17, 2023

Conversation

@rp-
Copy link
Contributor

@rp- rp- commented Oct 12, 2023

Description

A TODO was overseen and never implemented,
which could trigger the following bug:

If Linstor didn't create a resource (diskless or diskfull) on the cloudstack choosen node, it would not be able to copy the template data there, it even seems no error was
triggered and the new template file silently just became empty/corrupt.

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?

Local Linstor test cluster, fix is very isolated to Linstor primary storage

@rp-
linstor: Fix template volume missing on copy node
890c60b 
A TODO was overseen and never implemented,
which could trigger the following bug:

If Linstor didn't create a resource (diskless or diskfull) on
the cloudstack choosen node, it would not be able to copy the
template data there, it even seems no error was
triggered and the new template file silently just became
empty/corrupt.
@rp- rp- force-pushed the linstor-fix-missing-make-available branch from 9a9363e to 890c60b Compare October 12, 2023 13:49
@DaanHoogland
Copy link
Contributor

DaanHoogland commented Oct 12, 2023

@blueorangutan package

@blueorangutan
Copy link

blueorangutan commented Oct 12, 2023

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

@codecov
Copy link

codecov bot commented Oct 12, 2023 •
edited
Loading

Codecov Report

Merging #8082 (890c60b) into 4.18 (3e7f21a) will increase coverage by 0.00%.
Report is 3 commits behind head on 4.18.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##               4.18    #8082   +/-   ##
=========================================
  Coverage     13.06%   13.06%           
- Complexity     9109     9110    +1     
=========================================
  Files          2720     2720           
  Lines        257526   257533    +7     
  Branches      40150    40150           
=========================================
+ Hits          33655    33657    +2     
- Misses       219644   219648    +4     
- Partials       4227     4228    +1
Files Coverage Δ
.../hypervisor/kvm/storage/LinstorStorageAdaptor.java 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

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

@DaanHoogland
Copy link
Contributor

DaanHoogland commented Oct 13, 2023

@blueorangutan test

@blueorangutan
Copy link

blueorangutan commented Oct 13, 2023

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

@blueorangutan
Copy link

blueorangutan commented Oct 13, 2023

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

Test Result Time (s) Test File
test_02_upgrade_kubernetes_cluster Failure 574.27 test_kubernetes_clusters.py
test_03_deploy_vm_wrong_checksum Error 40.70 test_templates.py
test_09_list_templates_download_details Failure 0.06 test_templates.py
test_list_vms_metrics_admin Error 3607.90 test_metrics_api.py
test_list_vms_metrics_history Error 7.49 test_metrics_api.py
test_list_volumes_metrics_history Error 4.37 test_metrics_api.py

@DaanHoogland
Copy link
Contributor

DaanHoogland commented Oct 16, 2023

@blueorangutan LLtest

@blueorangutan
Copy link

blueorangutan commented Oct 16, 2023

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

@rohityadavcloud rohityadavcloud added this to the 4.18.2.0 milestone Oct 16, 2023
rohityadavcloud
rohityadavcloud approved these changes Oct 16, 2023
View reviewed changes
Copy link
Member

@rohityadavcloud rohityadavcloud left a comment

Choose a reason for hiding this comment

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

LGTM - didn't testing, I trust @rp- has tested this as this change in linstor specific plugin

@rohityadavcloud
Copy link
Member

rohityadavcloud commented Oct 16, 2023

@blueorangutan package

@blueorangutan
Copy link

blueorangutan commented Oct 16, 2023

@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
Copy link

blueorangutan commented Oct 16, 2023

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

@DaanHoogland
Copy link
Contributor

DaanHoogland commented Oct 16, 2023

@blueorangutan package

@blueorangutan
Copy link

blueorangutan commented Oct 16, 2023

@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
Copy link

blueorangutan commented Oct 16, 2023

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

@DaanHoogland
Copy link
Contributor

DaanHoogland commented Oct 16, 2023

@blueorangutan test

@blueorangutan
Copy link

blueorangutan commented Oct 16, 2023

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

@blueorangutan
Copy link

blueorangutan commented Oct 16, 2023

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

Test Result Time (s) Test File
test_02_list_cpvm_vm Failure 0.03 test_ssvm.py
test_04_cpvm_internals Failure 0.03 test_ssvm.py
test_03_deploy_vm_wrong_checksum Error 38.45 test_templates.py
test_09_list_templates_download_details Failure 0.03 test_templates.py

@blueorangutan
Copy link

blueorangutan commented Oct 17, 2023

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

Test Result Time (s) Test File
test_03_deploy_vm_wrong_checksum Error 41.58 test_templates.py
test_09_list_templates_download_details Failure 0.04 test_templates.py

shwstppr
shwstppr approved these changes Oct 17, 2023
View reviewed changes
Copy link
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.

Code LGTM

@shwstppr shwstppr closed this Oct 17, 2023
@shwstppr shwstppr reopened this Oct 17, 2023
@shwstppr
Copy link
Contributor

shwstppr commented Oct 17, 2023

@DaanHoogland are your concerns answered sufficiently?

@shwstppr
Copy link
Contributor

shwstppr commented Oct 17, 2023

Merging this based on test results

@shwstppr shwstppr merged commit 4a86a0d into apache:4.18 Oct 17, 2023
shwstppr pushed a commit to shapeblue/cloudstack that referenced this pull request Dec 27, 2023
@rp- @rohityadavcloud
linstor: Fix template volume missing on copy node (apache#8082)
d49b0c4 
A TODO was overseen and never implemented,
which could trigger the following bug:

If Linstor didn't create a resource (diskless or diskfull) on
the cloudstack choosen node, it would not be able to copy the
template data there, it even seems no error was
triggered and the new template file silently just became
empty/corrupt.

(cherry picked from commit 4a86a0d)
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
@rp- rp- deleted the linstor-fix-missing-make-available branch February 5, 2024 08:37
@rp- rp- mentioned this pull request Feb 6, 2024
Closed
rohityadavcloud pushed a commit to shapeblue/cloudstack that referenced this pull request Mar 31, 2024
@rp- @rohityadavcloud
linstor: Fix template volume missing on copy node (apache#8082)
478c486 
A TODO was overseen and never implemented,
which could trigger the following bug:

If Linstor didn't create a resource (diskless or diskfull) on
the cloudstack choosen node, it would not be able to copy the
template data there, it even seems no error was
triggered and the new template file silently just became
empty/corrupt.

(cherry picked from commit 4a86a0d)
Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
rp- added a commit to LINBIT/cloudstack that referenced this pull request May 16, 2024
@rp-
linstor: Fix template volume missing on copy node (apache#8082)
2b6a2d5 
A TODO was overseen and never implemented,
which could trigger the following bug:

If Linstor didn't create a resource (diskless or diskfull) on
the cloudstack choosen node, it would not be able to copy the
template data there, it even seems no error was
triggered and the new template file silently just became
empty/corrupt.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DaanHoogland DaanHoogland DaanHoogland left review comments

@rohityadavcloud rohityadavcloud rohityadavcloud approved these changes

@shwstppr shwstppr shwstppr approved these changes

Assignees

No one assigned

Labels

component:primary-storage component:storage has-3rdparty-components

Projects

None yet

Milestone

4.18.2.0

Development

Successfully merging this pull request may close these issues.

5 participants

@rp- @DaanHoogland @blueorangutan @rohityadavcloud @shwstppr