golangci added to verify target, fixes

.golangci.yaml

@@ -0,0 +1,33 @@
+# See https://github.com/golangci/golangci-lint/blob/master/.golangci.example.yml
+run:
+  tests: false
+
+  timeout: 5m
+
+  build-tags:
+    - aws
+
+  skip-dirs:
+    - tools
+    - website
+    - hack
+    - charts
+    - designs
+
+linters:
+  enable:
+    - asciicheck
+    - errorlint
+    - gosec
+    - revive
+    - stylecheck
+    - tparallel
+    - unconvert
+    - unparam
+  disable:
+    - errcheck
+    - prealloc
+
+  linters-settings:
+    gci:
+      local-prefixes: github.com/awslabs/karpenter

pkg/apis/provisioning/v1alpha4/provisioner_validation.go

@@ -19,12 +19,12 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/awslabs/karpenter/pkg/utils/functional"
-	"github.com/awslabs/karpenter/pkg/utils/ptr"
-
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/validation"
 	"knative.dev/pkg/apis"
+
+	"github.com/awslabs/karpenter/pkg/utils/functional"
+	"github.com/awslabs/karpenter/pkg/utils/ptr"
 )
 
 func (p *Provisioner) Validate(ctx context.Context) (errs *apis.FieldError) {

pkg/cloudprovider/aws/ami.go

@@ -55,7 +55,7 @@ func (p *AMIProvider) Get(ctx context.Context, constraints *v1alpha1.Constraints
 	// Separate instance types by unique queries
 	amiQueries := map[string][]cloudprovider.InstanceType{}
 	for _, instanceType := range instanceTypes {
-		query := p.getSSMQuery(constraints, instanceType, version)
+		query := p.getSSMQuery(instanceType, version)
 		amiQueries[query] = append(amiQueries[query], instanceType)
 	}
 	// Separate instance types by unique AMIIDs
@@ -84,7 +84,7 @@ func (p *AMIProvider) getAMIID(ctx context.Context, query string) (string, error
 	return ami, nil
 }
 
-func (p *AMIProvider) getSSMQuery(constraints *v1alpha1.Constraints, instanceType cloudprovider.InstanceType, version string) string {
+func (p *AMIProvider) getSSMQuery(instanceType cloudprovider.InstanceType, version string) string {
 	var amiSuffix string
 	if !instanceType.NvidiaGPUs().IsZero() || !instanceType.AWSNeurons().IsZero() {
 		amiSuffix = "-gpu"

pkg/cloudprovider/aws/errors.go

@@ -0,0 +1,39 @@
+/*
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+    http://www.apache.org/licenses/LICENSE-2.0
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package aws
+
+import (
+	"errors"
+
+	"github.com/aws/aws-sdk-go/aws/awserr"
+	"github.com/awslabs/karpenter/pkg/utils/functional"
+)
+
+var (
+	// This is not an exhaustive list, add to it as needed
+	notFoundErrorCodes = []string{
+		"InvalidInstanceID.NotFound",
+		"InvalidLaunchTemplateName.NotFoundException",
+	}
+)
+
+// isNotFound returns true if the err is an AWS error (even if it's
+// wrapped) and is a known to mean "not found" (as opposed to a more
+// serious or unexpected error)
+func isNotFound(err error) bool {
+	var awsError awserr.Error
+	if errors.As(err, &awsError) {
+		return functional.ContainsString(notFoundErrorCodes, awsError.Code())
+	}
+	return false
+}

pkg/cloudprovider/aws/fake/ec2api.go

@@ -93,8 +93,8 @@ func (e *EC2API) DescribeInstancesWithContext(_ context.Context, input *ec2.Desc
 		return e.DescribeInstancesOutput, nil
 	}
 	instances := []*ec2.Instance{}
-	for _, instanceId := range input.InstanceIds {
-		instance, _ := e.Instances.Load(*instanceId)
+	for _, instanceID := range input.InstanceIds {
+		instance, _ := e.Instances.Load(*instanceID)
 		instances = append(instances, instance.(*ec2.Instance))
 	}
 

pkg/cloudprovider/aws/instance.go

@@ -22,22 +22,17 @@ import (
 
 	"github.com/avast/retry-go"
 	"github.com/aws/aws-sdk-go/aws"
-	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/aws/aws-sdk-go/service/ec2/ec2iface"
-	"github.com/awslabs/karpenter/pkg/apis/provisioning/v1alpha4"
-	"github.com/awslabs/karpenter/pkg/cloudprovider"
-	"github.com/awslabs/karpenter/pkg/cloudprovider/aws/apis/v1alpha1"
-	"knative.dev/pkg/logging"
-
 	"go.uber.org/multierr"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
-)
+	"knative.dev/pkg/logging"
 
-const (
-	EC2InstanceIDNotFoundErrCode = "InvalidInstanceID.NotFound"
+	"github.com/awslabs/karpenter/pkg/apis/provisioning/v1alpha4"
+	"github.com/awslabs/karpenter/pkg/cloudprovider"
+	"github.com/awslabs/karpenter/pkg/cloudprovider/aws/apis/v1alpha1"
 )
 
 type InstanceProvider struct {
@@ -101,7 +96,7 @@ func (p *InstanceProvider) Terminate(ctx context.Context, node *v1.Node) error {
 	if _, err = p.ec2api.TerminateInstancesWithContext(ctx, &ec2.TerminateInstancesInput{
 		InstanceIds: []*string{id},
 	}); err != nil {
-		if aerr, ok := err.(awserr.Error); ok && aerr.Code() == EC2InstanceIDNotFoundErrCode {
+		if isNotFound(err) {
 			return nil
 		}
 		return fmt.Errorf("terminating instance %s, %w", node.Name, err)
@@ -204,8 +199,8 @@ func (p *InstanceProvider) getOverrides(instanceTypeOptions []cloudprovider.Inst
 
 func (p *InstanceProvider) getInstances(ctx context.Context, ids []*string) ([]*ec2.Instance, error) {
 	describeInstancesOutput, err := p.ec2api.DescribeInstancesWithContext(ctx, &ec2.DescribeInstancesInput{InstanceIds: ids})
-	if aerr, ok := err.(awserr.Error); ok && aerr.Code() == EC2InstanceIDNotFoundErrCode {
-		return nil, aerr
+	if isNotFound(err) {
+		return nil, err
 	}
 	if err != nil {
 		return nil, fmt.Errorf("failed to describe ec2 instances, %w", err)

pkg/cloudprovider/aws/launchtemplate.go

@@ -23,7 +23,6 @@ import (
 	"strings"
 
 	"github.com/aws/aws-sdk-go/aws"
-	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/aws/aws-sdk-go/service/ec2/ec2iface"
 	"github.com/awslabs/karpenter/pkg/cloudprovider"
@@ -131,7 +130,7 @@ func (p *LaunchTemplateProvider) ensureLaunchTemplate(ctx context.Context, optio
 		LaunchTemplateNames: []*string{aws.String(name)},
 	})
 	// 3. Create LT if one doesn't exist
-	if aerr, ok := err.(awserr.Error); ok && aerr.Code() == "InvalidLaunchTemplateName.NotFoundException" {
+	if isNotFound(err) {
 		launchTemplate, err = p.createLaunchTemplate(ctx, options)
 		if err != nil {
 			return nil, fmt.Errorf("creating launch template, %w", err)

pkg/controllers/allocation/binpacking/packer.go

@@ -107,7 +107,7 @@ func (p *packer) Pack(ctx context.Context, schedule *scheduling.Schedule, instan
 		key, err := hashstructure.Hash(packing, hashstructure.FormatV2, &hashstructure.HashOptions{SlicesAsSets: true})
 		if err == nil {
 			if mainPack, ok := packs[key]; ok {
-				mainPack.NodeQuantity += 1
+				mainPack.NodeQuantity++
 				mainPack.Pods = append(mainPack.Pods, packing.Pods...)
 				logging.FromContext(ctx).Infof("Incremented node count to %d on packing for %d pod(s) with instance type option(s) %v", mainPack.NodeQuantity, flattenedLen(packing.Pods...), instanceTypeNames(mainPack.InstanceTypeOptions))
 				continue

pkg/controllers/allocation/controller.go

@@ -19,19 +19,13 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/awslabs/karpenter/pkg/apis/provisioning/v1alpha4"
-	"github.com/awslabs/karpenter/pkg/cloudprovider"
-	"github.com/awslabs/karpenter/pkg/controllers/allocation/binpacking"
-	"github.com/awslabs/karpenter/pkg/controllers/allocation/scheduling"
-	"github.com/awslabs/karpenter/pkg/utils/functional"
 	"go.uber.org/multierr"
-	"knative.dev/pkg/logging"
-
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
 	corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
 	"k8s.io/client-go/util/workqueue"
+	"knative.dev/pkg/logging"
 	controllerruntime "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -42,6 +36,12 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
+
+	"github.com/awslabs/karpenter/pkg/apis/provisioning/v1alpha4"
+	"github.com/awslabs/karpenter/pkg/cloudprovider"
+	"github.com/awslabs/karpenter/pkg/controllers/allocation/binpacking"
+	"github.com/awslabs/karpenter/pkg/controllers/allocation/scheduling"
+	"github.com/awslabs/karpenter/pkg/utils/functional"
 )
 
 const (

pkg/controllers/allocation/filter.go

@@ -39,7 +39,8 @@ func (f *Filter) GetProvisionablePods(ctx context.Context, provisioner *v1alpha4
 	}
 	// 2. Filter pods that aren't provisionable
 	provisionable := []*v1.Pod{}
-	for _, p := range pods.Items {
+	for i := range pods.Items {
+		p := pods.Items[i]
 		if err := f.isProvisionable(&p, provisioner); err != nil {
 			logging.FromContext(ctx).Debugf("Ignored pod %s/%s when allocating for provisioner %s, %s",
 				p.Name, p.Namespace, provisioner.Name, err.Error(),

pkg/controllers/allocation/scheduling/constraints.go

@@ -44,13 +44,11 @@ func NewConstraints(ctx context.Context, constraints *v1alpha4.Constraints, pod
 	if err := generateLabels(constraints, pod); err != nil {
 		return nil, err
 	}
-	if err := generateTaints(constraints, pod); err != nil {
-		return nil, err
-	}
+	generateTaints(constraints, pod)
 	return constraints, nil
 }
 
-func generateTaints(constraints *v1alpha4.Constraints, pod *v1.Pod) error {
+func generateTaints(constraints *v1alpha4.Constraints, pod *v1.Pod) {
 	taints := scheduling.Taints(constraints.Taints)
 	for _, toleration := range pod.Spec.Tolerations {
 		// Only OpEqual is supported. OpExists does not make sense for
@@ -77,7 +75,6 @@ func generateTaints(constraints *v1alpha4.Constraints, pod *v1.Pod) error {
 		}
 	}
 	constraints.Taints = taints
-	return nil
 }
 
 func generateLabels(constraints *v1alpha4.Constraints, pod *v1.Pod) error {

pkg/controllers/allocation/scheduling/topology.go

@@ -38,10 +38,7 @@ type Topology struct {
 // Inject injects topology rules into pods using supported NodeSelectors
 func (t *Topology) Inject(ctx context.Context, constraints *v1alpha4.Constraints, pods []*v1.Pod) error {
 	// 1. Group pods by equivalent topology spread constraints
-	topologyGroups, err := t.getTopologyGroups(pods)
-	if err != nil {
-		return fmt.Errorf("splitting topology groups, %w", err)
-	}
+	topologyGroups := t.getTopologyGroups(pods)
 	// 2. Compute spread
 	for _, topologyGroup := range topologyGroups {
 		if err := t.computeCurrentTopology(ctx, constraints, topologyGroup); err != nil {
@@ -58,7 +55,7 @@ func (t *Topology) Inject(ctx context.Context, constraints *v1alpha4.Constraints
 }
 
 // getTopologyGroups separates pods with equivalent topology rules
-func (t *Topology) getTopologyGroups(pods []*v1.Pod) ([]*TopologyGroup, error) {
+func (t *Topology) getTopologyGroups(pods []*v1.Pod) []*TopologyGroup {
 	topologyGroupMap := map[uint64]*TopologyGroup{}
 	for _, pod := range pods {
 		for _, constraint := range pod.Spec.TopologySpreadConstraints {
@@ -75,7 +72,7 @@ func (t *Topology) getTopologyGroups(pods []*v1.Pod) ([]*TopologyGroup, error) {
 	for _, topologyGroup := range topologyGroupMap {
 		topologyGroups = append(topologyGroups, topologyGroup)
 	}
-	return topologyGroups, nil
+	return topologyGroups
 }
 
 func (t *Topology) computeCurrentTopology(ctx context.Context, constraints *v1alpha4.Constraints, topologyGroup *TopologyGroup) error {

pkg/controllers/node/controller.go

@@ -18,9 +18,6 @@ import (
 	"context"
 	"fmt"
 
-	"github.com/awslabs/karpenter/pkg/apis/provisioning/v1alpha4"
-	"github.com/awslabs/karpenter/pkg/utils/result"
-
 	"go.uber.org/multierr"
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
@@ -34,6 +31,9 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
+
+	"github.com/awslabs/karpenter/pkg/apis/provisioning/v1alpha4"
+	"github.com/awslabs/karpenter/pkg/utils/result"
 )
 
 // NewController constructs a controller instance

pkg/controllers/node/emptiness.go

@@ -86,7 +86,8 @@ func (r *Emptiness) isEmpty(ctx context.Context, n *v1.Node) (bool, error) {
 	if err := r.kubeClient.List(ctx, pods, client.MatchingFields{"spec.nodeName": n.Name}); err != nil {
 		return false, fmt.Errorf("listing pods for node %s, %w", n.Name, err)
 	}
-	for _, p := range pods.Items {
+	for i := range pods.Items {
+		p := pods.Items[i]
 		if pod.HasFailed(&p) {
 			continue
 		}

pkg/controllers/termination/controller.go

@@ -19,9 +19,6 @@ import (
 	"fmt"
 	"time"
 
-	provisioning "github.com/awslabs/karpenter/pkg/apis/provisioning/v1alpha4"
-	"github.com/awslabs/karpenter/pkg/cloudprovider"
-	"github.com/awslabs/karpenter/pkg/utils/functional"
 	"golang.org/x/time/rate"
 	"knative.dev/pkg/logging"
 
@@ -34,6 +31,10 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+
+	provisioning "github.com/awslabs/karpenter/pkg/apis/provisioning/v1alpha4"
+	"github.com/awslabs/karpenter/pkg/cloudprovider"
+	"github.com/awslabs/karpenter/pkg/utils/functional"
 )
 
 // Controller for the resource

pkg/controllers/termination/terminate.go

@@ -18,19 +18,18 @@ import (
 	"context"
 	"fmt"
 
+	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
+	corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
+	"knative.dev/pkg/logging"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
 	provisioning "github.com/awslabs/karpenter/pkg/apis/provisioning/v1alpha4"
 	"github.com/awslabs/karpenter/pkg/cloudprovider"
 	"github.com/awslabs/karpenter/pkg/scheduling"
 	"github.com/awslabs/karpenter/pkg/utils/functional"
 	"github.com/awslabs/karpenter/pkg/utils/injectabletime"
 	"github.com/awslabs/karpenter/pkg/utils/ptr"
-
-	"knative.dev/pkg/logging"
-
-	v1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/api/errors"
-	corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 type Terminator struct {

pkg/scheduling/taints.go

@@ -35,7 +35,8 @@ func (ts Taints) Has(taint v1.Taint) bool {
 
 // Tolerates returns true if the pod tolerates all taints
 func (ts Taints) Tolerates(pod *v1.Pod) (errs error) {
-	for _, taint := range ts {
+	for i := range ts {
+		taint := ts[i]
 		tolerates := false
 		for _, t := range pod.Spec.Tolerations {
 			tolerates = tolerates || t.ToleratesTaint(&taint)