Skip to content

Improving code related to the Agent properties - #6348

Merged
DaanHoogland merged 31 commits into
apache:mainfrom
scclouds:map-agent-properties
Dec 22, 2022
Merged

Improving code related to the Agent properties#6348
DaanHoogland merged 31 commits into
apache:mainfrom
scclouds:map-agent-properties

Conversation

@paulazomig

Copy link
Copy Markdown
Contributor

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

  • 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

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

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.

@acs-robot

Copy link
Copy Markdown

PR Analysis

https://sonarcloud.io/summary/new_code?id=apachecloudstack&pullRequest=6348

PR Coverage Report

CLASSINSTRUCTION MISSEDINSTRUCTION COVEREDBRANCH MISSEDBRANCH COVEREDLINE MISSEDLINE COVERED
BridgeVifDriver11161251511722229
IvsVifDriver812010601620
LibvirtComputingResource8573202610261441899452
OvsVifDriver643135821210634
KVMStorageProcessor598249844622123671

@DaanHooglandDaanHoogland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

good enhancement/cleanup

Comment threadagent/src/main/java/com/cloud/agent/AgentShell.java Outdated
Comment threadagent/src/main/java/com/cloud/agent/AgentShell.java
@yadvr

yadvr commented May 4, 2022

Copy link
Copy Markdown
Member

@paulazomig can you merge the latest main to your PR branch?

@github-actions

Copy link
Copy Markdown

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

@DaanHoogland

Copy link
Copy Markdown
Contributor

@paulazomig youĺl need to solve a conflict

@sonarqubecloud

Copy link
Copy Markdown

@nvazquez
nvazquez self-requested a review May 20, 2022 16:07
@nvazqueznvazquez added this to the 4.18.0.0 milestone May 20, 2022
@github-actions

Copy link
Copy Markdown

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

@acs-robot

Copy link
Copy Markdown

Found UI changes, kicking a new UI QA build
@blueorangutan ui

@blueorangutan

Copy link
Copy Markdown

@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

@blueorangutan

Copy link
Copy Markdown

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6348 (SL-JID-1838)

@DaanHoogland

Copy link
Copy Markdown
Contributor

SonarCloud Quality Gate failed. Quality Gate failed

BugA0 BugsVulnerabilityA0 VulnerabilitiesSecurity HotspotE1 Security HotspotCode SmellA100 Code Smells

20.8%20.8% Coverage0.0%0.0% Duplication

please have a look at these @paulazomig . There are certainly some that can be improved.

@GutoVeronezi

Copy link
Copy Markdown
Contributor

@GutoVeronezi the ssvm still won't come up with this message: image

@DaanHoogland, that message is printed when the secondary storage is not mounted in the SSVM; then, the script ssvm-check.sh tries to ping the "NFS server" to validate the connectivity. For some reason it infers that the NFS server is the seventeenth element of file /var/cache/cloud/cmdline; however, the index of the elements will vary according to some configurations. I created PR #6900 to handle this situation, could you take a loot at it?

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.

@DaanHoogland

Copy link
Copy Markdown
Contributor

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?

@blueorangutan

Copy link
Copy Markdown

Trillian Build Failed (tid-81)

@DaanHoogland

Copy link
Copy Markdown
Contributor
Tue 15 Nov 2022 08:15:53 AM UTC Starting guest services for kvm
Tue 15 Nov 2022 08:15:54 AM UTC acpiphp and pci_hotplug module already compiled in
Tue 15 Nov 2022 08:15:55 AM UTC Received a new non-empty cmdline file from qemu-guest-agent
Tue 15 Nov 2022 08:15:55 AM UTC Booting from cloudstack, remove old configuration files in /etc/cloudstack/
Tue 15 Nov 2022 08:15:55 AM UTC Applying iptables rules
Tue 15 Nov 2022 08:15:55 AM UTC Setting up interface: eth0
Tue 15 Nov 2022 08:15:55 AM UTC Setting up interface: eth1
Tue 15 Nov 2022 08:15:55 AM UTC Setting up interface: eth2
Tue 15 Nov 2022 08:15:55 AM UTC Received mgmt cidr : 10.1.32.0/20
Tue 15 Nov 2022 08:15:55 AM UTC Configuring sshd
Tue 15 Nov 2022 08:15:56 AM UTC Executing cloud-early-config
Tue 15 Nov 2022 08:15:56 AM UTC Scripts checksum detected: oldmd5=5158db51564e4825302ad52d2b4c2a69 newmd5=5158db51564e4825302ad52d2b4c2a69
Tue 15 Nov 2022 08:15:58 AM UTC Could not find patch file, retrying
Tue 15 Nov 2022 08:16:00 AM UTC Could not find patch file, retrying
Tue 15 Nov 2022 08:16:02 AM UTC Could not find patch file, retrying
Tue 15 Nov 2022 08:16:05 AM UTC Could not find patch file, retrying
Tue 15 Nov 2022 08:16:05 AM UTC Scripts checksum detected: oldmd5=5158db51564e4825302ad52d2b4c2a69 newmd5=daf7cd26e817274a0b57dde2d447be02
Tue 15 Nov 2022 08:16:05 AM UTC Patched scripts using /var/cache/cloud/cloud-scripts.tgz
Tue 15 Nov 2022 08:16:05 AM UTC Bootstrapping systemvm appliance
Tue 15 Nov 2022 08:16:08 AM UTC Configuring systemvm type=secstorage
Tue 15 Nov 2022 08:16:09 AM UTC Setting up secondary storage system vm
Tue 15 Nov 2022 08:16:09 AM UTC Incompleted parameters STORAGE_IP:, STORAGE_NETMASK:, STORAGE_CIDR:. Cannot setup storage network
Tue 15 Nov 2022 08:16:09 AM UTC Not setting up route of RFC1918 space to 10.1.32.1 because 10.1.55.41 is RFC1918.
Tue 15 Nov 2022 08:16:09 AM UTC Setting up entry in hosts
Tue 15 Nov 2022 08:16:09 AM UTC Applying iptables rules
Tue 15 Nov 2022 08:16:09 AM UTC Configuring apache2
Tue 15 Nov 2022 08:16:09 AM UTC Setting up apache web server
Tue 15 Nov 2022 08:16:09 AM UTC Setting up apache2 for post upload of volume/template
Tue 15 Nov 2022 08:16:10 AM UTC cloud: disable rp_filter
Tue 15 Nov 2022 08:16:10 AM UTC disable rpfilter
Tue 15 Nov 2022 08:16:10 AM UTC cloud: enable_fwding = 0
Tue 15 Nov 2022 08:16:10 AM UTC enable_fwding = 0
Tue 15 Nov 2022 08:16:10 AM UTC Processors = 1 Enable service = 0
Tue 15 Nov 2022 08:16:10 AM UTC Setting up NTP
Tue 15 Nov 2022 08:16:11 AM UTC Finished setting up systemvm
Tue 15 Nov 2022 08:16:11 AM UTC Finished setting up systemvm
2022-11-15 08:16:17,460 INFO [cloud.agent.AgentShell] (main:null) Agent started
2022-11-15 08:16:17,470 INFO [cloud.agent.AgentShell] (main:null) Implementation Version is 4.18.0.0-SNAPSHOT
2022-11-15 08:16:17,474 INFO [cloud.agent.AgentShell] (main:null) agent.properties found at /usr/local/cloud/systemvm/conf/agent.properties
2022-11-15 08:16:17,501 INFO [cloud.agent.AgentShell] (main:null) Defaulting to using properties file for storage
2022-11-15 08:16:17,503 INFO [cloud.agent.AgentShell] (main:null) Defaulting to the constant time backoff algorithm
2022-11-15 08:16:17,546 INFO [cloud.utils.LogUtils] (main:null) log4j configuration found at /usr/local/cloud/systemvm/conf/log4j-cloud.xml
2022-11-15 08:16:17,546 INFO [cloud.agent.AgentShell] (main:null) Using default Java settings for IPv6 preference for agent connection
2022-11-15 08:16:17,764 INFO [cloud.agent.Agent] (main:null) id is 2022-11-15 08:16:17,842 INFO [resource.consoleproxy.ConsoleProxyResource] (main:null) Receive proxyVmId in ConsoleProxyResource configuration as 0
2022-11-15 08:16:17,870 INFO [cloud.agent.Agent] (main:null) Agent [id = new : type = ConsoleProxyResource : zone = 1 : pod = 1 : workers = 10 : host = 10.1.32.133 : port = 8250
2022-11-15 08:16:17,906 INFO [utils.nio.NioClient] (main:null) Connecting to 10.1.32.133:8250
2022-11-15 08:16:17,914 INFO [utils.nio.Link] (main:null) Conf file found: /usr/local/cloud/systemvm/conf/agent.properties
2022-11-15 08:16:18,955 INFO [utils.nio.NioClient] (main:null) SSL: Handshake done
2022-11-15 08:16:18,957 INFO [utils.nio.NioClient] (main:null) Connected to 10.1.32.133:8250
2022-11-15 08:16:19,054 INFO [cloud.serializer.GsonHelper] (Agent-Handler-1:null) Default Builder inited.
2022-11-15 08:16:19,138 INFO [cloud.agent.Agent] (Agent-Handler-2:null) Process agent startup answer, agent id = 0
2022-11-15 08:16:19,142 INFO [cloud.agent.Agent] (Agent-Handler-2:null) Set agent id 0
2022-11-15 08:16:19,181 INFO [cloud.agent.Agent] (Agent-Handler-2:null) Startup Response Received: agent id = 0
2022-11-15 08:16:19,186 INFO [cloud.agent.Agent] (agentRequest-Handler-2:null) Processing agent ready command, agent id = 4
2022-11-15 08:16:19,194 INFO [cloud.agent.Agent] (agentRequest-Handler-2:null) Set agent id 4
2022-11-15 08:16:19,195 INFO [cloud.agent.Agent] (agentRequest-Handler-2:null) Ready command is processed for agent id = 4
2022-11-15 08:16:19,195 INFO [resource.consoleproxy.ConsoleProxyResource] (agentRequest-Handler-2:null) Receive ReadyCommand, response with ReadyAnswer
2022-11-15 08:16:19,249 INFO [cloud.agent.Agent] (agentRequest-Handler-3:null) Processing agent ready command, agent id = 4
2022-11-15 08:16:19,250 INFO [cloud.agent.Agent] (agentRequest-Handler-3:null) Set agent id 4
2022-11-15 08:16:19,257 INFO [cloud.agent.Agent] (agentRequest-Handler-3:null) Processed new management server list: 10.1.32.133@static
2022-11-15 08:16:19,258 INFO [cloud.agent.Agent] (agentRequest-Handler-3:null) Ready command is processed for agent id = 4
2022-11-15 08:16:19,258 INFO [resource.consoleproxy.ConsoleProxyResource] (agentRequest-Handler-3:null) Receive ReadyCommand, response with ReadyAnswer

I think I need to find time to enable debug logging and retry, @GutoVeronezi . I see nothing of interest there.

@GutoVeronezi

Copy link
Copy Markdown
Contributor

I am building an environment for this again. I take it it isn´t happening in your environment?

@DaanHoogland

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 Tue 15 Nov 2022 08:16:09 AM UTC Incompleted parameters STORAGE_IP:, STORAGE_NETMASK:, STORAGE_CIDR:. Cannot setup storage network can lead to somewhere.

@github-actions

Copy link
Copy Markdown

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

@sonarqubecloud

Copy link
Copy Markdown

@GutoVeronezi

Copy link
Copy Markdown
Contributor

@blueorangutan package

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 4966

@GutoVeronezi

GutoVeronezi commented Dec 15, 2022

Copy link
Copy Markdown
Contributor

@DaanHoogland I think I found out what was happening. When patching the system VMs, the file systemvm/agent/conf/agent.properties is copied to them. This file contains two hardcoded properties:

instance=ConsoleProxy
resource=com.cloud.agent.resource.consoleproxy.ConsoleProxyResource

As AgentShell was changed to use the configurations from agent.properties instead of the parameters passed in the command line, the SSVM was being started with the hardcoded property resource (com.cloud.agent.resource.consoleproxy.ConsoleProxyResource) and trying to start as a CPVM.

I made a change to remove those hardcoded properties when the system VM is a SSVM and to copy the content of file /var/cache/cloud/cmdline to agent.properties during the bootstrap, making AgentShell use the proper properties.

Following there is the content of the agent.properties of both system VMs after the change:

For SSVM
image

For CPVM
image


@DaanHoogland, can we run the tests again?

@DaanHoogland

Copy link
Copy Markdown
Contributor

@blueorangutan test

@blueorangutan

Copy link
Copy Markdown

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

@blueorangutan

Copy link
Copy Markdown

Trillian test result (tid-5522)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 40069 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6348-t5522-kvm-centos7.zip
Smoke tests completed. 105 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

TestResultTime (s)Test File

@DaanHoogland

Copy link
Copy Markdown
Contributor

@GutoVeronezi@JoaoJandre@nvazquez are we alright with this now?

@GutoVeronezi

Copy link
Copy Markdown
Contributor

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

@nvazquez

Copy link
Copy Markdown
Contributor

@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

@DaanHoogland

Copy link
Copy Markdown
Contributor

@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 ;)

@nvazqueznvazquez left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code LGTM

@DaanHoogland
DaanHoogland merged commit 0fe2e69 into apache:mainDec 22, 2022
Sign up for freeto 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.

10 participants

@paulazomig@acs-robot@yadvr@DaanHoogland@blueorangutan@JoaoJandre@GutoVeronezi@nvazquez@RodrigoDLopez@BryanMLima