Skip to content

Commit 737a82c

Browse files
committed
Reorder push command operations
Narrow down the window of opportunity for race conditions Reference issue: #18 The idea was presented in this PR: #19
1 parent c535f50 commit 737a82c

File tree

1 file changed

+26
-19
lines changed

1 file changed

+26
-19
lines changed

cmd/helms3/push.go

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"os"
77
"path/filepath"
8+
"time"
89

910
"github.com/pkg/errors"
1011
"k8s.io/helm/pkg/chartutil"
@@ -16,7 +17,15 @@ import (
1617
"github.com/hypnoglow/helm-s3/pkg/index"
1718
)
1819

20+
const (
21+
pushCommandDefaultTimeout = time.Second * 15
22+
)
23+
1924
func runPush(chartPath string, repoName string) error {
25+
// Just one big timeout for the whole operation.
26+
ctx, cancel := context.WithTimeout(context.Background(), pushCommandDefaultTimeout)
27+
defer cancel()
28+
2029
fpath, err := filepath.Abs(chartPath)
2130
if err != nil {
2231
return errors.WithMessage(err, "get chart abs path")
@@ -36,7 +45,8 @@ func runPush(chartPath string, repoName string) error {
3645

3746
storage := awss3.NewStorage(awsConfig)
3847

39-
// Load chart and calculate required params like hash.
48+
// Load chart, calculate required params like hash,
49+
// and upload the chart right away.
4050

4151
chart, err := chartutil.LoadFile(fname)
4252
if err != nil {
@@ -48,15 +58,26 @@ func runPush(chartPath string, repoName string) error {
4858
return err
4959
}
5060

61+
fchart, err := os.Open(fname)
62+
if err != nil {
63+
return errors.Wrap(err, "open chart file")
64+
}
65+
66+
if _, err := storage.Upload(ctx, repoEntry.URL+"/"+fname, fchart); err != nil {
67+
return errors.WithMessage(err, "upload chart to s3")
68+
}
69+
70+
// Next, update the repository index.
71+
// The gap between index fetching and uploading should be as small as
72+
// possible to make the best effort to avoid race conditions.
73+
// See https://github.com/hypnoglow/helm-s3/issues/18 for more info.
74+
5175
hash, err := provenance.DigestFile(fname)
5276
if err != nil {
5377
return errors.WithMessage(err, "get chart digest")
5478
}
5579

56-
// Fetch current index.
57-
58-
ctx, cancel := context.WithTimeout(context.Background(), defaultTimeout)
59-
defer cancel()
80+
// Fetch current index, update it and upload it back.
6081

6182
b, err := storage.FetchRaw(ctx, repoEntry.URL+"/index.yaml")
6283
if err != nil {
@@ -68,28 +89,14 @@ func runPush(chartPath string, repoName string) error {
6889
return errors.WithMessage(err, "load index from downloaded file")
6990
}
7091

71-
// Update index.
72-
7392
idx.Add(chart.GetMetadata(), fname, repoEntry.URL, hash)
7493
idx.SortEntries()
7594

76-
// Finally, upload both chart file and index.
77-
78-
fchart, err := os.Open(fname)
79-
if err != nil {
80-
return errors.Wrap(err, "open chart file")
81-
}
8295
idxReader, err := idx.Reader()
8396
if err != nil {
8497
return errors.WithMessage(err, "get index reader")
8598
}
8699

87-
ctx, cancel = context.WithTimeout(context.Background(), defaultTimeout*2)
88-
defer cancel()
89-
90-
if _, err := storage.Upload(ctx, repoEntry.URL+"/"+fname, fchart); err != nil {
91-
return errors.WithMessage(err, "upload chart to s3")
92-
}
93100
if _, err := storage.Upload(ctx, repoEntry.URL+"/index.yaml", idxReader); err != nil {
94101
return errors.WithMessage(err, "upload index to s3")
95102
}

0 commit comments

Comments
 (0)