guarantee MAC uniqueness by DaanHoogland · Pull Request #7634 · apache/cloudstack

DaanHoogland · 2023-06-15T12:22:33Z

Description

This PR makes sure a new MAC is unique by

using zone DB ID as default ZoneMacIdentifyer
checking if the mac exists in the nics table
if the mac exists a new one will be generated untill a free one is found.

This is a fix that will not generate duplicate MAC addresses in the future and prevent duplicates if prior versions generated randon MACs before.

Fixes: #7633

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?

The testing of this feature is dependend on randomly generated MAC addresses that are in the way when generating logicly deterministic. Any testing suggestions are appreciated.

codecov · 2023-06-16T08:23:28Z

Codecov Report

Merging #7634 (016c550) into 4.18 (1aa4f80) will increase coverage by 0.03%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##               4.18    #7634      +/-   ##
============================================
+ Coverage     12.98%   13.02%   +0.03%     
- Complexity     8989     9028      +39     
============================================
  Files          2716     2719       +3     
  Lines        256343   256846     +503     
  Branches      39981    40051      +70     
============================================
+ Hits          33291    33448     +157     
- Misses       218886   219211     +325     
- Partials       4166     4187      +21

Impacted Files	Coverage Δ
...ter/entity/api/db/dao/EngineDataCenterDaoImpl.java	`0.00% <ø> (ø)`
.../src/main/java/com/cloud/dc/dao/DataCenterDao.java	`0.00% <ø> (ø)`
.../main/java/com/cloud/dc/dao/DataCenterDaoImpl.java	`18.81% <ø> (+0.63%)`	⬆️
...ain/java/com/cloud/network/dao/NetworkDaoImpl.java	`3.21% <ø> (+<0.01%)`	⬆️
...ema/src/main/java/com/cloud/vm/dao/NicDaoImpl.java	`23.50% <ø> (+0.11%)`	⬆️
...a/com/cloud/hypervisor/hyperv/guru/HypervGuru.java	`0.00% <0.00%> (ø)`
...com/cloud/hypervisor/guru/VmwareVmImplementer.java	`0.00% <0.00%> (ø)`
...a/com/cloud/network/vm/NetScalerVMManagerImpl.java	`0.00% <0.00%> (ø)`
.../main/java/com/cloud/network/NetworkModelImpl.java	`11.11% <0.00%> (-0.05%)`	⬇️
...ava/com/cloud/network/guru/ControlNetworkGuru.java	`4.81% <0.00%> (ø)`

... and 41 files with indirect coverage changes

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

weizhouapache

code lgtm

@DaanHoogland
Ihink the title needs to be updated

BryanMLima

LGTM. I just made some suggestions over some typos around the code.

Co-authored-by: Bryan Lima <42067040+BryanMLima@users.noreply.github.com>

DaanHoogland · 2023-07-05T12:35:50Z

@blueorangutan package

blueorangutan · 2023-07-05T12:38: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-05T13:31:44Z

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

DaanHoogland · 2023-07-06T06:37:26Z

@blueorangutan test

blueorangutan · 2023-07-06T06:38:04Z

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

yadvr · 2023-07-07T17:26:33Z

ping @shwstppr @nvazquez for review
@blueorangutan test

blueorangutan · 2023-07-07T17:28:03Z

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

DaanHoogland · 2023-07-13T10:48:11Z

@blueorangutan test

blueorangutan · 2023-07-13T10:50:03Z

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

DaanHoogland · 2023-07-17T07:34:42Z

@blueorangutan package

blueorangutan · 2023-07-17T07:36: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-17T08:27:55Z

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

yadvr · 2023-07-18T07:03:15Z

@blueorangutan test

blueorangutan · 2023-07-18T07:04:04Z

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

blueorangutan · 2023-07-18T19:20:57Z

[SF] Trillian test result (tid-7098)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 42717 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7634-t7098-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`	78.56	test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks	`Error`	55.37	test_vm_life_cycle.py

* 4.18: Storage and volumes statistics tasks for StorPool primary storage (#7404) proper storage construction (#6797) guarantee MAC uniqueness (#7634) server: allow migration of all VMs with local storage on KVM (#7656) Add L2 networks to Zones with SG (#7719)

use zone DB ID as default ZoneMacIdentifyer

f78650c

boring-cyborg Bot added component:api component:networking labels Jun 15, 2023

DaanHoogland requested a review from weizhouapache June 15, 2023 12:22

remove unused methods

e909e39

boring-cyborg Bot added the component:orchestration label Jun 15, 2023

weizhouapache reviewed Jun 15, 2023

View reviewed changes

Comment thread server/src/main/java/com/cloud/network/NetworkModelImpl.java Outdated

rename occurances of NetworkModel from mgr to model

423b5b4

boring-cyborg Bot added component:hyperv component:vmware labels Jun 15, 2023

remove unused code

a6d84b1

boring-cyborg Bot added the component:database label Jun 15, 2023

apache deleted a comment from blueorangutan Jun 15, 2023

imports

6c7b6c3

apache deleted a comment from blueorangutan Jun 16, 2023

DaanHoogland requested a review from weizhouapache June 16, 2023 09:09

check for existence of MAC in NICs table

884731e

weizhouapache approved these changes Jun 19, 2023

View reviewed changes

DaanHoogland changed the title ~~use zone DB ID as default ZoneMacIdentifyer~~ garantee MAC uniqueness Jun 19, 2023

DaanHoogland marked this pull request as ready for review June 19, 2023 09:17

weizhouapache added this to the 4.18.1.0 milestone Jun 20, 2023

BryanMLima approved these changes Jun 20, 2023

View reviewed changes

Comment thread api/src/main/java/com/cloud/network/NetworkModel.java Outdated

Comment thread server/src/main/java/com/cloud/network/NetworkModelImpl.java Outdated

Apply suggestions from code review

9ccc3f7

Co-authored-by: Bryan Lima <42067040+BryanMLima@users.noreply.github.com>

apache deleted a comment from blueorangutan Jun 21, 2023

weizhouapache added the Severity:Major label Jun 26, 2023

yadvr requested review from nvazquez and shwstppr June 27, 2023 09:26

apache deleted a comment from blueorangutan Jul 5, 2023

DaanHoogland added the status:ready-for-merge label Jul 11, 2023

shwstppr reviewed Jul 13, 2023

View reviewed changes

Comment thread server/src/main/java/com/cloud/network/NetworkModelImpl.java Outdated

Update server/src/main/java/com/cloud/network/NetworkModelImpl.java

016c550

DaanHoogland merged commit 73a269e into apache:4.18 Jul 19, 2023

weizhouapache mentioned this pull request Jul 26, 2023

Duplicated mac addresses in a zone #7633

Closed

DaanHoogland removed the status:ready-for-merge label Dec 4, 2024

Conversation

DaanHoogland commented Jun 15, 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!

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

weizhouapache left a comment

Choose a reason for hiding this comment

Uh oh!

BryanMLima left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

DaanHoogland commented Jul 5, 2023

Uh oh!

blueorangutan commented Jul 5, 2023

Uh oh!

blueorangutan commented Jul 5, 2023

Uh oh!

DaanHoogland commented Jul 6, 2023

Uh oh!

blueorangutan commented Jul 6, 2023

Uh oh!

yadvr commented Jul 7, 2023

Uh oh!

blueorangutan commented Jul 7, 2023

Uh oh!

DaanHoogland commented Jul 13, 2023

Uh oh!

blueorangutan commented Jul 13, 2023

Uh oh!

Uh oh!

DaanHoogland commented Jul 17, 2023

Uh oh!

blueorangutan commented Jul 17, 2023

Uh oh!

blueorangutan commented Jul 17, 2023

Uh oh!

yadvr commented Jul 18, 2023

Uh oh!

blueorangutan commented Jul 18, 2023

Uh oh!

blueorangutan commented Jul 18, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

DaanHoogland commented Jun 15, 2023 •

edited

Loading

codecov Bot commented Jun 16, 2023 •

edited

Loading

BryanMLima left a comment •

edited

Loading