Copilot commented on code in PR #5:
URL:
https://github.com/apache/cloudstack-installer/pull/5#discussion_r2645239649
##########
install.sh:
##########
@@ -1858,14 +1858,26 @@ deploy_zone() {
local network="${HOST_IP%.*}.0/24"
# Find IPs for different purposes
local public_ips=($(find_free_ip_range "$network" 11 20)) # 20 IPs for
public
- local pod_ips=($(find_free_ip_range "$network" 41 20)) # 20 IPs for pod
+ if [[ ${#public_ips[@]} -eq 0 ]]; then
+ error_exit "No free public IPs found in $network"
+ fi
+ local last_public_ip="${public_ips[-1]}"
+ local last_public_octet="${last_public_ip##*.}"
+ local pod_start=$((last_public_octet + 1))
+ if (( pod_start >= 255 )); then
Review Comment:
The boundary check should verify if there's enough space for 20 pod IPs, not
just if pod_start is below 255. If pod_start is 235 or higher, requesting 20
IPs would exceed the valid IP range (up to .254). Consider checking if
`pod_start + 19 >= 255` instead to ensure all 20 requested IPs can fit in the
valid range.
```suggestion
if (( pod_start + 19 >= 255 )); then
```
##########
install.sh:
##########
@@ -1858,14 +1858,26 @@ deploy_zone() {
local network="${HOST_IP%.*}.0/24"
# Find IPs for different purposes
local public_ips=($(find_free_ip_range "$network" 11 20)) # 20 IPs for
public
- local pod_ips=($(find_free_ip_range "$network" 41 20)) # 20 IPs for pod
+ if [[ ${#public_ips[@]} -eq 0 ]]; then
+ error_exit "No free public IPs found in $network"
+ fi
+ local last_public_ip="${public_ips[-1]}"
+ local last_public_octet="${last_public_ip##*.}"
+ local pod_start=$((last_public_octet + 1))
+ if (( pod_start >= 255 )); then
+ error_exit "Not enough IP space to allocate pod IPs"
+ fi
+ local pod_ips=($(find_free_ip_range "$network" "$pod_start" 20)) # 20 IPs
for pod
+ if [[ ${#pod_ips[@]} -eq 0 ]]; then
+ error_exit "No free pod IPs found in $network"
+ fi
Review Comment:
The error check only verifies if the array is empty, but doesn't check if
the full 20 IPs were found. The find_free_ip_range function may return fewer
than 20 IPs if there aren't enough available. Consider checking if the array
length equals 20: `if [[ ${#pod_ips[@]} -lt 20 ]]` to ensure sufficient IPs
were allocated.
##########
install.sh:
##########
@@ -1858,14 +1858,26 @@ deploy_zone() {
local network="${HOST_IP%.*}.0/24"
# Find IPs for different purposes
local public_ips=($(find_free_ip_range "$network" 11 20)) # 20 IPs for
public
- local pod_ips=($(find_free_ip_range "$network" 41 20)) # 20 IPs for pod
+ if [[ ${#public_ips[@]} -eq 0 ]]; then
+ error_exit "No free public IPs found in $network"
+ fi
Review Comment:
The error check only verifies if the array is empty, but doesn't check if
the full 20 IPs were found. The find_free_ip_range function may return fewer
than 20 IPs if there aren't enough available. Consider checking if the array
length equals 20: `if [[ ${#public_ips[@]} -lt 20 ]]` to ensure sufficient IPs
were allocated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
