Merge pull request #103 from asobti/killValue

Fix for case where invalid KillType and KillValue result in single po…

Makefile

@@ -36,6 +36,10 @@ container:
 gofmt:
 	find . -path ./vendor -prune -o -name '*.go' -print | xargs -L 1 -I % gofmt -s -w %
 
+# Same as gofmt, but also orders imports
+goimports:
+	find . -path ./vendor -prune -o -name '*.go' -print | xargs -L 1 -I % goimports -w %
+
 clean:
 	rm -f kube-monkey
 

chaos/chaos.go

@@ -4,7 +4,7 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/golang/glog"
+	"github.com/pkg/errors"
 
 	"github.com/asobti/kube-monkey/config"
 	"github.com/asobti/kube-monkey/kubernetes"
@@ -102,32 +102,50 @@ func (c *Chaos) verifyExecution(clientset kube.Interface) error {
 func (c *Chaos) terminate(clientset kube.Interface) error {
 	killType, err := c.Victim().KillType(clientset)
 	if err != nil {
-		glog.Errorf("Failed to check KillType label for %s %s. Proceeding with termination of a single pod. Error: %v", c.Victim().Kind(), c.Victim().Name(), err.Error())
-		return c.terminatePod(clientset)
+		return errors.Wrapf(err, "Failed to check KillType label for %s %s", c.Victim().Kind(), c.Victim().Name())
 	}
 
-	killValue, err := c.Victim().KillValue(clientset)
-	if err != nil {
-		glog.Errorf("Failed to check KillValue label for %s %s. Proceeding with termination of a single pod. Error: %v", c.Victim().Kind(), c.Victim().Name(), err.Error())
-		return c.terminatePod(clientset)
+	killValue, err := c.getKillValue(clientset)
+
+	// KillAll is the only kill type that does not require a kill-value
+	if killType != config.KillAllLabelValue && err != nil {
+		return err
 	}
 
 	// Validate killtype
 	switch killType {
 	case config.KillFixedLabelValue:
 		return c.Victim().DeleteRandomPods(clientset, killValue)
 	case config.KillAllLabelValue:
-		killNum := c.Victim().KillNumberForKillingAll(clientset, killValue)
+		killNum, err := c.Victim().KillNumberForKillingAll(clientset)
+		if err != nil {
+			return err
+		}
 		return c.Victim().DeleteRandomPods(clientset, killNum)
 	case config.KillRandomMaxLabelValue:
-		killNum := c.Victim().KillNumberForMaxPercentage(clientset, killValue)
+		killNum, err := c.Victim().KillNumberForMaxPercentage(clientset, killValue)
+		if err != nil {
+			return err
+		}
 		return c.Victim().DeleteRandomPods(clientset, killNum)
 	case config.KillFixedPercentageLabelValue:
-		killNum := c.Victim().KillNumberForFixedPercentage(clientset, killValue)
+		killNum, err := c.Victim().KillNumberForFixedPercentage(clientset, killValue)
+		if err != nil {
+			return err
+		}
 		return c.Victim().DeleteRandomPods(clientset, killNum)
 	default:
-		return fmt.Errorf("Failed to recognize KillType label for %s %s", c.Victim().Kind(), c.Victim().Name())
+		return fmt.Errorf("failed to recognize KillType label for %s %s", c.Victim().Kind(), c.Victim().Name())
+	}
+}
+
+func (c *Chaos) getKillValue(clientset kube.Interface) (int, error) {
+	killValue, err := c.Victim().KillValue(clientset)
+	if err != nil {
+		return 0, errors.Wrapf(err, "Failed to check KillValue label for %s %s", c.Victim().Kind(), c.Victim().Name())
 	}
+
+	return killValue, nil
 }
 
 // Redundant for DeleteRandomPods(clientset,1) but DeleteRandomPod is faster

chaos/chaos_test.go

@@ -63,20 +63,19 @@ func (s *ChaosTestSuite) TestVerifyExecutionWhitelisted() {
 
 func (s *ChaosTestSuite) TestTerminateKillTypeError() {
 	v := s.chaos.victim.(*victimMock)
-	errMsg := "KillType Error"
-	v.On("KillType", s.client).Return("", errors.New(errMsg))
-	v.On("DeleteRandomPod", s.client).Return(nil)
-	_ = s.chaos.terminate(s.client)
+	err := errors.New("KillType Error")
+	v.On("KillType", s.client).Return("", err)
+
+	s.NotNil(s.chaos.terminate(s.client))
 	v.AssertExpectations(s.T())
 }
 
 func (s *ChaosTestSuite) TestTerminateKillValueError() {
 	v := s.chaos.victim.(*victimMock)
 	errMsg := "KillValue Error"
-	v.On("KillType", s.client).Return("", nil)
+	v.On("KillType", s.client).Return(config.KillFixedLabelValue, nil)
 	v.On("KillValue", s.client).Return(0, errors.New(errMsg))
-	v.On("DeleteRandomPod", s.client).Return(nil)
-	_ = s.chaos.terminate(s.client)
+	s.NotNil(s.chaos.terminate(s.client))
 	v.AssertExpectations(s.T())
 }
 
@@ -92,10 +91,9 @@ func (s *ChaosTestSuite) TestTerminateKillFixed() {
 
 func (s *ChaosTestSuite) TestTerminateAllPods() {
 	v := s.chaos.victim.(*victimMock)
-	killValue := 1
 	v.On("KillType", s.client).Return(config.KillAllLabelValue, nil)
-	v.On("KillValue", s.client).Return(killValue, nil)
-	v.On("KillNumberForKillingAll", s.client, killValue).Return(0)
+	v.On("KillValue", s.client).Return(0, nil)
+	v.On("KillNumberForKillingAll", s.client).Return(0, nil)
 	v.On("DeleteRandomPods", s.client, 0).Return(nil)
 	_ = s.chaos.terminate(s.client)
 	v.AssertExpectations(s.T())
@@ -106,7 +104,7 @@ func (s *ChaosTestSuite) TestTerminateKillRandomMaxPercentage() {
 	killValue := 1
 	v.On("KillType", s.client).Return(config.KillRandomMaxLabelValue, nil)
 	v.On("KillValue", s.client).Return(killValue, nil)
-	v.On("KillNumberForMaxPercentage", s.client, mock.AnythingOfType("int")).Return(0)
+	v.On("KillNumberForMaxPercentage", s.client, mock.AnythingOfType("int")).Return(0, nil)
 	v.On("DeleteRandomPods", s.client, 0).Return(nil)
 	_ = s.chaos.terminate(s.client)
 	v.AssertExpectations(s.T())
@@ -117,20 +115,35 @@ func (s *ChaosTestSuite) TestTerminateKillFixedPercentage() {
 	killValue := 1
 	v.On("KillType", s.client).Return(config.KillFixedPercentageLabelValue, nil)
 	v.On("KillValue", s.client).Return(killValue, nil)
-	v.On("KillNumberForFixedPercentage", s.client, mock.AnythingOfType("int")).Return(0)
+	v.On("KillNumberForFixedPercentage", s.client, mock.AnythingOfType("int")).Return(0, nil)
 	v.On("DeleteRandomPods", s.client, 0).Return(nil)
 	_ = s.chaos.terminate(s.client)
 	v.AssertExpectations(s.T())
 }
 
 func (s *ChaosTestSuite) TestInvalidKillType() {
 	v := s.chaos.victim.(*victimMock)
-	killValue := 1
 	v.On("KillType", s.client).Return("InvalidKillTypeHere", nil)
-	v.On("KillValue", s.client).Return(killValue, nil)
+	v.On("KillValue", s.client).Return(0, nil)
 	err := s.chaos.terminate(s.client)
 	v.AssertExpectations(s.T())
-	s.EqualError(err, "Failed to recognize KillType label for Pod "+v.Name()+"")
+	s.NotNil(err)
+}
+
+func (s *ChaosTestSuite) TestGetKillValue() {
+	v := s.chaos.victim.(*victimMock)
+	killValue := 5
+	v.On("KillValue", s.client).Return(killValue, nil)
+	result, err := s.chaos.getKillValue(s.client)
+	s.Nil(err)
+	s.Equal(killValue, result)
+}
+
+func (s *ChaosTestSuite) TestGetKillValueReturnsError() {
+	v := s.chaos.victim.(*victimMock)
+	v.On("KillValue", s.client).Return(0, errors.New("InvalidKillValue"))
+	_, err := s.chaos.getKillValue(s.client)
+	s.NotNil(err)
 }
 
 func (s *ChaosTestSuite) TestDurationToKillTime() {

chaos/chaosmock.go

@@ -46,19 +46,19 @@ func (vm *victimMock) DeleteRandomPods(clientset kube.Interface, killValue int)
 	return args.Error(0)
 }
 
-func (vm *victimMock) KillNumberForKillingAll(clientset kube.Interface, killValue int) int {
-	args := vm.Called(clientset, killValue)
-	return args.Int(0)
+func (vm *victimMock) KillNumberForKillingAll(clientset kube.Interface) (int, error) {
+	args := vm.Called(clientset)
+	return args.Int(0), args.Error(1)
 }
 
-func (vm *victimMock) KillNumberForMaxPercentage(clientset kube.Interface, killValue int) int {
+func (vm *victimMock) KillNumberForMaxPercentage(clientset kube.Interface, killValue int) (int, error) {
 	args := vm.Called(clientset, killValue)
-	return args.Int(0)
+	return args.Int(0), args.Error(1)
 }
 
-func (vm *victimMock) KillNumberForFixedPercentage(clientset kube.Interface, killValue int) int {
+func (vm *victimMock) KillNumberForFixedPercentage(clientset kube.Interface, killValue int) (int, error) {
 	args := vm.Called(clientset, killValue)
-	return args.Int(0)
+	return args.Int(0), args.Error(1)
 }
 
 func (vm *victimMock) IsBlacklisted() bool {

victims/victims.go

@@ -55,9 +55,9 @@ type VictimAPICalls interface {
 }
 
 type VictimKillNumberGenerator interface {
-	KillNumberForMaxPercentage(kube.Interface, int) int
-	KillNumberForKillingAll(kube.Interface, int) int
-	KillNumberForFixedPercentage(kube.Interface, int) int
+	KillNumberForMaxPercentage(kube.Interface, int) (int, error)
+	KillNumberForKillingAll(kube.Interface) (int, error)
+	KillNumberForFixedPercentage(kube.Interface, int) (int, error)
 }
 
 type VictimBase struct {
@@ -166,35 +166,33 @@ func (v *VictimBase) DeleteRandomPods(clientset kube.Interface, killNum int) err
 	switch {
 	case numPods == 0:
 		return fmt.Errorf("%s %s has no running pods at the moment", v.kind, v.name)
+	case killNum == 0:
+		return fmt.Errorf("no terminations requested for %s %s", v.kind, v.name)
 	case numPods < killNum:
 		glog.Warningf("%s %s has only %d currently running pods, but %d terminations requested", v.kind, v.name, numPods, killNum)
 		fallthrough
 	case numPods == killNum:
 		glog.V(6).Infof("Killing ALL %d running pods for %s %s", numPods, v.kind, v.name)
-	case killNum == 0:
-		return fmt.Errorf("No terminations requested for %s %s", v.kind, v.name)
 	case killNum < 0:
-		return fmt.Errorf("Cannot request negative terminations %d for %s %s", numPods, v.kind, v.name)
+		return fmt.Errorf("cannot request negative terminations %d for %s %s", killNum, v.kind, v.name)
 	case numPods > killNum:
-		glog.V(6).Infof("Killing %d running pods for %s %s", numPods, v.kind, v.name)
+		glog.V(6).Infof("Killing %d running pods for %s %s", killNum, v.kind, v.name)
 	default:
 		return fmt.Errorf("unexpected behavior for terminating %s %s", v.kind, v.name)
 	}
 
 	r := rand.New(rand.NewSource(time.Now().UnixNano()))
-	killCount := 0
-	for _, i := range r.Perm(numPods) {
-		if killCount == killNum {
-			// Report success
-			return nil
-		}
-		targetPod := pods[i].Name
-		glog.V(6).Infof("Terminating pod %s for %s %s\n", targetPod, v.kind, v.name)
+
+	for i := 0; i < killNum; i++ {
+		victimIndex := r.Intn(numPods)
+		targetPod := pods[victimIndex].Name
+
+		glog.V(6).Infof("Terminating pod %s for %s %s/%s\n", targetPod, v.kind, v.namespace, v.name)
+
 		err = v.DeletePod(clientset, targetPod)
 		if err != nil {
 			return err
 		}
-		killCount++
 	}
 
 	// Successful termination
@@ -264,69 +262,67 @@ func RandomPodName(pods []v1.Pod) string {
 }
 
 // Returns the number of pods to kill based on the number of all running pods
-func (v *VictimBase) KillNumberForKillingAll(clientset kube.Interface, killPercentage int) int {
-	killNum := v.numberOfRunningPods(clientset)
+func (v *VictimBase) KillNumberForKillingAll(clientset kube.Interface) (int, error) {
+	killNum, err := v.numberOfRunningPods(clientset)
+	if err != nil {
+		return 0, err
+	}
 
-	return killNum
+	return killNum, nil
 }
 
 // Returns the number of pods to kill based on a kill percentage and the number of running pods
-func (v *VictimBase) KillNumberForFixedPercentage(clientset kube.Interface, killPercentage int) int {
+func (v *VictimBase) KillNumberForFixedPercentage(clientset kube.Interface, killPercentage int) (int, error) {
 	if killPercentage == 0 {
 		glog.V(6).Infof("Not terminating any pods for %s %s as kill percentage is 0\n", v.kind, v.name)
 		// Report success
-		return 0
+		return 0, nil
 	}
-	if killPercentage < 0 {
-		glog.V(6).Infof("Expected kill percentage config %d to be between 0 and 100 for %s %s. Defaulting to 0", killPercentage, v.kind, v.name)
-		killPercentage = 0
-	}
-	if killPercentage > 100 {
-		glog.V(6).Infof("Expected kill percentage config %d to be between 0 and 100 for %s %s. Defaulting to 100", killPercentage, v.kind, v.name)
-		killPercentage = 100
+	if killPercentage < 0 || killPercentage > 100 {
+		return 0, fmt.Errorf("percentage value of %d is invalid. Must be [0-100]", killPercentage)
 	}
 
-	numRunningPods := v.numberOfRunningPods(clientset)
+	numRunningPods, err := v.numberOfRunningPods(clientset)
+	if err != nil {
+		return 0, err
+	}
 
 	numberOfPodsToKill := float64(numRunningPods) * float64(killPercentage) / 100
 	killNum := int(math.Floor(numberOfPodsToKill))
 
-	return killNum
+	return killNum, nil
 }
 
 // Returns a number of pods to kill based on a a random kill percentage (between 0 and maxPercentage) and the number of running pods
-func (v *VictimBase) KillNumberForMaxPercentage(clientset kube.Interface, maxPercentage int) int {
+func (v *VictimBase) KillNumberForMaxPercentage(clientset kube.Interface, maxPercentage int) (int, error) {
 	if maxPercentage == 0 {
-		glog.V(6).Infof("Not terminating any pods for %s %s as kill percentage is 0\n", v.kind, v.name)
+		glog.V(6).Infof("Not terminating any pods for %s %s as kill percentage is 0", v.kind, v.name)
 		// Report success
-		return 0
+		return 0, nil
 	}
-	if maxPercentage < 0 {
-		glog.V(6).Infof("Expected kill percentage config %d to be between 0 and 100 for %s %s. Defaulting to 0%%", maxPercentage, v.kind, v.name)
-		maxPercentage = 0
-	}
-	if maxPercentage > 100 {
-		glog.V(6).Infof("Expected kill percentage config %d to be between 0 and 100 for %s %s. Defaulting to 100%%", maxPercentage, v.kind, v.name)
-		maxPercentage = 100
+	if maxPercentage < 0 || maxPercentage > 100 {
+		return 0, fmt.Errorf("percentage value of %d is invalid. Must be [0-100]", maxPercentage)
 	}
 
-	numRunningPods := v.numberOfRunningPods(clientset)
+	numRunningPods, err := v.numberOfRunningPods(clientset)
+	if err != nil {
+		return 0, err
+	}
 
 	r := rand.New(rand.NewSource(time.Now().UnixNano()))
 	killPercentage := r.Intn(maxPercentage + 1) // + 1 because Intn works with half open interval [0,n) and we want [0,n]
 	numberOfPodsToKill := float64(numRunningPods) * float64(killPercentage) / 100
 	killNum := int(math.Floor(numberOfPodsToKill))
 
-	return killNum
+	return killNum, nil
 }
 
 // Returns the number of running pods or 0 if the operation fails
-func (v *VictimBase) numberOfRunningPods(clientset kube.Interface) int {
+func (v *VictimBase) numberOfRunningPods(clientset kube.Interface) (int, error) {
 	pods, err := v.RunningPods(clientset)
 	if err != nil {
-		glog.V(6).Infof("Failed to get list of running pods %s %s", v.kind, v.name)
-		return 0
+		return 0, errors.Wrapf(err, "Failed to get running pods for victim %s %s", v.kind, v.name)
 	}
 
-	return len(pods)
+	return len(pods), nil
 }