-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Improving code related to the Agent properties #6348
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
Conversation
PR Analysishttps://sonarcloud.io/summary/new_code?id=apachecloudstack&pullRequest=6348 PR Coverage Report
|
DaanHoogland
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good enhancement/cleanup
|
@paulazomig can you merge the latest main to your PR branch? |
agent/src/main/java/com/cloud/agent/properties/AgentPropertiesFileHandler.java
Outdated
Show resolved
Hide resolved
...ypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
Outdated
Show resolved
Hide resolved
...ypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
Outdated
Show resolved
Hide resolved
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
|
@paulazomig youĺl need to solve a conflict |
|
SonarCloud Quality Gate failed. |
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
…agent-properties
…agent-properties
…esource#configure
|
Found UI changes, kicking a new UI QA build |
|
@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
|
UI build: ✔️ |
please have a look at these @paulazomig . There are certainly some that can be improved. |
@DaanHoogland, that message is printed when the secondary storage is not mounted in the SSVM; then, the script Although the script problem, the main situation is that the secondary storage is not being mounted in the SSVM. Would it be possible for you to share the SSVM logs? This way we would have a better understanding of why the storage is not being mounted. |
I am building an environment for this again. I take it it isn´t happening in your environment? |
|
Trillian Build Failed (tid-81) |
I think I need to find time to enable debug logging and retry, @GutoVeronezi . I see nothing of interest there. |
Yes, perhaps due to divergent configurations or something I am missing. I will review the code and analyze the logs you shared (thanks, though). Maybe the message |
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
|
SonarCloud Quality Gate failed. |
|
@blueorangutan package |
|
@GutoVeronezi 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. |
|
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 4966 |
|
@DaanHoogland I think I found out what was happening. When patching the system VMs, the file As I made a change to remove those hardcoded properties when the system VM is a SSVM and to copy the content of file Following there is the content of the @DaanHoogland, can we run the tests again? |
|
@blueorangutan test |
|
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-5522)
|
|
@GutoVeronezi @JoaoJandre @nvazquez are we alright with this now? |
|
@DaanHoogland, I think that, with the last commit, we have all the cases addressed. If there is something I am missing, please, let me know. |
|
@DaanHoogland @GutoVeronezi I couldn't finish my review yet but it looks good so far! I would like to double check the agent.properties refactor so we don't miss any existing property |
thanks @nvazquez , let us know your verdict ;) |
nvazquez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM













Description
The classes AgentProperties and AgentPropertiesFileHandler were previously created to map the existing properties of the agent.properties file to facilitate their use; however, only a few properties were mapped at the time.
This PR aims to build on those changes. All of the properties of the agent.properties file were mapped in the AgentProperties class.
While executing the mapping, I found some properties that were used in the code, but were not documented in the "agent.properties" file. These properties were also documented, and registered accordingly in the properties file.
The way the properties were accessed in the code was changed, and now the values are obtained through the AgentPropertiesFileHandler class. Also, Unit Tests were added and modified according to necessity.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
This was manually tested in a local lab, by altering properties and performing main actions: creating and deleting VMs, creating new Volumes and snapshots, etc.