Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add golint to the linters #1682

Merged
merged 2 commits into from
Dec 5, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ run:
- e2e
skip-dirs:
- vendor
- pkg/client/clientset/versioned/fake
- pkg/client/clientset/(.*)/fake
linters-settings:
errcheck:
exclude: .errcheck.txt
Expand All @@ -14,3 +14,4 @@ linters:
- goimports
- gosec
- gocritic
- golint
12 changes: 6 additions & 6 deletions pkg/artifacts/artifacts_storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,13 +189,13 @@ func ConfigMapNeedsPVC(configMap *corev1.ConfigMap, err error, logger *zap.Sugar
logger.Warn("the configmap has no data")
return true, nil
}
if location, ok := configMap.Data[BucketLocationKey]; !ok {
location, ok := configMap.Data[BucketLocationKey]
if !ok {
return true, nil
}
logger.Warnf("the configmap key %q is empty", BucketLocationKey)
if strings.TrimSpace(location) == "" {
return true, nil
} else {
logger.Warnf("the configmap key %q is empty", BucketLocationKey)
if strings.TrimSpace(location) == "" {
return true, nil
}
}
return false, nil
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/credentials/gitcreds/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (dc *sshGitConfig) Set(value string) error {
secretName := parts[0]
url := parts[1]

e, err := newSshEntry(url, secretName)
e, err := newSSHEntry(url, secretName)
if err != nil {
return err
}
Expand Down Expand Up @@ -143,7 +143,7 @@ func (be *sshEntry) Write(sshDir string) error {
return ioutil.WriteFile(be.path(sshDir), []byte(be.privateKey), 0600)
}

func newSshEntry(u, secretName string) (*sshEntry, error) {
func newSSHEntry(u, secretName string) (*sshEntry, error) {
secretPath := credentials.VolumeName(secretName)

pk, err := ioutil.ReadFile(filepath.Join(secretPath, corev1.SSHAuthPrivateKey))
Expand Down
2 changes: 1 addition & 1 deletion pkg/pod/entrypoint_lookup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func (f fakeCache) Get(ref name.Reference, _, _ string) (v1.Image, error) {
return nil, fmt.Errorf("image %q not found", ref)
}
if d.seen {
return nil, fmt.Errorf("image %q was already looked up!", ref)
return nil, fmt.Errorf("image %q was already looked up", ref)
}
return d.img, nil
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/reconciler/pipelinerun/pipelinerun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,12 @@ func getRunName(pr *v1alpha1.PipelineRun) string {

// getPipelineRunController returns an instance of the PipelineRun controller/reconciler that has been seeded with
// d, where d represents the state of the system (existing resources) needed for the test.
func getPipelineRunController(t *testing.T, d test.Data) (test.TestAssets, func()) {
func getPipelineRunController(t *testing.T, d test.Data) (test.Assets, func()) {
ctx, _ := ttesting.SetupFakeContext(t)
c, _ := test.SeedTestData(t, ctx, d)
configMapWatcher := configmap.NewInformedWatcher(c.Kube, system.GetNamespace())
ctx, cancel := context.WithCancel(ctx)
return test.TestAssets{
return test.Assets{
Controller: NewController(images)(ctx, configMapWatcher),
Clients: c,
}, cancel
Expand Down
24 changes: 12 additions & 12 deletions pkg/reconciler/taskrun/taskrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ const (
taskNameLabelKey = pipeline.GroupName + pipeline.TaskLabelKey
taskRunNameLabelKey = pipeline.GroupName + pipeline.TaskRunLabelKey
workspaceDir = "/workspace"
currentApiVersion = "tekton.dev/v1alpha1"
currentAPIVersion = "tekton.dev/v1alpha1"
)

var (
Expand Down Expand Up @@ -237,7 +237,7 @@ func getRunName(tr *v1alpha1.TaskRun) string {

// getTaskRunController returns an instance of the TaskRun controller/reconciler that has been seeded with
// d, where d represents the state of the system (existing resources) needed for the test.
func getTaskRunController(t *testing.T, d test.Data) (test.TestAssets, func()) {
func getTaskRunController(t *testing.T, d test.Data) (test.Assets, func()) {
ctx, _ := ttesting.SetupFakeContext(t)
ctx, cancel := context.WithCancel(ctx)
cloudEventClientBehaviour := cloudevent.FakeClientBehaviour{
Expand All @@ -246,7 +246,7 @@ func getTaskRunController(t *testing.T, d test.Data) (test.TestAssets, func()) {
ctx = cloudevent.WithClient(ctx, &cloudEventClientBehaviour)
c, _ := test.SeedTestData(t, ctx, d)
configMapWatcher := configmap.NewInformedWatcher(c.Kube, system.GetNamespace())
return test.TestAssets{
return test.Assets{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yaaaassss

Controller: NewController(images)(ctx, configMapWatcher),
Clients: c,
}, cancel
Expand Down Expand Up @@ -286,7 +286,7 @@ func TestReconcile_ExplicitDefaultSA(t *testing.T) {
tb.PodLabel(taskRunNameLabelKey, "test-taskrun-run-success"),
tb.PodLabel(podconvert.ManagedByLabelKey, podconvert.ManagedByLabelValue),
tb.PodOwnerReference("TaskRun", "test-taskrun-run-success",
tb.OwnerReferenceAPIVersion(currentApiVersion)),
tb.OwnerReferenceAPIVersion(currentAPIVersion)),
tb.PodSpec(
tb.PodServiceAccountName(defaultSAName),
tb.PodVolumes(workspaceVolume, homeVolume, toolsVolume, downwardVolume),
Expand Down Expand Up @@ -321,7 +321,7 @@ func TestReconcile_ExplicitDefaultSA(t *testing.T) {
tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-sa-run-success"),
tb.PodLabel(podconvert.ManagedByLabelKey, podconvert.ManagedByLabelValue),
tb.PodOwnerReference("TaskRun", "test-taskrun-with-sa-run-success",
tb.OwnerReferenceAPIVersion(currentApiVersion)),
tb.OwnerReferenceAPIVersion(currentAPIVersion)),
tb.PodSpec(
tb.PodServiceAccountName("test-sa"),
tb.PodVolumes(workspaceVolume, homeVolume, toolsVolume, downwardVolume),
Expand Down Expand Up @@ -540,7 +540,7 @@ func TestReconcile(t *testing.T) {
tb.PodLabel(taskRunNameLabelKey, "test-taskrun-run-success"),
tb.PodLabel(podconvert.ManagedByLabelKey, podconvert.ManagedByLabelValue),
tb.PodOwnerReference("TaskRun", "test-taskrun-run-success",
tb.OwnerReferenceAPIVersion(currentApiVersion)),
tb.OwnerReferenceAPIVersion(currentAPIVersion)),
tb.PodSpec(
tb.PodVolumes(workspaceVolume, homeVolume, toolsVolume, downwardVolume),
tb.PodRestartPolicy(corev1.RestartPolicyNever),
Expand Down Expand Up @@ -574,7 +574,7 @@ func TestReconcile(t *testing.T) {
tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-sa-run-success"),
tb.PodLabel(podconvert.ManagedByLabelKey, podconvert.ManagedByLabelValue),
tb.PodOwnerReference("TaskRun", "test-taskrun-with-sa-run-success",
tb.OwnerReferenceAPIVersion(currentApiVersion)),
tb.OwnerReferenceAPIVersion(currentAPIVersion)),
tb.PodSpec(
tb.PodServiceAccountName("test-sa"),
tb.PodVolumes(workspaceVolume, homeVolume, toolsVolume, downwardVolume),
Expand Down Expand Up @@ -609,7 +609,7 @@ func TestReconcile(t *testing.T) {
tb.PodLabel(taskRunNameLabelKey, "test-taskrun-substitution"),
tb.PodLabel(podconvert.ManagedByLabelKey, podconvert.ManagedByLabelValue),
tb.PodOwnerReference("TaskRun", "test-taskrun-substitution",
tb.OwnerReferenceAPIVersion(currentApiVersion)),
tb.OwnerReferenceAPIVersion(currentAPIVersion)),
tb.PodSpec(
tb.PodVolumes(
workspaceVolume, homeVolume, toolsVolume, downwardVolume,
Expand Down Expand Up @@ -683,7 +683,7 @@ func TestReconcile(t *testing.T) {
tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-taskspec"),
tb.PodLabel(podconvert.ManagedByLabelKey, podconvert.ManagedByLabelValue),
tb.PodOwnerReference("TaskRun", "test-taskrun-with-taskspec",
tb.OwnerReferenceAPIVersion(currentApiVersion)),
tb.OwnerReferenceAPIVersion(currentAPIVersion)),
tb.PodSpec(
tb.PodVolumes(workspaceVolume, homeVolume, toolsVolume, downwardVolume),
tb.PodRestartPolicy(corev1.RestartPolicyNever),
Expand Down Expand Up @@ -735,7 +735,7 @@ func TestReconcile(t *testing.T) {
tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-cluster-task"),
tb.PodLabel(podconvert.ManagedByLabelKey, podconvert.ManagedByLabelValue),
tb.PodOwnerReference("TaskRun", "test-taskrun-with-cluster-task",
tb.OwnerReferenceAPIVersion(currentApiVersion)),
tb.OwnerReferenceAPIVersion(currentAPIVersion)),
tb.PodSpec(
tb.PodVolumes(workspaceVolume, homeVolume, toolsVolume, downwardVolume),
tb.PodRestartPolicy(corev1.RestartPolicyNever),
Expand Down Expand Up @@ -768,7 +768,7 @@ func TestReconcile(t *testing.T) {
tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-resource-spec"),
tb.PodLabel(podconvert.ManagedByLabelKey, podconvert.ManagedByLabelValue),
tb.PodOwnerReference("TaskRun", "test-taskrun-with-resource-spec",
tb.OwnerReferenceAPIVersion(currentApiVersion)),
tb.OwnerReferenceAPIVersion(currentAPIVersion)),
tb.PodSpec(
tb.PodVolumes(workspaceVolume, homeVolume, toolsVolume, downwardVolume),
tb.PodRestartPolicy(corev1.RestartPolicyNever),
Expand Down Expand Up @@ -818,7 +818,7 @@ func TestReconcile(t *testing.T) {
tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-pod"),
tb.PodLabel(podconvert.ManagedByLabelKey, podconvert.ManagedByLabelValue),
tb.PodOwnerReference("TaskRun", "test-taskrun-with-pod",
tb.OwnerReferenceAPIVersion(currentApiVersion)),
tb.OwnerReferenceAPIVersion(currentAPIVersion)),
tb.PodSpec(
tb.PodVolumes(workspaceVolume, homeVolume, toolsVolume, downwardVolume),
tb.PodRestartPolicy(corev1.RestartPolicyNever),
Expand Down
6 changes: 1 addition & 5 deletions pkg/reconciler/taskrun/timeout_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,5 @@ func CheckTimeout(tr *v1alpha1.TaskRun) bool {
return false
}
runtime := time.Since(tr.Status.StartTime.Time)
if runtime > timeout {
return true
} else {
return false
}
return runtime > timeout
}
12 changes: 6 additions & 6 deletions pkg/reconciler/timeout_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ type StatusKey interface {
GetRunKey() string
}

// backoff contains state of exponential backoff for a given StatusKey
type backoff struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this exported because GetBackoff is exported below? Can we unexport GetBackoff instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@imjasonh I should have looked 👼 let's see 😛

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are using GetBackoff in the reconcilier 🤔

// Backoff contains state of exponential backoff for a given StatusKey
type Backoff struct {
// NumAttempts reflects the number of times a given StatusKey has been delayed
NumAttempts uint
// NextAttempt is the point in time at which this backoff expires
Expand All @@ -66,7 +66,7 @@ type TimeoutSet struct {
stopCh <-chan struct{}
done map[string]chan bool
doneMut sync.Mutex
backoffs map[string]backoff
backoffs map[string]Backoff
backoffsMut sync.Mutex
}

Expand All @@ -78,7 +78,7 @@ func NewTimeoutHandler(
return &TimeoutSet{
stopCh: stopCh,
done: make(map[string]chan bool),
backoffs: make(map[string]backoff),
backoffs: make(map[string]Backoff),
logger: logger,
}
}
Expand Down Expand Up @@ -135,14 +135,14 @@ func (t *TimeoutSet) getOrCreateFinishedChan(runObj StatusKey) chan bool {
// describing the time at which the next attempt should be performed.
// Additionally a boolean is returned indicating whether a backoff for the
// TaskRun is already in progress.
func (t *TimeoutSet) GetBackoff(tr *v1alpha1.TaskRun) (backoff, bool) {
func (t *TimeoutSet) GetBackoff(tr *v1alpha1.TaskRun) (Backoff, bool) {
t.backoffsMut.Lock()
defer t.backoffsMut.Unlock()
b := t.backoffs[tr.GetRunKey()]
if time.Now().Before(b.NextAttempt) {
return b, true
}
b.NumAttempts += 1
b.NumAttempts++
b.NextAttempt = time.Now().Add(backoffDuration(b.NumAttempts, rand.Intn))
timeoutDeadline := tr.Status.StartTime.Time.Add(tr.Spec.Timeout.Duration)
if timeoutDeadline.Before(b.NextAttempt) {
Expand Down
5 changes: 3 additions & 2 deletions test/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,15 @@ type Informers struct {
Pod coreinformers.PodInformer
}

// TestAssets holds references to the controller, logs, clients, and informers.
type TestAssets struct {
// Assets holds references to the controller, logs, clients, and informers.
type Assets struct {
Controller *controller.Impl
Clients Clients
}

// SeedTestData returns Clients and Informers populated with the
// given Data.
// nolint: golint
func SeedTestData(t *testing.T, ctx context.Context, d Data) (Clients, Informers) {
c := Clients{
Kube: fakekubeclient.Get(ctx),
Expand Down
12 changes: 6 additions & 6 deletions test/helm_task_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,12 @@ func TestHelmDeployPipelineRun(t *testing.T) {
}

t.Log("Waiting for service to get external IP")
var serviceIp string
var serviceIP string
if err := WaitForServiceExternalIPState(c, namespace, helmDeployServiceName, func(svc *corev1.Service) (bool, error) {
ingress := svc.Status.LoadBalancer.Ingress
if ingress != nil {
if len(ingress) > 0 {
serviceIp = ingress[0].IP
serviceIP = ingress[0].IP
return true, nil
}
}
Expand All @@ -117,20 +117,20 @@ func TestHelmDeployPipelineRun(t *testing.T) {
knativetest.CleanupOnInterrupt(func() { helmCleanup(c, t, namespace) }, t.Logf)
defer helmCleanup(c, t, namespace)

if serviceIp != "" {
if serviceIP != "" {
t.Log("Polling service with external IP")
waitErr := wait.PollImmediate(100*time.Millisecond, 30*time.Second, func() (bool, error) {
resp, err := http.Get(fmt.Sprintf("http://%s:8080", serviceIp))
resp, err := http.Get(fmt.Sprintf("http://%s:8080", serviceIP))
if err != nil {
return false, nil
}
if resp != nil && resp.StatusCode != http.StatusOK {
return true, xerrors.Errorf("Expected 200 but received %d response code from service at http://%s:8080", resp.StatusCode, serviceIp)
return true, xerrors.Errorf("Expected 200 but received %d response code from service at http://%s:8080", resp.StatusCode, serviceIP)
}
return true, nil
})
if waitErr != nil {
t.Errorf("Error from pinging service IP %s : %s", serviceIp, waitErr)
t.Errorf("Error from pinging service IP %s : %s", serviceIP, waitErr)
}

} else {
Expand Down