Skip to content
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.

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

Avoid Null pointer at DomainChecker and enhance AssignVMCmd #4279

Merged
merged 3 commits into from Sep 1, 2020

Conversation

GabrielBrascher
Copy link
Member

@GabrielBrascher GabrielBrascher commented Aug 21, 2020

Description

When executing request assignVirtualMachine with null domainID and a valid projectID then a NullPointerException happens at DomainChecker.java.

Command example:

assign virtualmachine virtualmachineid=vmID projectid=projectID account=admin

The NullPointerException that is thrown at DomainChecker is handled at AssignVMCmd.java#L142, resulting in the following log message: Failed to move vm null.

My question is: does it make sense to assign VM to a project with domainID=null? If so, then the handling should be different. However, I think that it should not be null and therefore I added a proper log message.

Prior to commit

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)

How Has This Been Tested?

Request of API command assignVirtualMachine with project ID and null domainID.

assign virtualmachine virtualmachineid=vmID projectid=projectID account=admin

@davidjumani
Copy link
Contributor

@blueorangutan package

Copy link
Contributor

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

@rohityadavcloud
Copy link
Member

@blueorangutan package

@blueorangutan
Copy link

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result: ✔centos7 ✖centos8 ✔debian. JID-1817

@rohityadavcloud
Copy link
Member

@blueorangutan package

@blueorangutan
Copy link

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result: ✔centos7 ✔centos8 ✔debian. JID-1819

@rohityadavcloud
Copy link
Member

@blueorangutan package

@blueorangutan
Copy link

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result: ✔centos7 ✖centos8 ✔debian. JID-1836

@rohityadavcloud
Copy link
Member

@blueorangutan test

@blueorangutan
Copy link

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

@rohityadavcloud
Copy link
Member

Test manually rekicked

@rohityadavcloud
Copy link
Member

LGTM merging this based on Travis tests pass.

@rohityadavcloud rohityadavcloud merged commit d5acabd into apache:master Sep 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants