kvm agent: get the right controll cidr by DaanHoogland · Pull Request #7580 · apache/cloudstack

DaanHoogland · 2023-06-01T12:07:29Z

Description

This PR ...

Fixes: #7574

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

Screenshots (if appropriate):

How Has This Been Tested?

codecov · 2023-06-01T13:40:36Z

Codecov Report

Merging #7580 (8bc4f4f) into 4.18 (4138ca7) will increase coverage by 0.08%.
The diff coverage is 46.51%.

@@             Coverage Diff              @@
##               4.18    #7580      +/-   ##
============================================
+ Coverage     12.94%   13.02%   +0.08%     
- Complexity     8953     9028      +75     
============================================
  Files          2716     2719       +3     
  Lines        256137   256845     +708     
  Branches      39944    40050     +106     
============================================
+ Hits          33146    33457     +311     
- Misses       218831   219201     +370     
- Partials       4160     4187      +27

Impacted Files	Coverage Δ
...om/cloud/hypervisor/kvm/resource/IvsVifDriver.java	`0.00% <0.00%> (ø)`
...KubernetesClusterResourceModifierActionWorker.java	`0.00% <ø> (ø)`
...er/actionworkers/KubernetesClusterScaleWorker.java	`0.00% <0.00%> (ø)`
...bernetes/cluster/KubernetesClusterManagerImpl.java	`4.41% <69.56%> (+4.41%)`	⬆️
...cloud/hypervisor/kvm/resource/BridgeVifDriver.java	`9.63% <100.00%> (+0.07%)`	⬆️
...om/cloud/hypervisor/kvm/resource/OvsVifDriver.java	`15.32% <100.00%> (+0.22%)`	⬆️
...m/cloud/hypervisor/kvm/resource/VifDriverBase.java	`60.00% <100.00%> (+6.15%)`	⬆️

... and 41 files with indirect coverage changes

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

wido

So far it looks good. I'm going on holiday for two weeks now, but this is a fix in the right direction

DaanHoogland · 2023-06-02T10:42:20Z

So far it looks good. I'm going on holiday for two weeks now, but this is a fix in the right direction

any colleague that could test this, @wido ?

DaanHoogland · 2023-06-05T14:40:47Z

@blueorangutan package

blueorangutan · 2023-06-05T14:42:03Z

@DaanHoogland a [LL] 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 · 2023-06-05T15:49:08Z

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

DaanHoogland · 2023-06-06T11:44:50Z

@blueorangutan test

blueorangutan · 2023-06-06T11:46:03Z

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

blueorangutan · 2023-06-06T23:20:37Z

[SF] Trillian test result (tid-6695)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 40177 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7580-t6695-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_hostha_enable_ha_when_host_in_maintenance	`Error`	303.81	test_hostha_kvm.py

DaanHoogland · 2023-06-22T15:06:25Z

ping @wido can you test?

wido · 2023-06-22T15:35:18Z

ping @wido can you test?

Yes, I will. Just got back from holiday.

wido · 2023-06-23T13:47:00Z

So far this looks good to me!

wido

I think we are almost there!

Rubueno · 2023-06-29T08:45:16Z

@wido can you verify if this also fixes the problem mentioned in #6715? To verify if it is fixed now: bootstrap the management server with /16, then change to /20 and start a system vm.

wido · 2023-06-30T07:52:52Z

@wido can you verify if this also fixes the problem mentioned in #6715? To verify if it is fixed now: bootstrap the management server with /16, then change to /20 and start a system vm.

No, it doesn't. This PR is to fix the subnet assigned to the cloud0 bridge on the KVM agent.

What you mention is something I observed as well and didn't put much effort into that.

DaanHoogland · 2023-07-04T08:54:41Z

@blueorangutan package

blueorangutan · 2023-07-04T08:56:03Z

@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 · 2023-07-04T09:47:46Z

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

DaanHoogland · 2023-07-04T09:48:28Z

@blueorangutan test

blueorangutan · 2023-07-04T09:50:03Z

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

blueorangutan · 2023-07-04T21:47:37Z

[SF] Trillian test result (tid-6977)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 41497 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7580-t6977-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_01_migrate_VM_and_root_volume	`Error`	83.80	test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks	`Error`	51.25	test_vm_life_cycle.py

wido

This looks good to me!

DaanHoogland · 2023-07-06T06:23:57Z

@weizhouapache can you have a look?

DaanHoogland · 2023-07-19T08:36:36Z

@wido can you verify if this also fixes the problem mentioned in #6715? To verify if it is fixed now: bootstrap the management server with /16, then change to /20 and start a system vm.

No, it doesn't. This PR is to fix the subnet assigned to the cloud0 bridge on the KVM agent.

What you mention is something I observed as well and didn't put much effort into that.

@wido, @Rubueno , can you have a look at #7746 ? I intend to solve that issue there.

DaanHoogland added 2 commits June 1, 2023 14:03

get the right controll cidr

e30cee7

restore old property

d992ffa

boring-cyborg Bot added component:agent component:kvm labels Jun 1, 2023

DaanHoogland mentioned this pull request Jun 1, 2023

kvm: Agent does not honor control.cidr anymore #7574

Closed

DaanHoogland added 2 commits June 1, 2023 14:34

parameter naming convention

25863f2

rat

44752f1

DaanHoogland requested a review from wido June 1, 2023 14:17

DaanHoogland closed this Jun 1, 2023

DaanHoogland reopened this Jun 1, 2023

wido reviewed Jun 1, 2023

View reviewed changes

Comment thread plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/BridgeVifDriver.java Outdated

DaanHoogland marked this pull request as ready for review June 8, 2023 09:51

weizhouapache added this to the 4.18.1.0 milestone Jun 20, 2023

DaanHoogland linked an issue Jun 22, 2023 that may be closed by this pull request

kvm: Agent does not honor control.cidr anymore #7574

Closed

single-l

d4da6fa

wido requested changes Jun 26, 2023

View reviewed changes

Comment thread plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/VifDriverBase.java Outdated

Comment thread plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/VifDriverBase.java Outdated

weizhouapache changed the title ~~get the right controll cidr~~ kvm agent: get the right controll cidr Jun 26, 2023

weizhouapache added the Severity:Critical Critical bug label Jun 26, 2023

weizhouapache mentioned this pull request Jun 29, 2023

ControlNetworkGuru does not take into account custom CIDR when assigning System VM link local IP address #6715

Open

default should come from property

41b27a1

DaanHoogland requested a review from wido July 5, 2023 07:25

wido reviewed Jul 5, 2023

View reviewed changes

Comment thread plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/VifDriverBase.java Outdated

params removed

8bc4f4f

wido approved these changes Jul 5, 2023

View reviewed changes

DaanHoogland requested a review from weizhouapache July 6, 2023 06:23

yadvr approved these changes Jul 7, 2023

View reviewed changes

yadvr merged commit 2752c49 into apache:4.18 Jul 7, 2023

yadvr deleted the control.cidr branch July 7, 2023 17:28

Conversation

DaanHoogland commented Jun 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

codecov Bot commented Jun 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

wido left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DaanHoogland commented Jun 2, 2023

Uh oh!

DaanHoogland commented Jun 5, 2023

Uh oh!

blueorangutan commented Jun 5, 2023

Uh oh!

blueorangutan commented Jun 5, 2023

Uh oh!

DaanHoogland commented Jun 6, 2023

Uh oh!

blueorangutan commented Jun 6, 2023

Uh oh!

blueorangutan commented Jun 6, 2023

Uh oh!

DaanHoogland commented Jun 22, 2023

Uh oh!

wido commented Jun 22, 2023

Uh oh!

wido commented Jun 23, 2023

Uh oh!

wido left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Rubueno commented Jun 29, 2023

Uh oh!

wido commented Jun 30, 2023

Uh oh!

DaanHoogland commented Jul 4, 2023

Uh oh!

blueorangutan commented Jul 4, 2023

Uh oh!

blueorangutan commented Jul 4, 2023

Uh oh!

DaanHoogland commented Jul 4, 2023

Uh oh!

blueorangutan commented Jul 4, 2023

Uh oh!

blueorangutan commented Jul 4, 2023

Uh oh!

Uh oh!

wido left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Jul 6, 2023

Uh oh!

DaanHoogland commented Jul 19, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

DaanHoogland commented Jun 1, 2023 •

edited

Loading

codecov Bot commented Jun 1, 2023 •

edited

Loading