Skip to content

VMware plugin: fix check_mac_address() for vm.config not present#2030

Closed
swenske wants to merge 0 commit intobareos:masterfrom
swenske:master
Closed

VMware plugin: fix check_mac_address() for vm.config not present#2030
swenske wants to merge 0 commit intobareos:masterfrom
swenske:master

Conversation

@swenske
Copy link

@swenske swenske commented Nov 21, 2024 •
edited by sduehr
Loading

Avoid errors with VM without a config attribute (eg: a VM being cloned)

Thank you for contributing to the Bareos Project!

Please check

  • Short description and the purpose of this PR is present above this paragraph
  • Your name is present in the AUTHORS file (optional)

If you have any questions or problems, please give a comment in the PR.

Helpful documentation and best practices

Checklist for the reviewer of the PR (will be processed by the Bareos team)

Make sure you check/merge the PR using devtools/pr-tool to have some simple automated checks run and a proper changelog record added.

General
  • Is the PR title usable as CHANGELOG entry?
  • Purpose of the PR is understood
  • Commit descriptions are understandable and well formatted
  • Required backport PRs have been created
  • Correct milestone is set
Source code quality
  • Source code changes are understandable
  • Variable and function names are meaningful
  • Code comments are correct (logically and spelling)

@swenske
Copy link
Author

swenske commented Nov 21, 2024

When the vmware plugin recreates a VM it checks all MAC addresses to avoid a conflict.
If a cloning (or maybe also migration) task is in progress, the destination VM does not have a config yet and the following error occurs:

2024-11-20 15:53:18.874 | DEBUG | bareos-fd-vmware:check_mac_address:3131 - !!! vm: 'vim.VirtualMachine:vm-233350'
2024-11-20 15:53:18.878 | DEBUG | bareos-fd-vmware:check_mac_address:3132 - !!! vm.config: None
bareos-disk-sd-int-fr2.storage.xxxx.xxxx.xx (150): include/python_plugins_common.inc:125-30 bareosfd: Traceback (most recent call last):
 File "/usr/lib/bareos/plugins/BareosFdWrapper.py", line 59, in handle_plugin_event
 return bareos_fd_plugin_object.handle_plugin_event(event)
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/usr/lib/bareos/plugins/bareos-fd-vmware.py", line 1076, in handle_plugin_event
 return self.start_restore_job()
 ^^^^^^^^^^^^^^^^^^^^^^
 File "/usr/lib/bareos/plugins/bareos-fd-vmware.py", line 618, in start_restore_job
 return self.vadp.prepare_vm_restore()
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 File "/usr/lib/bareos/plugins/bareos-fd-vmware.py", line 1549, in prepare_vm_restore
 if not self.create_vm():
 ^^^^^^^^^^^^^^^
 File "/usr/lib/bareos/plugins/bareos-fd-vmware.py", line 1883, in create_vm
 self.check_mac_address(config_info)
 File "/usr/lib/bareos/plugins/bareos-fd-vmware.py", line 3133, in check_mac_address
 for dev in vm.config.hardware.device:
 ^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'hardware'

@swenske swenske changed the title fix: AttributeError: 'NoneType' object has no attribute 'hardware' bareos-fd-vmware.py: fix: AttributeError: 'NoneType' object has no attribute 'hardware' Nov 22, 2024
@arogge arogge requested a review from sduehr December 3, 2024 10:50
@arogge arogge added bugfix bug This addresses a bug labels Dec 3, 2024
@bruno-at-bareos
Copy link
Contributor

bruno-at-bareos commented Dec 9, 2024

Our CI report the following issues

[2024-12-09T12:49:58.060Z]  ✗  Commit checks failed:

[2024-12-09T12:49:58.060Z] 	1374799f3 fix: AttributeError: 'NoneType' object has no attribute 'hardware': headline too long (66 > 60)

[2024-12-09T12:49:58.060Z] 	1374799f3 fix: AttributeError: 'NoneType' object has no attribute 'hardware': body contains line longer 72 chars

[2024-12-09T12:49:58.061Z]  ✗  bareos-check-sources --since=12624050ca36025cac38db74fcbd84907983191a reported:

[2024-12-09T12:49:58.061Z] 	Plugin 'python black' would modify core/src/plugins/filed/python/vmware/bareos-fd-vmware.py'

Would be nice to have them fixed before the start of the review.
You can always run them from your PR (see in devtools/)

@swenske swenske changed the title bareos-fd-vmware.py: fix: AttributeError: 'NoneType' object has no attribute 'hardware' bareos-fd-vmware.py: fix: 'NoneType' object has no attribute 'hardware' Dec 9, 2024
sduehr
sduehr requested changes Dec 11, 2024
View reviewed changes
Copy link
Member

@sduehr sduehr left a comment

Choose a reason for hiding this comment

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

Thanks for you contribution, I just requested a small change to make the code better readable.

You are right, vm.config may not be present, the API docs also explains that.

@sduehr
Copy link
Member

sduehr commented Dec 12, 2024

I changed the title so that it is usable as a CHANGELOG entry.

The commit checks still fail, the headline of a commit message must not be longer than 60 characters, and lines in the body of the commit message must not be longer than 72 characters.

Could you please rebase your git branch and cleanup (squash) the commits? I'd suggest to use
VMware plugin: fix check_mac_address()
as the commit message headline.

See also https://docs.bareos.org/DeveloperGuide/gitworkflow.html
Note that a separate commit for the CHANGELOG entry is no longer necessary, this is automatically taken from the PR title now, we still have to remove that from the documentation.

And if you want, please add you name to the AUTHORS file.

@sduehr sduehr changed the title bareos-fd-vmware.py: fix: 'NoneType' object has no attribute 'hardware' VMware plugin: fix check_mac_address() for vm.config not present Dec 12, 2024
@swenske
Copy link
Author

swenske commented Dec 12, 2024 •
edited
Loading

I got a little confused with git, I hope I did it correctly.

edit: apparently not

@swenske swenske closed this Dec 12, 2024
@swenske swenske mentioned this pull request Dec 12, 2024
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sduehr sduehr sduehr requested changes

Assignees

@sduehr sduehr

Labels

bug This addresses a bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@swenske @bruno-at-bareos @sduehr @arogge