From 07690fe6f937e9a7b9164a64718f05544537718b Mon Sep 17 00:00:00 2001 From: Miguel Varela Ramos Date: Fri, 19 Mar 2021 12:36:03 +0100 Subject: [PATCH 1/7] Fix saving pointer reference in for/range loop --- pkg/operator/resources/job/batchapi/api.go | 8 ++++---- pkg/operator/resources/job/taskapi/api.go | 10 +++++----- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/operator/resources/job/batchapi/api.go b/pkg/operator/resources/job/batchapi/api.go index cf339d90f2..7e0643850a 100644 --- a/pkg/operator/resources/job/batchapi/api.go +++ b/pkg/operator/resources/job/batchapi/api.go @@ -128,8 +128,8 @@ func GetAllAPIs(virtualServices []istioclientnetworking.VirtualService, k8sJobs batchAPIsMap := map[string]*schema.APIResponse{} jobIDToK8sJobMap := map[string]*kbatch.Job{} - for _, kJob := range k8sJobs { - jobIDToK8sJobMap[kJob.Labels["jobID"]] = &kJob + for i, kJob := range k8sJobs { + jobIDToK8sJobMap[kJob.Labels["jobID"]] = &k8sJobs[i] } jobIDToPodsMap := map[string][]kcore.Pod{} @@ -229,8 +229,8 @@ func GetAPIByName(deployedResource *operator.DeployedResource) ([]schema.APIResp } jobIDToK8sJobMap := map[string]*kbatch.Job{} - for _, kJob := range k8sJobs { - jobIDToK8sJobMap[kJob.Labels["jobID"]] = &kJob + for i, kJob := range k8sJobs { + jobIDToK8sJobMap[kJob.Labels["jobID"]] = &k8sJobs[i] } endpoint, err := operator.APIEndpoint(api) diff --git a/pkg/operator/resources/job/taskapi/api.go b/pkg/operator/resources/job/taskapi/api.go index 0ae98cb3d9..fa6c8b6bae 100644 --- a/pkg/operator/resources/job/taskapi/api.go +++ b/pkg/operator/resources/job/taskapi/api.go @@ -123,8 +123,8 @@ func GetAllAPIs(virtualServices []istioclientnetworking.VirtualService, k8sJobs taskAPIsMap := map[string]*schema.APIResponse{} jobIDToK8sJobMap := map[string]*kbatch.Job{} - for _, job := range k8sJobs { - jobIDToK8sJobMap[job.Labels["jobID"]] = &job + for i, kJob := range k8sJobs { + jobIDToK8sJobMap[kJob.Labels["jobID"]] = &k8sJobs[i] } jobIDToPodsMap := map[string][]kcore.Pod{} @@ -204,7 +204,7 @@ func GetAllAPIs(virtualServices []istioclientnetworking.VirtualService, k8sJobs return taskAPIList, nil } -// GetAllAPIs returns a single task API and its most recently submitted job along with all running task jobs +// GetAPIByName returns a single task API and its most recently submitted job along with all running task jobs func GetAPIByName(deployedResource *operator.DeployedResource) ([]schema.APIResponse, error) { virtualService := deployedResource.VirtualService @@ -220,8 +220,8 @@ func GetAPIByName(deployedResource *operator.DeployedResource) ([]schema.APIResp } jobIDToK8sJobMap := map[string]*kbatch.Job{} - for _, job := range k8sJobs { - jobIDToK8sJobMap[job.Labels["jobID"]] = &job + for i, kJob := range k8sJobs { + jobIDToK8sJobMap[kJob.Labels["jobID"]] = &k8sJobs[i] } endpoint, err := operator.APIEndpoint(api) From ba35eadd409f131678b2bc69e872b468d87bff0b Mon Sep 17 00:00:00 2001 From: Miguel Varela Ramos Date: Fri, 19 Mar 2021 13:05:25 +0100 Subject: [PATCH 2/7] Fix or ignore looppointer warnings --- pkg/lib/archive/archiver.go | 16 ++++++++-------- pkg/lib/k8s/pod.go | 1 + pkg/operator/endpoints/info.go | 4 +++- pkg/operator/main.go | 8 ++++---- pkg/operator/resources/asyncapi/api.go | 2 +- pkg/operator/resources/asyncapi/status.go | 2 +- pkg/operator/resources/job/batchapi/cron.go | 1 + pkg/operator/resources/job/taskapi/cron.go | 2 +- pkg/operator/resources/job/worker_stats.go | 4 ++-- pkg/operator/resources/realtimeapi/api.go | 2 +- pkg/operator/resources/realtimeapi/status.go | 10 +++++----- pkg/operator/resources/trafficsplitter/api.go | 2 +- pkg/operator/resources/validations.go | 4 ++-- 13 files changed, 31 insertions(+), 27 deletions(-) diff --git a/pkg/lib/archive/archiver.go b/pkg/lib/archive/archiver.go index 1a02724643..40a8dd9878 100644 --- a/pkg/lib/archive/archiver.go +++ b/pkg/lib/archive/archiver.go @@ -46,29 +46,29 @@ func archive(input *Input, arc archiver) (strset.Set, error) { addedPaths := strset.New() var err error - for _, byteInput := range input.Bytes { - err = addBytesToArchive(&byteInput, input, arc, addedPaths) + for i := range input.Bytes { + err = addBytesToArchive(&input.Bytes[i], input, arc, addedPaths) if err != nil { return nil, err } } - for _, fileInput := range input.Files { - err = addFileToArchive(&fileInput, input, arc, addedPaths) + for i := range input.Files { + err = addFileToArchive(&input.Files[i], input, arc, addedPaths) if err != nil { return nil, err } } - for _, dirInput := range input.Dirs { - err = addDirToArchive(&dirInput, input, arc, addedPaths) + for i := range input.Dirs { + err = addDirToArchive(&input.Dirs[i], input, arc, addedPaths) if err != nil { return nil, err } } - for _, fileListInput := range input.FileLists { - err = addFileListToArchive(&fileListInput, input, arc, addedPaths) + for i := range input.FileLists { + err = addFileListToArchive(&input.FileLists[i], input, arc, addedPaths) if err != nil { return nil, err } diff --git a/pkg/lib/k8s/pod.go b/pkg/lib/k8s/pod.go index ea6ed8c14b..084471adb7 100644 --- a/pkg/lib/k8s/pod.go +++ b/pkg/lib/k8s/pod.go @@ -139,6 +139,7 @@ func GetPodReadyTime(pod *kcore.Pod) *time.Time { if condition.LastTransitionTime.Time.IsZero() { return nil } + condition := condition return &condition.LastTransitionTime.Time } } diff --git a/pkg/operator/endpoints/info.go b/pkg/operator/endpoints/info.go index 8d762a9e3d..3d64d2778e 100644 --- a/pkg/operator/endpoints/info.go +++ b/pkg/operator/endpoints/info.go @@ -94,6 +94,8 @@ func getNodeInfos() ([]schema.NodeInfo, int, error) { spotPriceCache := make(map[string]float64) // instance type -> spot price for _, node := range nodes { + node := node + instanceType := node.Labels["beta.kubernetes.io/instance-type"] nodeGroupName := node.Labels["alpha.eksctl.io/nodegroup-name"] isSpot := strings.Contains(strings.ToLower(node.Labels["lifecycle"]), "spot") @@ -145,7 +147,7 @@ func getNodeInfos() ([]schema.NodeInfo, int, error) { node.NumReplicas++ } - cpu, mem, gpu, inf := k8s.TotalPodCompute(&pod.Spec) + cpu, mem, gpu, inf := k8s.TotalPodCompute(&pod.Spec) // nolint: looppointer node.ComputeAvailable.CPU.SubQty(cpu) node.ComputeAvailable.Mem.SubQty(mem) diff --git a/pkg/operator/main.go b/pkg/operator/main.go index 00ce9a8a12..ebeb35b77f 100644 --- a/pkg/operator/main.go +++ b/pkg/operator/main.go @@ -71,7 +71,7 @@ func main() { exit.Error(errors.Wrap(err, "init")) } - for _, deployment := range deployments { + for i, deployment := range deployments { apiKind := deployment.Labels["apiKind"] if userconfig.KindFromString(apiKind) == userconfig.RealtimeAPIKind || userconfig.KindFromString(apiKind) == userconfig.AsyncAPIKind { @@ -84,15 +84,15 @@ func main() { switch apiKind { case userconfig.RealtimeAPIKind.String(): - if err := realtimeapi.UpdateAutoscalerCron(&deployment, api); err != nil { + if err := realtimeapi.UpdateAutoscalerCron(&deployments[i], api); err != nil { operatorLogger.Fatal(errors.Wrap(err, "init")) } case userconfig.AsyncAPIKind.String(): - if err := asyncapi.UpdateMetricsCron(&deployment); err != nil { + if err := asyncapi.UpdateMetricsCron(&deployments[i]); err != nil { operatorLogger.Fatal(errors.Wrap(err, "init")) } - if err := asyncapi.UpdateAutoscalerCron(&deployment, *api); err != nil { + if err := asyncapi.UpdateAutoscalerCron(&deployments[i], *api); err != nil { operatorLogger.Fatal(errors.Wrap(err, "init")) } } diff --git a/pkg/operator/resources/asyncapi/api.go b/pkg/operator/resources/asyncapi/api.go index e354f3d779..a176f4630b 100644 --- a/pkg/operator/resources/asyncapi/api.go +++ b/pkg/operator/resources/asyncapi/api.go @@ -224,7 +224,7 @@ func GetAllAPIs(pods []kcore.Pod, deployments []kapps.Deployment) ([]schema.APIR realtimeAPIs := make([]schema.APIResponse, len(apis)) for i, api := range apis { - endpoint, err := operator.APIEndpoint(&api) + endpoint, err := operator.APIEndpoint(&api) // nolint: looppointer if err != nil { return nil, err } diff --git a/pkg/operator/resources/asyncapi/status.go b/pkg/operator/resources/asyncapi/status.go index 834a9fa1f4..4faf4b01ef 100644 --- a/pkg/operator/resources/asyncapi/status.go +++ b/pkg/operator/resources/asyncapi/status.go @@ -241,7 +241,7 @@ func getReplicaCounts(deployment *kapps.Deployment, pods []kcore.Pod) status.Rep if pod.Labels["apiName"] != deployment.Labels["apiName"] { continue } - addPodToReplicaCounts(&pod, deployment, &counts) + addPodToReplicaCounts(&pod, deployment, &counts) // nolint: looppointer } return counts diff --git a/pkg/operator/resources/job/batchapi/cron.go b/pkg/operator/resources/job/batchapi/cron.go index 53de0d7c6d..69a842c454 100644 --- a/pkg/operator/resources/job/batchapi/cron.go +++ b/pkg/operator/resources/job/batchapi/cron.go @@ -365,6 +365,7 @@ func checkForJobFailure(jobKey spec.JobKey, k8sJob kbatch.Job) (bool, error) { reasonFound := false pods, _ := config.K8s.ListPodsByLabel("jobID", jobKey.ID) for _, pod := range pods { + pod := pod // to avoid loop pointer bugs if k8s.WasPodOOMKilled(&pod) { jobLogger.Error("at least one worker was killed because it ran out of out of memory") return true, errors.FirstError( diff --git a/pkg/operator/resources/job/taskapi/cron.go b/pkg/operator/resources/job/taskapi/cron.go index 2ad9f97cd6..20ceaf1da0 100644 --- a/pkg/operator/resources/job/taskapi/cron.go +++ b/pkg/operator/resources/job/taskapi/cron.go @@ -204,7 +204,7 @@ func reconcileInProgressJob(jobState *job.State, jobFound bool) (status.JobCode, func checkIfJobCompleted(jobKey spec.JobKey, k8sJob kbatch.Job) error { pods, _ := config.K8s.ListPodsByLabel("jobID", jobKey.ID) for _, pod := range pods { - if k8s.WasPodOOMKilled(&pod) { + if k8s.WasPodOOMKilled(&pod) { // nolint: looppointer return errors.FirstError( job.SetWorkerOOMStatus(jobKey), deleteJobRuntimeResources(jobKey), diff --git a/pkg/operator/resources/job/worker_stats.go b/pkg/operator/resources/job/worker_stats.go index ead07b7822..9b95cfa1dc 100644 --- a/pkg/operator/resources/job/worker_stats.go +++ b/pkg/operator/resources/job/worker_stats.go @@ -35,8 +35,8 @@ func GetWorkerCountsForJob(k8sJob kbatch.Job, pods []kcore.Pod) status.WorkerCou } workerCounts := status.WorkerCounts{} - for _, pod := range pods { - addPodToWorkerCounts(&pod, &workerCounts) + for i := range pods { + addPodToWorkerCounts(&pods[i], &workerCounts) } return workerCounts diff --git a/pkg/operator/resources/realtimeapi/api.go b/pkg/operator/resources/realtimeapi/api.go index d973efef9f..e0f0ad7d55 100644 --- a/pkg/operator/resources/realtimeapi/api.go +++ b/pkg/operator/resources/realtimeapi/api.go @@ -201,7 +201,7 @@ func GetAllAPIs(pods []kcore.Pod, deployments []kapps.Deployment) ([]schema.APIR realtimeAPIs := make([]schema.APIResponse, len(apis)) for i, api := range apis { - endpoint, err := operator.APIEndpoint(&api) + endpoint, err := operator.APIEndpoint(&api) // nolint: looppointer if err != nil { return nil, err } diff --git a/pkg/operator/resources/realtimeapi/status.go b/pkg/operator/resources/realtimeapi/status.go index ce884a38f2..dabc45a26e 100644 --- a/pkg/operator/resources/realtimeapi/status.go +++ b/pkg/operator/resources/realtimeapi/status.go @@ -62,12 +62,12 @@ func GetStatus(apiName string) (*status.Status, error) { func GetAllStatuses(deployments []kapps.Deployment, pods []kcore.Pod) ([]status.Status, error) { statuses := make([]status.Status, len(deployments)) - for i, deployment := range deployments { - status, err := apiStatus(&deployment, pods) + for i := range deployments { + st, err := apiStatus(&deployments[i], pods) if err != nil { return nil, err } - statuses[i] = *status + statuses[i] = *st } sort.Slice(statuses, func(i, j int) bool { @@ -96,11 +96,11 @@ func getReplicaCounts(deployment *kapps.Deployment, pods []kcore.Pod) status.Rep counts := status.ReplicaCounts{} counts.Requested = *deployment.Spec.Replicas - for _, pod := range pods { + for i, pod := range pods { if pod.Labels["apiName"] != deployment.Labels["apiName"] { continue } - addPodToReplicaCounts(&pod, deployment, &counts) + addPodToReplicaCounts(&pods[i], deployment, &counts) } return counts diff --git a/pkg/operator/resources/trafficsplitter/api.go b/pkg/operator/resources/trafficsplitter/api.go index 801431b22d..5f69d8ed26 100644 --- a/pkg/operator/resources/trafficsplitter/api.go +++ b/pkg/operator/resources/trafficsplitter/api.go @@ -139,7 +139,7 @@ func GetAllAPIs(virtualServices []istioclientnetworking.VirtualService) ([]schem } for _, trafficSplitter := range apis { - endpoint, err := operator.APIEndpoint(&trafficSplitter) + endpoint, err := operator.APIEndpoint(&trafficSplitter) // nolint: looppointer if err != nil { return nil, err } diff --git a/pkg/operator/resources/validations.go b/pkg/operator/resources/validations.go index 7e62417c2a..d9732e9713 100644 --- a/pkg/operator/resources/validations.go +++ b/pkg/operator/resources/validations.go @@ -236,12 +236,12 @@ func awsManagedValidateK8sCompute(compute *userconfig.Compute, maxMemMap map[str func validateEndpointCollisions(api *userconfig.API, virtualServices []istioclientnetworking.VirtualService) error { for _, virtualService := range virtualServices { - gateways := k8s.ExtractVirtualServiceGateways(&virtualService) + gateways := k8s.ExtractVirtualServiceGateways(&virtualService) // nolint: looppointer if !gateways.Has("apis-gateway") { continue } - endpoints := k8s.ExtractVirtualServiceEndpoints(&virtualService) + endpoints := k8s.ExtractVirtualServiceEndpoints(&virtualService) // nolint: looppointer for endpoint := range endpoints { if s.EnsureSuffix(endpoint, "/") == s.EnsureSuffix(*api.Networking.Endpoint, "/") && virtualService.Labels["apiName"] != api.Name { return errors.Wrap(spec.ErrorDuplicateEndpoint(virtualService.Labels["apiName"]), userconfig.NetworkingKey, userconfig.EndpointKey, endpoint) From b2b3857b44cccfbdd15ceb9a30995b174cd49890 Mon Sep 17 00:00:00 2001 From: Miguel Varela Ramos Date: Fri, 19 Mar 2021 13:05:54 +0100 Subject: [PATCH 3/7] Add looppointer to lint.sh and tools --- Makefile | 1 + build/lint.sh | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/Makefile b/Makefile index 6db44937ee..5e9e51203e 100644 --- a/Makefile +++ b/Makefile @@ -225,6 +225,7 @@ registry-clean-aws: tools: @go get -u -v golang.org/x/lint/golint + @go get -u -v github.com/kyoh86/looppointer/cmd/looppointer @go get -u -v github.com/VojtechVitek/rerun/cmd/rerun @go get -u -v github.com/go-delve/delve/cmd/dlv @if [[ "$$OSTYPE" == "darwin"* ]]; then brew install parallel; elif [[ "$$OSTYPE" == "linux"* ]]; then sudo apt-get install -y parallel; else echo "your operating system is not supported"; fi diff --git a/build/lint.sh b/build/lint.sh index 31cc7962fa..6d5a2bc3dd 100755 --- a/build/lint.sh +++ b/build/lint.sh @@ -34,6 +34,11 @@ if ! command -v golint >/dev/null 2>&1; then exit 1 fi +if ! command -v looppointer >/dev/null 2>&1; then + echo "looppointer must be installed" + exit 1 +fi + if ! command -v gofmt >/dev/null 2>&1; then echo "gofmt must be installed" exit 1 @@ -52,6 +57,12 @@ if [[ $output ]]; then exit 1 fi +output=$(looppointer "$ROOT/..." || true) +if [[ $output ]]; then + echo "$output" + exit 1 +fi + output=$(gofmt -s -l "$ROOT") if [[ $output ]]; then echo "go files not properly formatted:" From 86421faa5fc0028cf7c0a53af1de8cce59e4556b Mon Sep 17 00:00:00 2001 From: Miguel Varela Ramos Date: Fri, 19 Mar 2021 13:15:21 +0100 Subject: [PATCH 4/7] Fix looppointer ignores --- pkg/operator/endpoints/info.go | 2 +- pkg/operator/resources/asyncapi/api.go | 2 +- pkg/operator/resources/asyncapi/status.go | 2 +- pkg/operator/resources/job/taskapi/cron.go | 2 +- pkg/operator/resources/realtimeapi/api.go | 2 +- pkg/operator/resources/trafficsplitter/api.go | 2 +- pkg/operator/resources/validations.go | 4 ++-- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/operator/endpoints/info.go b/pkg/operator/endpoints/info.go index 3d64d2778e..cd6329d312 100644 --- a/pkg/operator/endpoints/info.go +++ b/pkg/operator/endpoints/info.go @@ -147,7 +147,7 @@ func getNodeInfos() ([]schema.NodeInfo, int, error) { node.NumReplicas++ } - cpu, mem, gpu, inf := k8s.TotalPodCompute(&pod.Spec) // nolint: looppointer + cpu, mem, gpu, inf := k8s.TotalPodCompute(&pod.Spec) // nolint:looppointer node.ComputeAvailable.CPU.SubQty(cpu) node.ComputeAvailable.Mem.SubQty(mem) diff --git a/pkg/operator/resources/asyncapi/api.go b/pkg/operator/resources/asyncapi/api.go index a176f4630b..f689bda160 100644 --- a/pkg/operator/resources/asyncapi/api.go +++ b/pkg/operator/resources/asyncapi/api.go @@ -224,7 +224,7 @@ func GetAllAPIs(pods []kcore.Pod, deployments []kapps.Deployment) ([]schema.APIR realtimeAPIs := make([]schema.APIResponse, len(apis)) for i, api := range apis { - endpoint, err := operator.APIEndpoint(&api) // nolint: looppointer + endpoint, err := operator.APIEndpoint(&api) // nolint:looppointer if err != nil { return nil, err } diff --git a/pkg/operator/resources/asyncapi/status.go b/pkg/operator/resources/asyncapi/status.go index 4faf4b01ef..a42dd84a50 100644 --- a/pkg/operator/resources/asyncapi/status.go +++ b/pkg/operator/resources/asyncapi/status.go @@ -241,7 +241,7 @@ func getReplicaCounts(deployment *kapps.Deployment, pods []kcore.Pod) status.Rep if pod.Labels["apiName"] != deployment.Labels["apiName"] { continue } - addPodToReplicaCounts(&pod, deployment, &counts) // nolint: looppointer + addPodToReplicaCounts(&pod, deployment, &counts) // nolint:looppointer } return counts diff --git a/pkg/operator/resources/job/taskapi/cron.go b/pkg/operator/resources/job/taskapi/cron.go index 20ceaf1da0..34cea20929 100644 --- a/pkg/operator/resources/job/taskapi/cron.go +++ b/pkg/operator/resources/job/taskapi/cron.go @@ -204,7 +204,7 @@ func reconcileInProgressJob(jobState *job.State, jobFound bool) (status.JobCode, func checkIfJobCompleted(jobKey spec.JobKey, k8sJob kbatch.Job) error { pods, _ := config.K8s.ListPodsByLabel("jobID", jobKey.ID) for _, pod := range pods { - if k8s.WasPodOOMKilled(&pod) { // nolint: looppointer + if k8s.WasPodOOMKilled(&pod) { // nolint:looppointer return errors.FirstError( job.SetWorkerOOMStatus(jobKey), deleteJobRuntimeResources(jobKey), diff --git a/pkg/operator/resources/realtimeapi/api.go b/pkg/operator/resources/realtimeapi/api.go index e0f0ad7d55..6c1af8c6a2 100644 --- a/pkg/operator/resources/realtimeapi/api.go +++ b/pkg/operator/resources/realtimeapi/api.go @@ -201,7 +201,7 @@ func GetAllAPIs(pods []kcore.Pod, deployments []kapps.Deployment) ([]schema.APIR realtimeAPIs := make([]schema.APIResponse, len(apis)) for i, api := range apis { - endpoint, err := operator.APIEndpoint(&api) // nolint: looppointer + endpoint, err := operator.APIEndpoint(&api) // nolint:looppointer if err != nil { return nil, err } diff --git a/pkg/operator/resources/trafficsplitter/api.go b/pkg/operator/resources/trafficsplitter/api.go index 5f69d8ed26..d8729a14ff 100644 --- a/pkg/operator/resources/trafficsplitter/api.go +++ b/pkg/operator/resources/trafficsplitter/api.go @@ -139,7 +139,7 @@ func GetAllAPIs(virtualServices []istioclientnetworking.VirtualService) ([]schem } for _, trafficSplitter := range apis { - endpoint, err := operator.APIEndpoint(&trafficSplitter) // nolint: looppointer + endpoint, err := operator.APIEndpoint(&trafficSplitter) // nolint:looppointer if err != nil { return nil, err } diff --git a/pkg/operator/resources/validations.go b/pkg/operator/resources/validations.go index d9732e9713..0a9037b6ba 100644 --- a/pkg/operator/resources/validations.go +++ b/pkg/operator/resources/validations.go @@ -236,12 +236,12 @@ func awsManagedValidateK8sCompute(compute *userconfig.Compute, maxMemMap map[str func validateEndpointCollisions(api *userconfig.API, virtualServices []istioclientnetworking.VirtualService) error { for _, virtualService := range virtualServices { - gateways := k8s.ExtractVirtualServiceGateways(&virtualService) // nolint: looppointer + gateways := k8s.ExtractVirtualServiceGateways(&virtualService) // nolint:looppointer if !gateways.Has("apis-gateway") { continue } - endpoints := k8s.ExtractVirtualServiceEndpoints(&virtualService) // nolint: looppointer + endpoints := k8s.ExtractVirtualServiceEndpoints(&virtualService) // nolint:looppointer for endpoint := range endpoints { if s.EnsureSuffix(endpoint, "/") == s.EnsureSuffix(*api.Networking.Endpoint, "/") && virtualService.Labels["apiName"] != api.Name { return errors.Wrap(spec.ErrorDuplicateEndpoint(virtualService.Labels["apiName"]), userconfig.NetworkingKey, userconfig.EndpointKey, endpoint) From e2990a317d324a55dd0a42e3c5da5c4b92ba7fc7 Mon Sep 17 00:00:00 2001 From: Miguel Varela Ramos Date: Fri, 19 Mar 2021 13:15:55 +0100 Subject: [PATCH 5/7] Update circleci config.yml --- .circleci/config.yml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index a34fffff2a..41c16636f7 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -98,8 +98,12 @@ jobs: - checkout - setup_remote_docker - install-go - - run: go get -u -v golang.org/x/lint/golint - - run: sudo pip install black aiohttp + - run: + name: Install Linting Tools + command: | + go get -u -v golang.org/x/lint/golint + go get -u -v github.com/kyoh86/looppointer/cmd/looppointer + sudo pip install black aiohttp - run: name: Lint command: make lint From 062236a527aea414f963b5b26ca7da38798627b9 Mon Sep 17 00:00:00 2001 From: vishal Date: Fri, 19 Mar 2021 10:20:35 -0400 Subject: [PATCH 6/7] Propagate non-zero exit code --- build/lint.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/lint.sh b/build/lint.sh index 6d5a2bc3dd..bbdc70bc13 100755 --- a/build/lint.sh +++ b/build/lint.sh @@ -57,7 +57,7 @@ if [[ $output ]]; then exit 1 fi -output=$(looppointer "$ROOT/..." || true) +output=$(looppointer "$ROOT/...") if [[ $output ]]; then echo "$output" exit 1 From bbe649741072caf7d8e37ecc82ef757a74898f8b Mon Sep 17 00:00:00 2001 From: vishal Date: Fri, 19 Mar 2021 11:36:21 -0400 Subject: [PATCH 7/7] Remove nolint:looppointer labels --- pkg/lib/k8s/pod.go | 5 +++-- pkg/operator/endpoints/info.go | 10 ++++++---- pkg/operator/main.go | 9 +++++---- pkg/operator/resources/asyncapi/api.go | 5 +++-- pkg/operator/resources/asyncapi/status.go | 6 ++++-- pkg/operator/resources/job/batchapi/cron.go | 4 ++-- pkg/operator/resources/job/taskapi/cron.go | 4 ++-- pkg/operator/resources/realtimeapi/api.go | 5 +++-- pkg/operator/resources/realtimeapi/status.go | 3 ++- pkg/operator/resources/trafficsplitter/api.go | 5 +++-- pkg/operator/resources/validations.go | 7 ++++--- 11 files changed, 37 insertions(+), 26 deletions(-) diff --git a/pkg/lib/k8s/pod.go b/pkg/lib/k8s/pod.go index 084471adb7..ca3d30b078 100644 --- a/pkg/lib/k8s/pod.go +++ b/pkg/lib/k8s/pod.go @@ -134,12 +134,13 @@ func IsPodReady(pod *kcore.Pod) bool { } func GetPodReadyTime(pod *kcore.Pod) *time.Time { - for _, condition := range pod.Status.Conditions { + for i := range pod.Status.Conditions { + condition := pod.Status.Conditions[i] + if condition.Type == "Ready" && condition.Status == kcore.ConditionTrue { if condition.LastTransitionTime.Time.IsZero() { return nil } - condition := condition return &condition.LastTransitionTime.Time } } diff --git a/pkg/operator/endpoints/info.go b/pkg/operator/endpoints/info.go index cd6329d312..4463924867 100644 --- a/pkg/operator/endpoints/info.go +++ b/pkg/operator/endpoints/info.go @@ -93,8 +93,8 @@ func getNodeInfos() ([]schema.NodeInfo, int, error) { nodeInfoMap := make(map[string]*schema.NodeInfo, len(nodes)) // node name -> info spotPriceCache := make(map[string]float64) // instance type -> spot price - for _, node := range nodes { - node := node + for i := range nodes { + node := nodes[i] instanceType := node.Labels["beta.kubernetes.io/instance-type"] nodeGroupName := node.Labels["alpha.eksctl.io/nodegroup-name"] @@ -130,7 +130,9 @@ func getNodeInfos() ([]schema.NodeInfo, int, error) { var numPendingReplicas int - for _, pod := range pods { + for i := range pods { + pod := pods[i] + _, isAPIPod := pod.Labels["apiName"] if pod.Spec.NodeName == "" && isAPIPod { @@ -147,7 +149,7 @@ func getNodeInfos() ([]schema.NodeInfo, int, error) { node.NumReplicas++ } - cpu, mem, gpu, inf := k8s.TotalPodCompute(&pod.Spec) // nolint:looppointer + cpu, mem, gpu, inf := k8s.TotalPodCompute(&pod.Spec) node.ComputeAvailable.CPU.SubQty(cpu) node.ComputeAvailable.Mem.SubQty(mem) diff --git a/pkg/operator/main.go b/pkg/operator/main.go index ebeb35b77f..94ad2b7e57 100644 --- a/pkg/operator/main.go +++ b/pkg/operator/main.go @@ -71,7 +71,8 @@ func main() { exit.Error(errors.Wrap(err, "init")) } - for i, deployment := range deployments { + for i := range deployments { + deployment := deployments[i] apiKind := deployment.Labels["apiKind"] if userconfig.KindFromString(apiKind) == userconfig.RealtimeAPIKind || userconfig.KindFromString(apiKind) == userconfig.AsyncAPIKind { @@ -84,15 +85,15 @@ func main() { switch apiKind { case userconfig.RealtimeAPIKind.String(): - if err := realtimeapi.UpdateAutoscalerCron(&deployments[i], api); err != nil { + if err := realtimeapi.UpdateAutoscalerCron(&deployment, api); err != nil { operatorLogger.Fatal(errors.Wrap(err, "init")) } case userconfig.AsyncAPIKind.String(): - if err := asyncapi.UpdateMetricsCron(&deployments[i]); err != nil { + if err := asyncapi.UpdateMetricsCron(&deployment); err != nil { operatorLogger.Fatal(errors.Wrap(err, "init")) } - if err := asyncapi.UpdateAutoscalerCron(&deployments[i], *api); err != nil { + if err := asyncapi.UpdateAutoscalerCron(&deployment, *api); err != nil { operatorLogger.Fatal(errors.Wrap(err, "init")) } } diff --git a/pkg/operator/resources/asyncapi/api.go b/pkg/operator/resources/asyncapi/api.go index f689bda160..bb61885b13 100644 --- a/pkg/operator/resources/asyncapi/api.go +++ b/pkg/operator/resources/asyncapi/api.go @@ -223,8 +223,9 @@ func GetAllAPIs(pods []kcore.Pod, deployments []kapps.Deployment) ([]schema.APIR realtimeAPIs := make([]schema.APIResponse, len(apis)) - for i, api := range apis { - endpoint, err := operator.APIEndpoint(&api) // nolint:looppointer + for i := range apis { + api := apis[i] + endpoint, err := operator.APIEndpoint(&api) if err != nil { return nil, err } diff --git a/pkg/operator/resources/asyncapi/status.go b/pkg/operator/resources/asyncapi/status.go index a42dd84a50..56fab0182b 100644 --- a/pkg/operator/resources/asyncapi/status.go +++ b/pkg/operator/resources/asyncapi/status.go @@ -237,11 +237,13 @@ func getReplicaCounts(deployment *kapps.Deployment, pods []kcore.Pod) status.Rep counts := status.ReplicaCounts{} counts.Requested = *deployment.Spec.Replicas - for _, pod := range pods { + for i := range pods { + pod := pods[i] + if pod.Labels["apiName"] != deployment.Labels["apiName"] { continue } - addPodToReplicaCounts(&pod, deployment, &counts) // nolint:looppointer + addPodToReplicaCounts(&pod, deployment, &counts) } return counts diff --git a/pkg/operator/resources/job/batchapi/cron.go b/pkg/operator/resources/job/batchapi/cron.go index 69a842c454..b9fea7b535 100644 --- a/pkg/operator/resources/job/batchapi/cron.go +++ b/pkg/operator/resources/job/batchapi/cron.go @@ -364,8 +364,8 @@ func checkForJobFailure(jobKey spec.JobKey, k8sJob kbatch.Job) (bool, error) { reasonFound := false pods, _ := config.K8s.ListPodsByLabel("jobID", jobKey.ID) - for _, pod := range pods { - pod := pod // to avoid loop pointer bugs + for i := range pods { + pod := pods[i] // to avoid loop pointer bugs if k8s.WasPodOOMKilled(&pod) { jobLogger.Error("at least one worker was killed because it ran out of out of memory") return true, errors.FirstError( diff --git a/pkg/operator/resources/job/taskapi/cron.go b/pkg/operator/resources/job/taskapi/cron.go index 34cea20929..146e6db239 100644 --- a/pkg/operator/resources/job/taskapi/cron.go +++ b/pkg/operator/resources/job/taskapi/cron.go @@ -203,8 +203,8 @@ func reconcileInProgressJob(jobState *job.State, jobFound bool) (status.JobCode, func checkIfJobCompleted(jobKey spec.JobKey, k8sJob kbatch.Job) error { pods, _ := config.K8s.ListPodsByLabel("jobID", jobKey.ID) - for _, pod := range pods { - if k8s.WasPodOOMKilled(&pod) { // nolint:looppointer + for i := range pods { + if k8s.WasPodOOMKilled(&pods[i]) { return errors.FirstError( job.SetWorkerOOMStatus(jobKey), deleteJobRuntimeResources(jobKey), diff --git a/pkg/operator/resources/realtimeapi/api.go b/pkg/operator/resources/realtimeapi/api.go index 6c1af8c6a2..56c00414a0 100644 --- a/pkg/operator/resources/realtimeapi/api.go +++ b/pkg/operator/resources/realtimeapi/api.go @@ -200,8 +200,9 @@ func GetAllAPIs(pods []kcore.Pod, deployments []kapps.Deployment) ([]schema.APIR realtimeAPIs := make([]schema.APIResponse, len(apis)) - for i, api := range apis { - endpoint, err := operator.APIEndpoint(&api) // nolint:looppointer + for i := range apis { + api := apis[i] + endpoint, err := operator.APIEndpoint(&api) if err != nil { return nil, err } diff --git a/pkg/operator/resources/realtimeapi/status.go b/pkg/operator/resources/realtimeapi/status.go index dabc45a26e..9b38395e7a 100644 --- a/pkg/operator/resources/realtimeapi/status.go +++ b/pkg/operator/resources/realtimeapi/status.go @@ -96,7 +96,8 @@ func getReplicaCounts(deployment *kapps.Deployment, pods []kcore.Pod) status.Rep counts := status.ReplicaCounts{} counts.Requested = *deployment.Spec.Replicas - for i, pod := range pods { + for i := range pods { + pod := pods[i] if pod.Labels["apiName"] != deployment.Labels["apiName"] { continue } diff --git a/pkg/operator/resources/trafficsplitter/api.go b/pkg/operator/resources/trafficsplitter/api.go index d8729a14ff..c093308439 100644 --- a/pkg/operator/resources/trafficsplitter/api.go +++ b/pkg/operator/resources/trafficsplitter/api.go @@ -138,8 +138,9 @@ func GetAllAPIs(virtualServices []istioclientnetworking.VirtualService) ([]schem return nil, err } - for _, trafficSplitter := range apis { - endpoint, err := operator.APIEndpoint(&trafficSplitter) // nolint:looppointer + for i := range apis { + trafficSplitter := apis[i] + endpoint, err := operator.APIEndpoint(&trafficSplitter) if err != nil { return nil, err } diff --git a/pkg/operator/resources/validations.go b/pkg/operator/resources/validations.go index 0a9037b6ba..f52555179a 100644 --- a/pkg/operator/resources/validations.go +++ b/pkg/operator/resources/validations.go @@ -235,13 +235,14 @@ func awsManagedValidateK8sCompute(compute *userconfig.Compute, maxMemMap map[str } func validateEndpointCollisions(api *userconfig.API, virtualServices []istioclientnetworking.VirtualService) error { - for _, virtualService := range virtualServices { - gateways := k8s.ExtractVirtualServiceGateways(&virtualService) // nolint:looppointer + for i := range virtualServices { + virtualService := virtualServices[i] + gateways := k8s.ExtractVirtualServiceGateways(&virtualService) if !gateways.Has("apis-gateway") { continue } - endpoints := k8s.ExtractVirtualServiceEndpoints(&virtualService) // nolint:looppointer + endpoints := k8s.ExtractVirtualServiceEndpoints(&virtualService) for endpoint := range endpoints { if s.EnsureSuffix(endpoint, "/") == s.EnsureSuffix(*api.Networking.Endpoint, "/") && virtualService.Labels["apiName"] != api.Name { return errors.Wrap(spec.ErrorDuplicateEndpoint(virtualService.Labels["apiName"]), userconfig.NetworkingKey, userconfig.EndpointKey, endpoint)