Damans227 commented on code in PR #13977:
URL: https://github.com/apache/cloudstack/pull/13977#discussion_r4219274841
##########
test/integration/smoke/test_routers_network_ops.py:
##########
@@ -287,7 +287,7 @@ def
test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true(self):
)
expected = 1
- ssh_command = "curl -v -m 1 -o index.html -sL www.google.com"
+ ssh_command = "wget -t 1 -T 5 www.google.com"
Review Comment:
is the switch from curl to wget in these router tests part of this fix?
looks unrelated to the template copy
##########
vmware-base/src/main/java/com/cloud/hypervisor/vmware/util/VmwareContext.java:
##########
@@ -442,32 +443,82 @@ public void uploadVmdkFile(String httpMethod, String
urlString, String localFile
conn.setRequestProperty("content-length", Long.toString(new
File(localFileName).length()));
connectWithRetry(conn);
- BufferedOutputStream bos = null;
- BufferedInputStream is = null;
try {
- bos = new BufferedOutputStream(conn.getOutputStream());
- is = new BufferedInputStream(new FileInputStream(localFileName));
- int bufferSize = ChunkSize;
- byte[] buffer = new byte[bufferSize];
+ writeVmdkFileToConnection(conn, httpMethod, urlString,
localFileName, totalBytesUpdated, progressUpdater);
+ checkUploadResponse(conn, httpMethod, urlString, localFileName);
+ } finally {
+ conn.disconnect();
+ }
+ }
+
+ private void writeVmdkFileToConnection(HttpURLConnection conn, String
httpMethod, String urlString, String localFileName, long totalBytesUpdated,
+ ActionDelegate<Long> progressUpdater) throws IOException {
+ long bytesWrittenThisCall = 0;
+ try (BufferedOutputStream bos = new
BufferedOutputStream(conn.getOutputStream());
+ BufferedInputStream is = new BufferedInputStream(new
FileInputStream(localFileName))) {
+ byte[] buffer = new byte[ChunkSize];
while (true) {
- int bytesRead = is.read(buffer, 0, bufferSize);
+ int bytesRead = is.read(buffer, 0, ChunkSize);
if (bytesRead == -1) {
break;
}
bos.write(buffer, 0, bytesRead);
totalBytesUpdated += bytesRead;
+ bytesWrittenThisCall += bytesRead;
bos.flush();
if (progressUpdater != null)
progressUpdater.action(new Long(totalBytesUpdated));
}
bos.flush();
- } finally {
- if (is != null)
- is.close();
- if (bos != null)
- bos.close();
+ } catch (IOException e) {
+ throw new IOException(String.format("Upload of %s to %s %s failed
after writing %d of %d bytes for this file " +
+ "(%d bytes total written so far for this import):
%s",
+ localFileName, httpMethod, urlString,
bytesWrittenThisCall, new File(localFileName).length(), totalBytesUpdated,
e.getMessage()), e);
+ }
+ }
- conn.disconnect();
+ /**
+ * HttpURLConnection does not surface a failed request just because the
client finished writing the
+ * request body without an IOException: with chunked transfer encoding
many HTTP servers, including
+ * ESXi's NFC endpoint, read and discard the whole body before responding
with an error status (e.g. a
+ * VMFS file lock, an out-of-space datastore, or an authentication/session
failure). Silently ignoring
+ * the response code is exactly how a rejected/short write can look like a
"successful" upload to
+ * CloudStack. Surface the actual status code, reason phrase and (if any)
response body so failures like
+ * "file locked" are visible in the CloudStack logs instead of only in
vCenter/ESXi's own logs, if at all.
+ */
+ private void checkUploadResponse(HttpURLConnection conn, String
httpMethod, String urlString, String localFileName) throws IOException {
+ int responseCode;
+ try {
+ responseCode = conn.getResponseCode();
Review Comment:
is waiting for this response the actual fix? before, the connection was
closed without reading it, so esxi may not have finished the copy
--
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]
