Skip to content

agent: enable ssl only for kvm agent (not in system vms) - #6371

Merged
nvazquez merged 4 commits into
apache:mainfrom
weizhouapache:4.17-fix-ssvm-ssl
May 12, 2022
Merged

agent: enable ssl only for kvm agent (not in system vms)#6371
nvazquez merged 4 commits into
apache:mainfrom
weizhouapache:4.17-fix-ssvm-ssl

Conversation

@weizhouapache

Copy link
Copy Markdown
Member

Description

This PR fixes the issue that template cannot be downloaded from https site in 4.17.0.0 RC2.

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

Screenshots (if appropriate):

How Has This Been Tested?

@weizhouapache

Copy link
Copy Markdown
MemberAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

@weizhouapache a 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
Copy Markdown

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

@nvazquez

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

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

if (!LIBVIRT_COMPUTING_RESOURCE.equalsIgnoreCase(resource)) {
s_logger.info("This is not a cloudstack kvm agent, ignoring");
return;
}

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.

@nvazquez@weizhouapache Could this cause an issue with (a) direct download, (b) ssl enabled cpvm and ssvm. Should we attempt to fix the issue by importing or referencing the system keystore too where java/ca certs are stored and imported (for x1/letsencrypt) @Pearl1594 ?

@weizhouapacheweizhouapacheMay 10, 2022

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

@rohityadavcloud@nvazquez@Pearl1594
this part of code was introduced in #6200 which aims to fix an issue with direct download on KVM.
with this pr, SSL is still enabled on KVM hosts, but not in system VMs where Java process load key store realhostip.keystone (same as the behavior without #6200 )

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.

Makes sense if this was relatively newer code.

@weizhouapache

Copy link
Copy Markdown
MemberAuthor

@blueorangutan ubuntu20 kvm-ubuntu20

@nvazqueznvazquez added this to the 4.17.0.0 milestone May 11, 2022
@nvazquez

Copy link
Copy Markdown
Contributor

Verified HTTPS templates working on KVM env with this fix

return;
}
final String resource = getProperty(null, "resource");
if (!LIBVIRT_COMPUTING_RESOURCE.equalsIgnoreCase(resource)) {

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.

The use of LIBVIRT_COMPUTING_RESOURCE or com.cloud.hypervisor.kvm.resource.LibvirtComputingResource may be misleading (for ex. what if this isn't the resource name? for example, we do support lxc etc?).

Can you simply add a check if the agent is either is in systemvm (for ex. /etc/cloudstack-release would exist) or if we're on a KVM host (check for /dev/kvm exists or libvirt running)? (Depending on what we're trying to achieve here). The other option usually is to check the resource class via some method.

What do you think @weizhouapache@nvazquez ?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

@rohityadavcloud
I will move all these codes to LibvirtComputingResource which makes more sense.
I will verify the issue fixed by #6200 as well.

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.

+1

@blueorangutan

Copy link
Copy Markdown

Trillian test result (tid-4158)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 36897 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6371-t4158-kvm-centos7.zip
Smoke tests completed. 96 look OK, 0 have errors
Only failed tests results shown below:

TestResultTime (s)Test File

@acs-robot

Copy link
Copy Markdown

Found UI changes, kicking a new UI QA build
@blueorangutan ui

@blueorangutan

Copy link
Copy Markdown

@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6371 (SL-JID-1547)

@weizhouapache

Copy link
Copy Markdown
MemberAuthor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

@sonarqubecloud

Copy link
Copy Markdown

@blueorangutan

Copy link
Copy Markdown

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

@nvazquez

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

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

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

tested - LGTM

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

LGTM

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

LGTM, manually tested

@blueorangutan

Copy link
Copy Markdown

Trillian test result (tid-4161)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35729 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6371-t4161-kvm-centos7.zip
Smoke tests completed. 83 look OK, 5 have errors
Only failed tests results shown below:

TestResultTime (s)Test File
test_05_stop_ssvmError29.77test_ssvm.py
test_06_stop_cpvmError3.00test_ssvm.py
test_07_reboot_ssvmError3.00test_ssvm.py
test_08_reboot_cpvmError3.00test_ssvm.py
test_09_reboot_ssvm_forcedError3.00test_ssvm.py
test_10_reboot_cpvm_forcedError3.00test_ssvm.py
test_11_destroy_ssvmError3.00test_ssvm.py
test_12_destroy_cpvmError3.00test_ssvm.py
test_13_ss_nfs_version_on_ssvmError3.00test_ssvm.py
test_03_deploy_and_scale_kubernetes_clusterFailure27.97test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_clusterFailure57.66test_kubernetes_clusters.py
test_08_upgrade_kubernetes_ha_clusterFailure34.02test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_clusterFailure31.93test_kubernetes_clusters.py
ContextSuite context=TestKubernetesCluster>:teardownError116.86test_kubernetes_clusters.py
ContextSuite context=TestPrivateVlansL2Networks>:setupError836.82test_network.py
test_hostha_enable_ha_when_host_disconectedError788.40test_hostha_kvm.py
test_hostha_enable_ha_when_host_in_maintenanceError303.82test_hostha_kvm.py
test_01_deploy_vm_on_specific_hostError1.16test_vm_deployment_planner.py
test_04_deploy_vm_on_host_override_pod_and_clusterError0.20test_vm_deployment_planner.py

@nvazquez

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

Trillian test result (tid-4166)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 37350 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6371-t4166-kvm-centos7.zip
Smoke tests completed. 96 look OK, 0 have errors
Only failed tests results shown below:

TestResultTime (s)Test File

@nvazquez
nvazquez merged commit 8f39a04 into apache:mainMay 12, 2022
Pearl1594 pushed a commit to shapeblue/cloudstack that referenced this pull request Sep 6, 2022
apache#151)
* agent: enable ssl only for kvm agent (not in system vms)
* Revert "agent: enable ssl only for kvm agent (not in system vms)"
This reverts commit b2d76ba.
* Revert "KVM: Enable SSL if keystore exists (apache#6200)"
This reverts commit 4525f8c.
* KVM: Enable SSL if keystore exists in LibvirtComputingResource.java
Co-authored-by: Wei Zhou <weizhou@apache.org>
@weizhouapache
weizhouapache deleted the 4.17-fix-ssvm-ssl branch December 9, 2022 10:07
weizhouapache added a commit to weizhouapache/cloudstack that referenced this pull request Aug 30, 2023
…en enable SSL for kvm agent
This was implemented in PR#6200 and apache#6371 , but broken in PR#6348
DaanHoogland pushed a commit to weizhouapache/cloudstack that referenced this pull request Oct 18, 2023
…en enable SSL for kvm agent
This was implemented in PR#6200 and apache#6371 , but broken in PR#6348
Sign up for freeto 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.

7 participants

@weizhouapache@blueorangutan@nvazquez@acs-robot@yadvr@Pearl1594@slavkap