Skip to content

Conversation

@paulazomig
Copy link
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

PR Analysis

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

PR Coverage Report

CLASS INSTRUCTION MISSED INSTRUCTION COVERED BRANCH MISSED BRANCH COVERED LINE MISSED LINE COVERED
BridgeVifDriver 1116 125 151 17 222 29
IvsVifDriver 812 0 106 0 162 0
LibvirtComputingResource 8573 2026 1026 144 1899 452
OvsVifDriver 643 135 82 12 106 34
KVMStorageProcessor 5982 498 446 22 1236 71

Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

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

good enhancement/cleanup

@rohityadavcloud
Copy link
Member

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

@github-actions
Copy link

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

@DaanHoogland
Copy link
Contributor

@paulazomig youĺl need to solve a conflict

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 1 Security Hotspot
Code Smell A 98 Code Smells

19.3% 19.3% Coverage
0.0% 0.0% Duplication

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

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

@acs-robot
Copy link

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

@blueorangutan
Copy link

@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

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

@DaanHoogland
Copy link
Contributor

SonarCloud Quality Gate failed. Quality Gate failed

Bug A 0 Bugs Vulnerability A 0 Vulnerabilities Security Hotspot E 1 Security Hotspot Code Smell A 100 Code Smells

20.8% 20.8% Coverage 0.0% 0.0% Duplication

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

@GutoVeronezi
Copy link
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
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

Trillian Build Failed (tid-81)

@DaanHoogland
Copy link
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
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

github-actions bot commented Dec 5, 2022

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

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 1 Security Hotspot
Code Smell A 0 Code Smells

43.7% 43.7% Coverage
0.0% 0.0% Duplication

@GutoVeronezi
Copy link
Contributor

@blueorangutan package

@blueorangutan
Copy link

@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

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

@GutoVeronezi
Copy link
Contributor

GutoVeronezi commented Dec 15, 2022

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

@blueorangutan test

@blueorangutan
Copy link

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

@blueorangutan
Copy link

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:

Test Result Time (s) Test File

@DaanHoogland
Copy link
Contributor

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

@GutoVeronezi
Copy link
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
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
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 ;)

Copy link
Contributor

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

@DaanHoogland DaanHoogland merged commit 0fe2e69 into apache:main Dec 22, 2022
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.

10 participants