fix(limit): limit release history number (#159) (#160)

* fix(limit): limit release history number

* fix

* change variable name

* add a test case

* add two test cases

* fix lint

* tweak comments

* tweak code

* tweak comments

Co-authored-by: Lisheng Zheng <[email protected]>

Co-authored-by: Lisheng Zheng <[email protected]>

cmd/controller/app/controllers.go

@@ -90,6 +90,7 @@ func startGCController(ctx ControllerContext) error {
 		ctx.InformerStore,
 		ctx.AvailableKinds,
 		ctx.IgnoredKinds,
+		ctx.HistoryLimit,
 	)
 	if err != nil {
 		return err

cmd/controller/app/options/options.go

@@ -26,6 +26,10 @@ type ReleaseServer struct {
 	ResyncPeriod time.Duration
 	// ReleaseResyncPeriod is the resync period to invoke informer event handler.
 	ReleaseResyncPeriod time.Duration
+
+	// The number of releaseHistory to retain to allow rollback.
+	// Defaults to 50.
+	HistoryLimit int32
 }
 
 // NewReleaseServer creates a new CMServer with a default config.
@@ -47,4 +51,5 @@ func (s *ReleaseServer) AddFlags(fs *pflag.FlagSet, allControllers []string) {
 	fs.Int32Var(&s.ConcurrentStatusSyncs, "concurrent-status-syncs", s.ConcurrentStatusSyncs, "The number of status controller worker that are allowed to sync concurrently")
 	fs.DurationVar(&s.ResyncPeriod, "resync-period", s.ResyncPeriod, "ResyncPeriod describes the period of informer resync")
 	fs.DurationVar(&s.ReleaseResyncPeriod, "handler-resync-period", s.ReleaseResyncPeriod, "ReleaseResyncPeriod is the resync period to invoke informer event handler")
+	fs.Int32Var(&s.HistoryLimit, "history-limit", 50, "The number of releaseHistory to retain to allow rollback")
 }

cmd/controller/app/release.go

@@ -50,11 +50,15 @@ type ControllerContext struct {
 	Stop <-chan struct{}
 	// ReleaseResyncPeriod is the resync period to invoke informer event handler for release
 	ReleaseResyncPeriod time.Duration
+	// The number of releaseHistory to retain to allow rollback.
+	// Defaults to 50.
+	HistoryLimit int32
 }
 
 // Run runs the ReleaseServer. This should never exit.
 func Run(s *options.ReleaseServer) error {
 	glog.Infof("Initialize release server")
+	glog.Infof("Retain release history number %v", s.HistoryLimit)
 	glog.Infof("Rudder Build Information, %v", version.Get().Pretty())
 	kubeConfig, err := clientcmd.BuildConfigFromFlags("", s.Kubeconfig)
 	if err != nil {
@@ -92,6 +96,7 @@ func Run(s *options.ReleaseServer) error {
 		IgnoredKinds:        IgnoredKinds(),
 		Stop:                stop,
 		ReleaseResyncPeriod: s.ReleaseResyncPeriod,
+		HistoryLimit:        s.HistoryLimit,
 	}
 	initializers, err := NewControllerInitializers(s.Controllers)
 	if err != nil {

pkg/controller/gc/controller.go

@@ -1,6 +1,9 @@
 package gc
 
 import (
+	"fmt"
+	"strconv"
+	"strings"
 	"sync"
 	"time"
 
@@ -175,19 +178,22 @@ type GarbageCollector struct {
 	ignored       []schema.GroupVersionKind // indicates which resources should be ignored
 	workers       int32
 	working       int32
+	historyLimit  int32
 }
 
 // NewGarbageCollector creates a garbage collector.
 func NewGarbageCollector(clients kube.ClientPool, codec kube.Codec,
 	store store.IntegrationStore, targets, ignored []schema.GroupVersionKind,
+	historyLimit int32,
 ) (*GarbageCollector, error) {
 	gc := &GarbageCollector{
-		clients:   clients,
-		codec:     codec,
-		store:     store,
-		resources: newReleaseResources(ignored),
-		queue:     workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()),
-		ignored:   ignored,
+		clients:      clients,
+		codec:        codec,
+		store:        store,
+		resources:    newReleaseResources(ignored),
+		queue:        workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()),
+		ignored:      ignored,
+		historyLimit: historyLimit,
 	}
 	releaseInformer, err := store.InformerFor(gvkRelease)
 	if err != nil {
@@ -385,7 +391,12 @@ func (gc *GarbageCollector) collect(release *releaseapi.Release) error {
 		switch {
 		case res.gvk == gvkReleaseHistory:
 			// Check history
-			if releaseAlived {
+			ifRetain, err := gc.ifRetainHistory(release, res.name)
+			if err != nil {
+				glog.Errorf("get retain info for resource %s/%s failed %v", res.namespace, res.name, err)
+				return err
+			}
+			if releaseAlived && ifRetain {
 				continue
 			}
 			fallthrough
@@ -416,3 +427,29 @@ func (gc *GarbageCollector) collect(release *releaseapi.Release) error {
 
 	return nil
 }
+
+// ifRetainHistory tell if retain the release history, it will retain the history which between
+// (latestVersion-historyLimit : latestVersion] actually.
+// Why don't use latestVersion as parameter directly, if you want acquire latestVersion, you need
+// list all histories first which is not graceful. On the other hand in a sentence, the current
+// policy also satisfies the demand of limiting history number because the current version will be
+// equal with the latest version after updating the release.
+func (gc *GarbageCollector) ifRetainHistory(rls *releaseapi.Release, rlsHistoryName string) (bool, error) {
+	// The rls name may be "hello" and rlsHistory name be "hellowe12-v0", the first validation will ignore it.
+	// But when invoke `strconv.Atoi` the digit string "12-v0" will failed. So it is still invalid.
+	if !strings.HasPrefix(rlsHistoryName, rls.Name) {
+		return false, fmt.Errorf("cur release history %v is not belong the release %v", rlsHistoryName, rls.Name)
+	}
+	// the "-v" occupy two byte
+	version, err := strconv.Atoi(rlsHistoryName[len(rls.Name)+2:])
+	if err != nil {
+		return false, err
+	}
+	if version <= 0 {
+		return false, fmt.Errorf("cur release history %v version %v is  invalid", rlsHistoryName, version)
+	}
+	if version+int(gc.historyLimit) > int(rls.Status.Version) {
+		return true, nil
+	}
+	return false, nil
+}

pkg/controller/gc/controller_test.go

@@ -0,0 +1,52 @@
+package gc
+
+import (
+	"fmt"
+	"reflect"
+	"testing"
+
+	releaseapi "github.com/caicloud/clientset/pkg/apis/release/v1alpha1"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func newRelease(name string, version int32) *releaseapi.Release {
+	return &releaseapi.Release{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: name,
+		},
+		Status: releaseapi.ReleaseStatus{
+			Version: version,
+		},
+	}
+}
+
+func TestIsRetainHistory(t *testing.T) {
+	gc := &GarbageCollector{
+		historyLimit: 5,
+	}
+	testCases := []struct {
+		rls        string
+		curVersion int32
+		rlsHistory string
+		want       bool
+		err        error
+	}{
+		{"hello", 3, "hello-v1", true, nil},
+		{"hello", 10, "hello-v1", false, nil},
+		{"hello", 3, "hello-v0", false, fmt.Errorf("cur release history hello-v0 version 0 is  invalid")},
+		{"hello", 3, "hello-v-9", false, fmt.Errorf("cur release history hello-v-9 version -9 is  invalid")},
+		{"hello", 10, "hello-v5", false, nil},
+		{"hello", 10, "hello-v6", true, nil},
+		{"hello", 10, "hello1-v6", false, nil},
+		{"hello", 10, "hello-v20", true, nil},
+		{"hello", 10, "hello1-v20", false, nil},
+	}
+
+	for _, ca := range testCases {
+		get, err := gc.ifRetainHistory(newRelease(ca.rls, ca.curVersion), ca.rlsHistory)
+		if reflect.DeepEqual(err, ca.want) || get != ca.want {
+			t.Errorf("limit %v got %v but condition is %v err %v", gc.historyLimit, get, ca, err)
+		}
+	}
+}