func: set all the checks functions and types as private, remove the check for nil policy

Author: Juanadelacuesta

policy/check.go

@@ -41,7 +41,7 @@ type checkRunner struct {
 }
 
 // NewCheckHandler returns a new checkHandler instance.
-func NewCheckRunner(config *CheckRunnerConfig, c *sdk.ScalingPolicyCheck) *checkRunner {
+func newCheckRunner(config *CheckRunnerConfig, c *sdk.ScalingPolicyCheck) *checkRunner {
 	return &checkRunner{
 		log:            config.Log,
 		check:          c,
@@ -131,7 +131,7 @@ func (ch *checkRunner) runStrategy(ctx context.Context, currentCount int64, ms s
 	return *runResp.Action, nil
 }
 
-// QueryMetrics wraps the apm.Query call to provide operational functionality.
+// queryMetrics wraps the apm.Query call to provide operational functionality.
 func (ch *checkRunner) queryMetrics(ctx context.Context) (sdk.TimestampedMetrics, error) {
 	ch.log.Debug("querying source", "query", ch.check.Query, "source", ch.check.Source)
 
@@ -177,11 +177,11 @@ func (ch *checkRunner) queryMetrics(ctx context.Context) (sdk.TimestampedMetrics
 	return ms, nil
 }
 
-func (ch *checkRunner) Group() string {
+func (ch *checkRunner) group() string {
 	return ch.check.Group
 }
 
-func (ch *checkRunner) RunCheckAndCapCount(ctx context.Context, currentCount int64) (sdk.ScalingAction, error) {
+func (ch *checkRunner) runCheckAndCapCount(ctx context.Context, currentCount int64) (sdk.ScalingAction, error) {
 	ch.log.Debug("received policy check for evaluation")
 
 	metrics, err := ch.queryMetrics(ctx)

policy/check_test.go

@@ -148,7 +148,7 @@ func TestCheckHandler_getNewCountFromMetrics(t *testing.T) {
 				},
 			}
 
-			runner := NewCheckRunner(&CheckRunnerConfig{
+			runner := newCheckRunner(&CheckRunnerConfig{
 				Log:            hclog.NewNullLogger(),
 				StrategyRunner: sr,
 				Policy:         tt.policy,
@@ -226,7 +226,7 @@ func TestCheckHandler_runAPMQuery(t *testing.T) {
 				},
 			}
 
-			handler := NewCheckRunner(&CheckRunnerConfig{
+			handler := newCheckRunner(&CheckRunnerConfig{
 				Log:           hclog.NewNullLogger(),
 				MetricsGetter: mockLooker,
 				Policy: &sdk.ScalingPolicy{

policy/handler.go

@@ -61,8 +61,8 @@ type limiter interface {
 }
 
 type checker interface {
-	RunCheckAndCapCount(ctx context.Context, currentCount int64) (sdk.ScalingAction, error)
-	Group() string
+	runCheckAndCapCount(ctx context.Context, currentCount int64) (sdk.ScalingAction, error)
+	group() string
 }
 
 // Handler monitors a policy for changes and controls when them are sent for
@@ -107,39 +107,40 @@ type Handler struct {
 }
 
 type HandlerConfig struct {
-	UpdatesChan         chan *sdk.ScalingPolicy
-	ErrChan             chan<- error
-	Policy              *sdk.ScalingPolicy
-	Log                 hclog.Logger
-	TargetController    targetpkg.Controller
-	Limiter             *Limiter
-	DependencyGetter    dependencyGetter
+	UpdatesChan      chan *sdk.ScalingPolicy
+	ErrChan          chan<- error
+	Policy           *sdk.ScalingPolicy
+	Log              hclog.Logger
+	TargetController targetpkg.Controller
+	Limiter          *Limiter
+	DependencyGetter dependencyGetter
+
+	// Ent only field
 	HistoricalAPMGetter HistoricalAPMGetter
 	EvaluateAfter       time.Duration
 }
 
 func NewPolicyHandler(config HandlerConfig) (*Handler, error) {
-
 	h := &Handler{
 		log: config.Log,
 		mutators: []Mutator{
 			NomadAPMMutator{},
 		},
 		pm:                  config.DependencyGetter,
-		historicalAPMGetter: config.HistoricalAPMGetter,
-		evaluateAfter:       config.EvaluateAfter,
 		targetController:    config.TargetController,
 		updatesCh:           config.UpdatesChan,
 		policy:              config.Policy,
 		errChn:              config.ErrChan,
 		limiter:             config.Limiter,
 		stateLock:           sync.RWMutex{},
 		state:               StateIdle,
+		historicalAPMGetter: config.HistoricalAPMGetter,
+		evaluateAfter:       config.EvaluateAfter,
 	}
 
 	err := h.loadCheckRunners()
 	if err != nil {
-		return nil, fmt.Errorf("unable to load the checks for the handler: %w", err)
+		return nil, fmt.Errorf("failed to load check handlers: %w", err)
 	}
 
 	currentStatus, err := h.runTargetStatus()
@@ -174,7 +175,8 @@ func NewPolicyHandler(config HandlerConfig) (*Handler, error) {
 	return h, nil
 }
 
-// Convert the last event string.
+// checkForOutOfBandEvents tries to determine if there has been any recent
+// scaling events in case of the autoscaler going offline.
 func checkForOutOfBandEvents(status *sdk.TargetStatus) (int64, error) {
 	// If the target status includes a last event meta key, check for cooldown
 	// due to out-of-band events. This is also useful if the Autoscaler has
@@ -208,7 +210,7 @@ func (h *Handler) loadCheckRunners() error {
 				return fmt.Errorf("failed to get APM for strategy %s: %w", check.Strategy.Name, err)
 			}
 
-			runner := NewCheckRunner(&CheckRunnerConfig{
+			runner := newCheckRunner(&CheckRunnerConfig{
 				Log: h.log.Named("check_handler").With("check", check.Name,
 					"source", check.Source, "strategy", check.Strategy.Name),
 				StrategyRunner: s,
@@ -437,13 +439,13 @@ func (h *Handler) calculateNewCount(ctx context.Context, currentCount int64) (sd
 	checkGroups := make(map[string][]checkResult)
 
 	for _, ch := range h.checkRunners {
-		action, err := ch.RunCheckAndCapCount(ctx, currentCount)
+		action, err := ch.runCheckAndCapCount(ctx, currentCount)
 		if err != nil {
 			return sdk.ScalingAction{}, fmt.Errorf("failed get count from metrics: %v", err)
 
 		}
 
-		g := ch.Group()
+		g := ch.group()
 		checkGroups[g] = append(checkGroups[g], checkResult{
 			action:  &action,
 			handler: ch,

policy/handler_oss.go

@@ -20,7 +20,7 @@ type noopVerticalCheckRunner struct {
 	policy *sdk.ScalingPolicy
 }
 
-func (nv *noopVerticalCheckRunner) RunCheckAndCapCount(_ context.Context, currentCount int64) (sdk.ScalingAction, error) {
+func (nv *noopVerticalCheckRunner) runCheckAndCapCount(_ context.Context, currentCount int64) (sdk.ScalingAction, error) {
 	a := sdk.ScalingAction{
 		Direction: sdk.ScaleDirectionNone,
 		Count:     currentCount,
@@ -31,7 +31,7 @@ func (nv *noopVerticalCheckRunner) RunCheckAndCapCount(_ context.Context, curren
 	return a, nil
 }
 
-func (nv *noopVerticalCheckRunner) Group() string {
+func (nv *noopVerticalCheckRunner) group() string {
 	return ""
 }
 

policy/handler_test.go

@@ -290,7 +290,6 @@ func Test_pickWinnerActionFromGroups(t *testing.T) {
 				must.Eq(t, tt.wantAction.Direction, result.action.Direction)
 				must.Eq(t, tt.wantAction.Count, result.action.Count)
 				must.NotNil(t, result.handler)
-				//must.Eq(t, tt.wantHandler.check.Name, result.handler.check.Name)
 			}
 		})
 	}

sdk/policy.go

@@ -87,7 +87,7 @@ type ScalingPolicy struct {
 // Validate applies validation rules that are independent of policy source.
 func (p *ScalingPolicy) Validate() error {
 	if p == nil {
-		return errors.New("empty policy")
+		return nil
 	}
 
 	var result *multierror.Error

sdk/policy_test.go

@@ -16,11 +16,6 @@ func TestScalingPolicy_Validate(t *testing.T) {
 		policy        *ScalingPolicy
 		expectedError string
 	}{
-		{
-			name:          "nil policy",
-			policy:        nil,
-			expectedError: "empty policy",
-		},
 		{
 			name: "no type",
 			policy: &ScalingPolicy{