Fix race condition in `helm list` (#4620)

* Fix race in helm list when partitioning

Problem:
The chunks slice that is passed through the channel is reused for each
partition. This means that encoding the release into a message is racing with
populating the next partition, causing the results to sometimes not fit in the
message, and the release list to be incorrect

Solution:
Allocate a new slice for each partition

Issue #3322
Signed-off-by: Brian Marshall <bmarshall13@users.noreply.github.com>
(cherry picked from commit a0858e29d877b8aa19478133865e45958ff21a9e)

* fix import sorting
Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com>

* ref(release_server_test): use NewReleaseServer()
Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com>

* add unit test for race condition in `helm list`
Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com>

pkg/tiller/release_list.go

@@ -18,11 +18,12 @@ package tiller
 
 import (
 	"fmt"
+	"regexp"
+
 	"github.com/golang/protobuf/proto"
 	"k8s.io/helm/pkg/proto/hapi/release"
 	"k8s.io/helm/pkg/proto/hapi/services"
 	relutil "k8s.io/helm/pkg/releaseutil"
-	"regexp"
 )
 
 // ListReleases lists the releases found by the server.
@@ -140,7 +141,7 @@ func (s *ReleaseServer) partition(rels []*release.Release, cap int) <-chan []*re
 				s.Log("partitioned at %d with %d releases (cap=%d)", fill, len(chunk), cap)
 				chunks <- chunk
 				// reset paritioning state
-				chunk = chunk[:0]
+				chunk = nil
 				fill = 0
 			}
 			chunk = append(chunk, rls)

pkg/tiller/release_list_test.go

@@ -274,3 +274,26 @@ func TestReleasesNamespace(t *testing.T) {
 		t.Errorf("Expected 2 releases, got %d", len(mrs.val.Releases))
 	}
 }
+
+func TestReleasePartition(t *testing.T) {
+	var rl []*release.Release
+	rs := rsFixture()
+	rs.Log = t.Logf
+	num := 7
+	for i := 0; i < num; i++ {
+		rel := releaseStub()
+		rel.Name = fmt.Sprintf("rel-%d", i)
+		rl = append(rl, rel)
+	}
+	visited := map[string]bool{}
+
+	chunks := rs.partition(rl, 0)
+	for chunk := range chunks {
+		for _, rel := range chunk {
+			if visited[rel.Name] {
+				t.Errorf("%s was already visited", rel.Name)
+			}
+			visited[rel.Name] = true
+		}
+	}
+}

pkg/tiller/release_server_test.go

@@ -112,24 +112,16 @@ data:
   name: value
 `
 
-func rsFixture() *ReleaseServer {
-	clientset := fake.NewSimpleClientset()
-	return &ReleaseServer{
-		ReleaseModule: &LocalReleaseModule{
-			clientset: clientset,
-		},
-		env:       MockEnvironment(),
-		clientset: clientset,
-		Log:       func(_ string, _ ...interface{}) {},
-	}
-}
-
 type chartOptions struct {
 	*chart.Chart
 }
 
 type chartOption func(*chartOptions)
 
+func rsFixture() *ReleaseServer {
+	return NewReleaseServer(MockEnvironment(), fake.NewSimpleClientset(), false)
+}
+
 func buildChart(opts ...chartOption) *chart.Chart {
 	c := &chartOptions{
 		Chart: &chart.Chart{