Skip to content

Commit 3a838db

Browse files
dmitshurheschi
authored andcommitted
internal/relui: wait for all files in uploadArtifacts
CL 429275 and CL 429536 have hidden partially-written files from the release test. I think a remaining race was that uploadArtifacts only waits for the main artifact file, and uploadArtifact copies that one first. So it was possible for fakeCDNLoad to periodically copy all the main files but not yet one of the .sha256 files, and the test release workflow would complete too early, before fakeCDNLoad finished its work. This race is very unlikely to happen in production since everything else takes longer. Still, we consider the .sha256 metadata file as non-optional, so modify uploadArtifacts to wait for all files to be made available before considering its job as done. Fixes golang/go#55178. (Third time's the charm, right?) Updates golang/go#53972. Change-Id: Id470da1fb94ba91b292c63a53e34705b30ba8d66 Reviewed-on: https://go-review.googlesource.com/c/build/+/448135 TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Heschi Kreinick <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> Run-TryBot: Dmitri Shuralyov <[email protected]>
1 parent d781efd commit 3a838db

File tree

1 file changed

+12
-6
lines changed

1 file changed

+12
-6
lines changed

internal/relui/workflows.go

+12-6
Original file line numberDiff line numberDiff line change
@@ -914,26 +914,30 @@ func (tasks *BuildReleaseTasks) uploadArtifacts(ctx *wf.TaskContext, artifacts [
914914
return err
915915
}
916916

917-
todo := map[artifact]bool{}
917+
todo := map[string]bool{} // URLs we're waiting on becoming available.
918918
for _, a := range artifacts {
919919
if err := uploadArtifact(scratchFS, servingFS, a); err != nil {
920920
return err
921921
}
922-
todo[a] = true
922+
todo[tasks.DownloadURL+"/"+a.Filename] = true
923+
todo[tasks.DownloadURL+"/"+a.Filename+".sha256"] = true
924+
if a.GPGSignature != "" {
925+
todo[tasks.DownloadURL+"/"+a.Filename+".asc"] = true
926+
}
923927
}
924928

925929
check := func() (int, bool, error) {
926-
for _, a := range artifacts {
930+
for url := range todo {
927931
ctx, cancel := context.WithTimeout(ctx, 5*time.Second)
928932
defer cancel()
929-
resp, err := ctxhttp.Head(ctx, http.DefaultClient, tasks.DownloadURL+"/"+a.Filename)
933+
resp, err := ctxhttp.Head(ctx, http.DefaultClient, url)
930934
if err != nil && err != context.DeadlineExceeded {
931935
return 0, false, err
932936
}
933937
resp.Body.Close()
934938
cancel()
935939
if resp.StatusCode == http.StatusOK {
936-
delete(todo, a)
940+
delete(todo, url)
937941
}
938942
}
939943
return 0, len(todo) == 0, nil
@@ -942,6 +946,8 @@ func (tasks *BuildReleaseTasks) uploadArtifacts(ctx *wf.TaskContext, artifacts [
942946
return err
943947
}
944948

949+
// uploadArtifact copies the artifact a and its metadata files
950+
// from scratchFS to servingFS.
945951
func uploadArtifact(scratchFS, servingFS fs.FS, a artifact) error {
946952
in, err := scratchFS.Open(a.ScratchPath)
947953
if err != nil {
@@ -964,12 +970,12 @@ func uploadArtifact(scratchFS, servingFS fs.FS, a artifact) error {
964970
if err := gcsfs.WriteFile(servingFS, a.Filename+".sha256", []byte(a.SHA256)); err != nil {
965971
return err
966972
}
967-
968973
if a.GPGSignature != "" {
969974
if err := gcsfs.WriteFile(servingFS, a.Filename+".asc", []byte(a.GPGSignature)); err != nil {
970975
return err
971976
}
972977
}
978+
973979
return nil
974980
}
975981

0 commit comments

Comments
 (0)