github-vet / github-vet/rangeloop-pointer-findings

john-deng/hicli-vendor: github.com/openshift/origin/test/common/build/controllers.go; 136 LoC

Open
#8,346 0 comments 0 reactions 0 assignees View on GitHub
fresh large
Dominant language
No language data
Stars
0
Forks
0
PR merge metrics
PR metrics pending

Description

Found a possible issue in [john-deng/hicli-vendor](https://www.github.com/john-deng/hicli-vendor) at [github.com/openshift/origin/test/common/build/controllers.go](https://github.com/john-deng/hicli-vendor/blob/bba4ecd772d120ba795d0d4d3f70db0b0bd0c508/github.com/openshift/origin/test/common/build/controllers.go#L210-L345)

Below is the message reported by the analyzer for this snippet of code. Beware that the analyzer only reports the first
issue it finds, so please do not limit your consideration to the contents of the below message.

> range-loop variable test used in defer or goroutine at line 230

[Click here to see the code in its original context.](https://github.com/john-deng/hicli-vendor/blob/bba4ecd772d120ba795d0d4d3f70db0b0bd0c508/github.com/openshift/origin/test/common/build/controllers.go#L210-L345)

Click here to show the 136 line(s) of Go which triggered the analyzer.

```go
for _, test := range tests {
// Setup communications channels
podReadyChan := make(chan *kapi.Pod) // Will receive a value when a build pod is ready
errChan := make(chan error) // Will receive a value when an error occurs

// Create a build
b, err := buildClient.Builds(ns).Create(mockBuild())
if err != nil {
t.Fatal(err)
}

// Watch build pod for transition to pending
podWatch, err := kClient.Core().Pods(ns).Watch(metav1.ListOptions{FieldSelector: fields.OneTermEqualSelector("metadata.name", buildapi.GetBuildPodName(b)).String()})
if err != nil {
t.Fatal(err)
}
go func() {
for e := range podWatch.ResultChan() {
pod, ok := e.Object.(*kapi.Pod)
if !ok {
t.Fatalf("%s: unexpected object received: %#v\n", test.Name, e.Object)
}
glog.Infof("pod watch event received for pod %s/%s: %v, pod phase: %v", pod.Namespace, pod.Name, e.Type, pod.Status.Phase)
if pod.Status.Phase == kapi.PodPending {
podReadyChan <- pod
break
}
}
}()

var pod *kapi.Pod
select {
case pod = <-podReadyChan:
if pod.Status.Phase != kapi.PodPending {
t.Errorf("Got wrong pod phase: %s", pod.Status.Phase)
podWatch.Stop()
continue
}

case <-time.After(BuildControllersWatchTimeout):
t.Errorf("Timed out waiting for build pod to be ready")
podWatch.Stop()
continue
}
podWatch.Stop()

for _, state := range test.States {
if err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
// Update pod state and verify that corresponding build state happens accordingly
pod, err := kClient.Core().Pods(ns).Get(pod.Name, metav1.GetOptions{})
if err != nil {
return err
}
if pod.Status.Phase == state.PodPhase {
return fmt.Errorf("another client altered the pod phase to %s: %#v", state.PodPhase, pod)
}
pod.Status.Phase = state.PodPhase
if pod.Status.Phase == kapi.PodSucceeded {
pod.Status.ContainerStatuses = []kapi.ContainerStatus{
{
Name: "container",
State: kapi.ContainerState{
Terminated: &kapi.ContainerStateTerminated{
ExitCode: 0,
},
},
},
}
}
_, err = kClient.Core().Pods(ns).UpdateStatus(pod)
return err
}); err != nil {
t.Fatal(err)
}

shouldContinue := func() bool {
buildWatch, err := buildClient.Builds(ns).Watch(metav1.ListOptions{FieldSelector: fields.OneTermEqualSelector("metadata.name", b.Name).String(), ResourceVersion: b.ResourceVersion})
if err != nil {
t.Fatal(err)
}
defer buildWatch.Stop()

stateReached := make(chan struct{})
go func() {
done := false
for e := range buildWatch.ResultChan() {
var ok bool
b, ok = e.Object.(*buildapi.Build)
if !ok {
errChan <- fmt.Errorf("unexpected object received: %#v", e.Object)
return
}
glog.Infof("build watch event received for build %s/%s: %v, build phase: %v", b.Namespace, b.Name, e.Type, b.Status.Phase)
if e.Type != watchapi.Modified {
errChan <- fmt.Errorf("unexpected event received: %s, object: %#v", e.Type, e.Object)
return
}
if done && b.Status.Phase != state.BuildPhase {
errChan <- fmt.Errorf("build %s/%s transitioned to new state (%s) after reaching desired state", b.Namespace, b.Name, b.Status.Phase)
return
}
if b.Status.Phase == state.BuildPhase {
done = true
stateReached <- struct{}{}
}
}
}()

select {
case err := <-errChan:
t.Errorf("%s: Error %v", test.Name, err)
return false
case <-time.After(BuildControllerTestTransitionTimeout):
t.Errorf("%s: Timed out waiting for build %s/%s to reach state %s. Current state: %s", test.Name, b.Namespace, b.Name, state.BuildPhase, b.Status.Phase)
return false
case <-stateReached:
glog.Infof("%s: build %s/%s reached desired state of %s", test.Name, b.Namespace, b.Name, state.BuildPhase)
}

// After state is reached, continue waiting some time to check for unexpected transitions
select {
case err := <-errChan:
t.Errorf("%s: Error %v", test.Name, err)
return false

case <-time.After(BuildControllerTestWait):
// After waiting for a set time, if no other state is reached, continue to wait for next state transition
return true
}
}()

if !shouldContinue {
break
}
}
}

```

Leave a reaction on this issue to contribute to the project by classifying this instance as a **Bug** :-1:, **Mitigated** :+1:, or **Desirable Behavior** :rocket:
See the descriptions of the classifications [here](https://github.com/github-vet/rangeclosure-findings#how-can-i-help) for more information.

commit ID: bba4ecd772d120ba795d0d4d3f70db0b0bd0c508

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.