From 9d57a8f598123e9beb845b610d2cc8e8a60127fc Mon Sep 17 00:00:00 2001 From: "Guo, Larry (NSB - CN/Qingdao)" Date: Mon, 10 Aug 2020 15:26:50 +0800 Subject: [PATCH] upload template enhancement Support upload ovf which has no ovf:size specified for each dependency file ShowUploadProgress can exit whenever task stop, such like cancelled on GUI Return status directly for reponse which has no body, such like 416, 400 Signed-off-by: Guo, Larry (NSB - CN/Qingdao) --- govcd/api.go | 56 ++++++++++-------- govcd/api_vcd_test.go | 9 +-- govcd/catalog_test.go | 27 ++++++--- govcd/catalogitem_test.go | 2 +- govcd/sample_govcd_test_config.json | 2 + govcd/sample_govcd_test_config.yaml | 3 + govcd/upload.go | 30 +++++++--- govcd/uploadtask.go | 9 +++ test-resources/template_without_vmdk_size.ova | Bin 0 -> 81920 bytes 9 files changed, 90 insertions(+), 48 deletions(-) create mode 100644 test-resources/template_without_vmdk_size.ova diff --git a/govcd/api.go b/govcd/api.go index 40dbb1aec..84a830e86 100644 --- a/govcd/api.go +++ b/govcd/api.go @@ -245,6 +245,11 @@ func ParseErr(resp *http.Response, errType error) error { return fmt.Errorf("[ParseErr]: error parsing error body for non-200 request: %s (%+v)", err, resp) } + // response body maybe empty for some error, such like 416, 400 + if errType.Error() == "API Error: 0: " { + errType = fmt.Errorf(resp.Status) + } + return errType } @@ -297,31 +302,32 @@ func checkRespWithErrType(resp *http.Response, err, errType error) (*http.Respon return resp, nil // Invalid request, parse the XML error returned and return it. case - http.StatusBadRequest, // 400 - http.StatusUnauthorized, // 401 - http.StatusForbidden, // 403 - http.StatusNotFound, // 404 - http.StatusMethodNotAllowed, // 405 - http.StatusNotAcceptable, // 406 - http.StatusProxyAuthRequired, // 407 - http.StatusRequestTimeout, // 408 - http.StatusConflict, // 409 - http.StatusGone, // 410 - http.StatusLengthRequired, // 411 - http.StatusPreconditionFailed, // 412 - http.StatusRequestEntityTooLarge, // 413 - http.StatusRequestURITooLong, // 414 - http.StatusUnsupportedMediaType, // 415 - http.StatusLocked, // 423 - http.StatusFailedDependency, // 424 - http.StatusUpgradeRequired, // 426 - http.StatusPreconditionRequired, // 428 - http.StatusTooManyRequests, // 429 - http.StatusRequestHeaderFieldsTooLarge, // 431 - http.StatusUnavailableForLegalReasons, // 451 - http.StatusInternalServerError, // 500 - http.StatusServiceUnavailable, // 503 - http.StatusGatewayTimeout: // 504 + http.StatusBadRequest, // 400 + http.StatusUnauthorized, // 401 + http.StatusForbidden, // 403 + http.StatusNotFound, // 404 + http.StatusMethodNotAllowed, // 405 + http.StatusNotAcceptable, // 406 + http.StatusProxyAuthRequired, // 407 + http.StatusRequestTimeout, // 408 + http.StatusConflict, // 409 + http.StatusGone, // 410 + http.StatusLengthRequired, // 411 + http.StatusPreconditionFailed, // 412 + http.StatusRequestEntityTooLarge, // 413 + http.StatusRequestURITooLong, // 414 + http.StatusUnsupportedMediaType, // 415 + http.StatusRequestedRangeNotSatisfiable, // 416 + http.StatusLocked, // 423 + http.StatusFailedDependency, // 424 + http.StatusUpgradeRequired, // 426 + http.StatusPreconditionRequired, // 428 + http.StatusTooManyRequests, // 429 + http.StatusRequestHeaderFieldsTooLarge, // 431 + http.StatusUnavailableForLegalReasons, // 451 + http.StatusInternalServerError, // 500 + http.StatusServiceUnavailable, // 503 + http.StatusGatewayTimeout: // 504 return nil, ParseErr(resp, errType) // Unhandled response. default: diff --git a/govcd/api_vcd_test.go b/govcd/api_vcd_test.go index e8a7e4706..a4f3d291a 100644 --- a/govcd/api_vcd_test.go +++ b/govcd/api_vcd_test.go @@ -171,10 +171,11 @@ type TestConfig struct { VerboseCleanup bool `yaml:"verboseCleanup,omitempty"` } `yaml:"logging"` OVA struct { - OvaPath string `yaml:"ovaPath,omitempty"` - OvaChunkedPath string `yaml:"ovaChunkedPath,omitempty"` - OvaMultiVmPath string `yaml:"ovaMultiVmPath,omitempty"` - OvfPath string `yaml:"ovfPath,omitempty"` + OvaPath string `yaml:"ovaPath,omitempty"` + OvaChunkedPath string `yaml:"ovaChunkedPath,omitempty"` + OvaMultiVmPath string `yaml:"ovaMultiVmPath,omitempty"` + OvaWithoutSizePath string `yaml:"ovaWithoutSizePath,omitempty"` + OvfPath string `yaml:"ovfPath,omitempty"` } `yaml:"ova"` Media struct { MediaPath string `yaml:"mediaPath,omitempty"` diff --git a/govcd/catalog_test.go b/govcd/catalog_test.go index f3af9a786..b09498034 100644 --- a/govcd/catalog_test.go +++ b/govcd/catalog_test.go @@ -155,7 +155,7 @@ func doesCatalogExist(check *C, org *AdminOrg) { func (vcd *TestVCD) Test_UploadOvf(check *C) { fmt.Printf("Running: %s\n", check.TestName()) - skipWhenOvaPathMissing(vcd, check) + skipWhenOvaPathMissing(vcd.config.OVA.OvaPath, check) checkUploadOvf(vcd, check, vcd.config.OVA.OvaPath, vcd.config.VCD.Catalog.Name, TestUploadOvf) } @@ -164,7 +164,7 @@ func (vcd *TestVCD) Test_UploadOvf(check *C) { func (vcd *TestVCD) Test_UploadOvf_chunked(check *C) { fmt.Printf("Running: %s\n", check.TestName()) - skipWhenOvaPathMissing(vcd, check) + skipWhenOvaPathMissing(vcd.config.OVA.OvaChunkedPath, check) checkUploadOvf(vcd, check, vcd.config.OVA.OvaChunkedPath, vcd.config.VCD.Catalog.Name, TestUploadOvf+"2") } @@ -173,7 +173,7 @@ func (vcd *TestVCD) Test_UploadOvf_chunked(check *C) { func (vcd *TestVCD) Test_UploadOvf_progress_works(check *C) { fmt.Printf("Running: %s\n", check.TestName()) - skipWhenOvaPathMissing(vcd, check) + skipWhenOvaPathMissing(vcd.config.OVA.OvaPath, check) itemName := TestUploadOvf + "3" catalog, org := findCatalog(vcd, check, vcd.config.VCD.Catalog.Name) @@ -202,7 +202,7 @@ func (vcd *TestVCD) Test_UploadOvf_progress_works(check *C) { func (vcd *TestVCD) Test_UploadOvf_ShowUploadProgress_works(check *C) { fmt.Printf("Running: %s\n", check.TestName()) - skipWhenOvaPathMissing(vcd, check) + skipWhenOvaPathMissing(vcd.config.OVA.OvaPath, check) itemName := TestUploadOvf + "4" catalog, org := findCatalog(vcd, check, vcd.config.VCD.Catalog.Name) @@ -240,7 +240,7 @@ func (vcd *TestVCD) Test_UploadOvf_ShowUploadProgress_works(check *C) { func (vcd *TestVCD) Test_UploadOvf_error_withSameItem(check *C) { fmt.Printf("Running: %s\n", check.TestName()) - skipWhenOvaPathMissing(vcd, check) + skipWhenOvaPathMissing(vcd.config.OVA.OvaPath, check) itemName := TestUploadOvf + "5" @@ -264,7 +264,7 @@ func (vcd *TestVCD) Test_UploadOvf_error_withSameItem(check *C) { func (vcd *TestVCD) Test_UploadOvf_cleaned_extracted_files(check *C) { fmt.Printf("Running: %s\n", check.TestName()) - skipWhenOvaPathMissing(vcd, check) + skipWhenOvaPathMissing(vcd.config.OVA.OvaPath, check) itemName := TestUploadOvf + "6" @@ -289,10 +289,19 @@ func (vcd *TestVCD) Test_UploadOvf_cleaned_extracted_files(check *C) { func (vcd *TestVCD) Test_UploadOvfFile(check *C) { fmt.Printf("Running: %s\n", check.TestName()) - skipWhenOvaPathMissing(vcd, check) + skipWhenOvaPathMissing(vcd.config.OVA.OvfPath, check) checkUploadOvf(vcd, check, vcd.config.OVA.OvfPath, vcd.config.VCD.Catalog.Name, TestUploadOvf+"7") } +// Tests System function UploadOvf by creating catalog and +// checking ova file without vmdk size specified can be uploaded. +func (vcd *TestVCD) Test_UploadOvf_withoutVMDKSize(check *C) { + fmt.Printf("Running: %s\n", check.TestName()) + + skipWhenOvaPathMissing(vcd.config.OVA.OvaWithoutSizePath, check) + checkUploadOvf(vcd, check, vcd.config.OVA.OvaWithoutSizePath, vcd.config.VCD.Catalog.Name, TestUploadOvf+"8") +} + func countFolders() int { files, err := ioutil.ReadDir(os.TempDir()) if err != nil { @@ -349,8 +358,8 @@ func getOrg(vcd *TestVCD, check *C) *AdminOrg { return org } -func skipWhenOvaPathMissing(vcd *TestVCD, check *C) { - if vcd.config.OVA.OvaPath == "" || vcd.config.OVA.OvaChunkedPath == "" { +func skipWhenOvaPathMissing(ovaPath string, check *C) { + if ovaPath == "" { check.Skip("Skipping test because no ova path given") } } diff --git a/govcd/catalogitem_test.go b/govcd/catalogitem_test.go index 67b6c8b3f..1bfa8cc82 100644 --- a/govcd/catalogitem_test.go +++ b/govcd/catalogitem_test.go @@ -45,7 +45,7 @@ func (vcd *TestVCD) Test_GetVAppTemplate(check *C) { // Tests System function Delete by creating catalog item and // deleting it after. func (vcd *TestVCD) Test_Delete(check *C) { - skipWhenOvaPathMissing(vcd, check) + skipWhenOvaPathMissing(vcd.config.OVA.OvaPath, check) AddToCleanupList(TestDeleteCatalogItem, "catalogItem", vcd.org.Org.Name+"|"+vcd.config.VCD.Catalog.Name, "Test_Delete") // Fetching organization diff --git a/govcd/sample_govcd_test_config.json b/govcd/sample_govcd_test_config.json index 32e57b81f..7943e0515 100644 --- a/govcd/sample_govcd_test_config.json +++ b/govcd/sample_govcd_test_config.json @@ -75,6 +75,8 @@ "ova": { "ovaPath": "../test-resources/test_vapp_template.ova", "ovaChunkedPath": "../test-resources/template_with_custom_chunk_size.ova", + "ovaMultiVmPath": "../test-resources/vapp_with_3_vms.ova", + "ovaWithoutSizePath": "../test-resources/template_without_vmdk_size.ova", "ovfPath": "../test-resources/test_vapp_template_ovf/descriptor.ovf" }, "media": { diff --git a/govcd/sample_govcd_test_config.yaml b/govcd/sample_govcd_test_config.yaml index 51673357f..9d3da1c30 100644 --- a/govcd/sample_govcd_test_config.yaml +++ b/govcd/sample_govcd_test_config.yaml @@ -162,6 +162,9 @@ ova: # The ova with multi VMs for tests. ovaMultiVmPath: ../test-resources/vapp_with_3_vms.ova # + # The ova with no VMDK size in ovf for tests. + ovaWithoutSizePath: ../test-resources/template_without_vmdk_size.ova + # # The ovf for uploading catalog item for tests. ovfPath: ../test-resources/test_vapp_template_ovf/descriptor.ovf media: diff --git a/govcd/upload.go b/govcd/upload.go index ca8e08a26..9938764b8 100644 --- a/govcd/upload.go +++ b/govcd/upload.go @@ -67,14 +67,6 @@ func uploadFile(client *Client, filePath string, uDetails uploadDetails) (int64, var count int var pieceSize int64 - // do not allow smaller than 1kb - if uDetails.uploadPieceSize > 1024 && uDetails.uploadPieceSize < uDetails.fileSizeToUpload { - pieceSize = uDetails.uploadPieceSize - } else { - pieceSize = defaultPieceSize - } - - util.Logger.Printf("[TRACE] Uploading will use piece size: %#v \n", pieceSize) // #nosec G304 - linter does not like 'filePath' to be a variable. However this is necessary for file uploads. file, err := os.Open(filePath) if err != nil { @@ -92,6 +84,26 @@ func uploadFile(client *Client, filePath string, uDetails uploadDetails) (int64, defer file.Close() + fileSize := fileInfo.Size() + // file size in OVF maybe not exist, use real file size instead + if uDetails.fileSizeToUpload == -1 { + uDetails.fileSizeToUpload = fileSize + uDetails.allFilesSize += fileSize + } + // TODO: file size in OVF maybe wrong? how to handle that? + if uDetails.fileSizeToUpload != fileSize { + fmt.Printf("WARNING：file size %d in OVF is not align with real file size %d, upload task may hung.", + uDetails.fileSizeToUpload, fileSize) + } + + // do not allow smaller than 1kb + if uDetails.uploadPieceSize > 1024 && uDetails.uploadPieceSize < uDetails.fileSizeToUpload { + pieceSize = uDetails.uploadPieceSize + } else { + pieceSize = defaultPieceSize + } + + util.Logger.Printf("[TRACE] Uploading will use piece size: %#v \n", pieceSize) part = make([]byte, pieceSize) for { @@ -122,7 +134,7 @@ func uploadFile(client *Client, filePath string, uDetails uploadDetails) (int64, return 0, err } - return fileInfo.Size(), nil + return fileSize, nil } // Create Request with right headers and range settings. Support multi part file upload. diff --git a/govcd/uploadtask.go b/govcd/uploadtask.go index ba62af19d..66bc87568 100644 --- a/govcd/uploadtask.go +++ b/govcd/uploadtask.go @@ -39,6 +39,15 @@ func (uploadTask *UploadTask) ShowUploadProgress() error { fmt.Printf("\rUpload progress %.2f%%", uploadTask.uploadProgress.LockedGet()) if uploadTask.uploadProgress.LockedGet() == 100.00 { + fmt.Println() + break + } + // Upload may be cancelled by user on GUI manually, detect task status + if err := uploadTask.Refresh(); err != nil { + return err + } + if uploadTask.Task.Task.Status != "queued" && uploadTask.Task.Task.Status != "preRunning" && uploadTask.Task.Task.Status != "running" { + fmt.Println() break } time.Sleep(1 * time.Second) diff --git a/test-resources/template_without_vmdk_size.ova b/test-resources/template_without_vmdk_size.ova new file mode 100644 index 0000000000000000000000000000000000000000..e88d75886bf3713aa18949b4c6eee114b43e6c8e GIT binary patch literal 81920 zcmeI2TXWks7RR0TO*8!t808_|ozaD|Y}uYztz$WjwsD+|a@l=ph9V)0V~OOFlpOV` z`^k3?ZWKt$uH`u0Z0D~@Y#G2g0DkA-;Og_#OM*Cyl4i7=?0k_=n?C#d-5vVn^!D2J zZ%XQP_j~(0&Th}ybDWOTqkLz#y}P@^+FztKC8dira}&mPW^R(KN_qC5zn5>9{XhEt z^oLLL8C&us4WjV)iPLO9VLbFAKL{_6pDeOT_qSa}g4$nWn50Y%*&PJ;=EGV0R6&0M29 zaqqAD4Ns#*;_;K&Eb`op*lnnQx^`1nbwi1gY0alJC=|Zll^i=XW?KI^e>swX8$n3! z8hV_h(%?vGd8u2Fi>u@tjlu3_vzDL62kp-8N2f_%(fpM9M(}2{nG)kDbs9_0Z=`X9 z8k_}Q!#O;l|GP~dUfMQy<3Ji*dr*sgLg@2>n-NB^9*-ZXebGwb~tV=^MV;S-)vxZtTs(uvQ58D}D1OcPEKVmWVg z+T*S>aeIy3j?-y$C+?(i*g4#5w7YwLd+c=&dk4D>KS-|{ZSoEOsxHL?8u=5_)fjj8 z_d1<+uhH|JZlmjW$Bpr%+i7_FhmPYPc6n+(b5qyq{O_Qx1Pz@vQiP35` zcNk8hzV0p)jbH(#By%~cpGsY_$hw0jeZ0yz z`z!1G($OVFO+)G-^_RYF(MMUc;6gpz8juo}z01QzY5W8s4Zw*@Hf^Ob_ku~F0$kb> zrfE*I`Rs9;CET4~#91&8e&xQ+UBIy*OYpCFb{!>lvUU{-_U)9jS&(Kdny^`PN!86* zs3g*j{IxpSm(ZQ_<9Zl{TzX~hgaf^<5w?Ci4N^w`TqZk2>f;&puQ82gK2O*p_JkCL@H;on~=H59;vj~?>ft- z78t|6+Tb|4ylvr!^PLUKrTY*{ouv@z&MHGof7aQ_b9IFQB1euHT7?`tkch}on4wRj*-VXgIkd#onejCHxOC(AqX`c>LduXV zBku`~ueIESE!B#d&0TLAg#3o+O9Q80Rj#6p7*)y%Wi2DxTV;#bYP$4G(R|r}_ab_U z4Hy-=DO{PUf_hQt+j36Z3ZW@VD2gA~^}H&zoyeGaT#ut`o?L{goXi>UnM?C^UO0-2 zWhb;C7fLBY$9Y+~tPmkj}mnC17a=md zG@BhYr-1?RS4#kn()`G`z*KXrvYZJY^XL zV+ua17M99A70r#rlpKf#ji_q9OvjzNatZzWZV>zY)fhCVfb zmT)|gLsoi_)NoU&+tmsG=`ZJ>8V-44RgcWhH7HA#;VA#7!Cc@a_Ton;zMt?ky$D~E z*}_cpc4K;81G%ayu%Pu6`M zG?wHk#!;LEb2nM7gR~Z>&`V3GdS5I`br06MtG%{e{mh|VOv*Towj|^kbA30aH4IXBtXk|l9wNZr+voqJevmIiW-ow^kKq#qc&7+c?N&i#^9 z(VBYF*b@5o(ZL=pBK*ot{9loX=YgU#k|!LyL_>O-fFY2TUpRnL2ivr zaR{PitkjBxj<3cg^q-YtrUPnd_HH$%3B`I|`TN49c_&--yUt;Ex7{_ZDrZ;fEESNa zCw*~vqHEunZ)>@8fEc^p6$SR@wb&TFohH%Za#{!yYQy~pM~UI!>@$KyQF`kbv3m#K zDLBR6tv>Nb4e*nPnEvf3@tkfs;uRYt!P5Ahoo&0DjEZ(Buw}Q86_&S|5oHbJgS!py zf{C2&gW4#+?@%L?)+yPqbgt>Ne=)&+y#`Z7Dzt2ruNL#MSl>*T9@y;k^*b}!*aa(E zZ01hBb0z8TQ`$pR%udOU2du->7rpb> z6pHDTqoDaSAZXR2^V0`btm5StF&rH+(~0|w_YZ6|b?O_7xCoat<-cg*Pyn9H;9(R7i~2|3*xJYc(R{mQZrsylW$BtiH$%rDb8cKpUDv8JPN!~o z$wz@-x-ZX@D-xQjl;=zE9!2@&RS?WqPzoOz@$}bYS zqgS*_@}8XdA;fruKmbhJy*xKKsL}phsP;lUxX~`S?fTGl!&1EX8 zPNR9epg>l@8D z&v+(o07aHs2g&qH9??^>WYzRmGkT67-jq=RddFGJopRTgIs4sGPD+=>k&@c&sJvMe zUWyw&>g{kK>)RhYKKI5wueay$-N^ypJvcn*wKY4g#Prtf#fSIm8NQHoocaSq_pjai z&^{0V0T2KI5C8!X009sH0T2KI5C8!X009sH0T2KI5C8!X009sH0T2KI5C8!X009sH z0T2KI5C8!X009sH0T2KI5C8!X009sH0T2KI5C8!X009sH0T2KI5C8!X009sH0T2KI z5C8!X009sH0T2KI5C8!X009sH0T2KI5C8!X009sH0T2KI5C8!X009sH0T2KI5C8!X z009sH0T2KI5C8!X009sH0T2KI5C8!X009sH0T2KI5C8!X009sH0T2KI5C8!X009sH z0T2KI5C8!X009sH0T2KI5C8!X009sH0T2KI5C8!X009sH0T2KI5C8!X009sH0T2KI z5C8!X009sH0T2KI5C8!X009sH0T2KI5C8!X009sH0T2KI5C8!X009sH0T2KI5C8!X z009sH0T2KI5C8!X009sH0T2KI5C8!X009sH0T2KI5C8!X009sH0T2KI5C8!X009sH z0T2KI5C8!X0D-TKz$2?c-&o)A6$C)w&riS}|KI-kyNe!{2>d)ae=qF*Xor6OsJ!7f uUB>u^`MqOJ5dYR?Oxc|p`>vK3)S37N0w4eaAOHd&00JNY0wC~J5%@2*Ep+Pu literal 0 HcmV?d00001