diff --git a/.golangci.yml b/.golangci.yml index 51bf67939..3bda4241f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -80,6 +80,10 @@ linters-settings: - name: var-naming - name: unconditional-recursion - name: waitgroup-by-value + # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#struct-tag + - name: struct-tag + arguments: + - "json,inline" staticcheck: go: "1.18" unused: diff --git a/Tools.mk b/Tools.mk index c35ebaf05..24b7eed3d 100644 --- a/Tools.mk +++ b/Tools.mk @@ -32,7 +32,7 @@ KUSTOMIZE := $(TOOLS_DIR)/kustomize-$(KUSTOMIZE_VER) SETUP_ENVTEST_VER := v0.0.0-20220304125252-9ee63fc65a97 SETUP_ENVTEST := $(TOOLS_DIR)/setup-envtest-$(SETUP_ENVTEST_VER) -GOLANGCI_LINT_VER := v1.52 +GOLANGCI_LINT_VER := v1.61 GOLANGCI_LINT := $(TOOLS_DIR)/golangci-lint-$(GOLANGCI_LINT_VER) YAMLFMT_VER := v0.6 diff --git a/api/v1alpha1/workflow_methods.go b/api/v1alpha1/workflow_methods.go index 978f406d4..7e1e78493 100644 --- a/api/v1alpha1/workflow_methods.go +++ b/api/v1alpha1/workflow_methods.go @@ -60,7 +60,7 @@ func (w *Workflow) getTaskActionInfo() taskInfo { INNER: for ai, action := range task.Actions { // Find the first non-successful action - switch action.Status { + switch action.Status { //nolint:exhaustive // WorkflowStatePreparing is only used in Workflows not Actions. case WorkflowStateSuccess: actionIndex++ continue diff --git a/api/v1alpha1/workflow_types.go b/api/v1alpha1/workflow_types.go index 7f1ac530a..a3e2919b5 100644 --- a/api/v1alpha1/workflow_types.go +++ b/api/v1alpha1/workflow_types.go @@ -89,7 +89,7 @@ type Status struct { // is no information available. A Reason clarifies an HTTP status // code but does not override it. // +optional - Reason metav1.StatusReason `json:"reason,omitempty" protobuf:"bytes,4,opt,name=reason,casttype=StatusReason"` + Reason metav1.StatusReason `json:"reason,omitempty" protobuf:"bytes,4,opt,name=reason"` // Extended data associated with the reason. Each reason may define its // own extended details. This field is optional and the data returned // is not guaranteed to conform to any schema except that defined by diff --git a/cmd/tink-controller-v1alpha2/main.go b/cmd/tink-controller-v1alpha2/main.go index 50b0c9238..590207ee4 100644 --- a/cmd/tink-controller-v1alpha2/main.go +++ b/cmd/tink-controller-v1alpha2/main.go @@ -73,14 +73,14 @@ func NewRootCommand() *cobra.Command { cmd := &cobra.Command{ Use: "tink-controller", - PreRunE: func(cmd *cobra.Command, args []string) error { + PreRunE: func(cmd *cobra.Command, _ []string) error { viper, err := createViper(logger) if err != nil { return fmt.Errorf("config init: %w", err) } return applyViper(viper, cmd) }, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { logger.Info("Starting controller version " + version) ccfg := clientcmd.NewNonInteractiveDeferredLoadingClientConfig( @@ -172,7 +172,7 @@ func applyViper(v *viper.Viper, cmd *cobra.Command) error { for _, err := range errors { errs = append(errs, err.Error()) } - return fmt.Errorf(strings.Join(errs, ", ")) + return fmt.Errorf("%s", strings.Join(errs, ", ")) } return nil diff --git a/cmd/tink-controller/main.go b/cmd/tink-controller/main.go index 10726a13e..d1686b601 100644 --- a/cmd/tink-controller/main.go +++ b/cmd/tink-controller/main.go @@ -60,14 +60,14 @@ func NewRootCommand() *cobra.Command { cmd := &cobra.Command{ Use: "tink-controller", - PreRunE: func(cmd *cobra.Command, args []string) error { + PreRunE: func(cmd *cobra.Command, _ []string) error { viper, err := createViper(logger) if err != nil { return fmt.Errorf("config init: %w", err) } return applyViper(viper, cmd) }, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { logger.Info("Starting controller version " + version) ccfg := clientcmd.NewNonInteractiveDeferredLoadingClientConfig( @@ -146,7 +146,7 @@ func applyViper(v *viper.Viper, cmd *cobra.Command) error { for _, err := range errors { errs = append(errs, err.Error()) } - return fmt.Errorf(strings.Join(errs, ", ")) + return fmt.Errorf("%s", strings.Join(errs, ", ")) } return nil diff --git a/cmd/tink-server/main.go b/cmd/tink-server/main.go index a9cae0bdc..78051a09b 100644 --- a/cmd/tink-server/main.go +++ b/cmd/tink-server/main.go @@ -78,14 +78,14 @@ func NewRootCommand() *cobra.Command { cmd := &cobra.Command{ Use: "tink-server", - PreRunE: func(cmd *cobra.Command, args []string) error { + PreRunE: func(cmd *cobra.Command, _ []string) error { viper, err := createViper(logger) if err != nil { return err } return applyViper(viper, cmd) }, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { // I am not sure if it is right for this to be here, // but as last step I want to keep compatibility with // what we have for a little bit and I thinik that's @@ -201,7 +201,7 @@ func applyViper(v *viper.Viper, cmd *cobra.Command) error { for _, err := range errors { errs = append(errs, err.Error()) } - return fmt.Errorf(strings.Join(errs, ", ")) + return fmt.Errorf("%s", strings.Join(errs, ", ")) } return nil diff --git a/cmd/tink-worker/cmd/root.go b/cmd/tink-worker/cmd/root.go index 6bdce966a..d4155ef2d 100644 --- a/cmd/tink-worker/cmd/root.go +++ b/cmd/tink-worker/cmd/root.go @@ -39,10 +39,10 @@ func NewRootCommand(version string) *cobra.Command { Use: "tink-worker", Short: "Tink Worker", Version: version, - PreRunE: func(cmd *cobra.Command, args []string) error { + PreRunE: func(cmd *cobra.Command, _ []string) error { return initViper(logger, cmd) }, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { retryInterval := viper.GetDuration("retry-interval") retries := viper.GetInt("max-retry") workerID := viper.GetString("id") diff --git a/cmd/tink-worker/worker/log_capturer_test.go b/cmd/tink-worker/worker/log_capturer_test.go index dad44c5b7..9b257beeb 100644 --- a/cmd/tink-worker/worker/log_capturer_test.go +++ b/cmd/tink-worker/worker/log_capturer_test.go @@ -94,7 +94,7 @@ func TestLogCapturerContextLogger(t *testing.T) { } for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { + t.Run(tc.name, func(_ *testing.T) { logger := zapr.NewLogger(zap.Must(zap.NewDevelopment())) ctx := context.Background() if tc.logger != nil { diff --git a/cmd/virtual-worker/cmd/root.go b/cmd/virtual-worker/cmd/root.go index fa9b0e93c..46f04c4c2 100644 --- a/cmd/virtual-worker/cmd/root.go +++ b/cmd/virtual-worker/cmd/root.go @@ -34,10 +34,10 @@ func NewRootCommand(version string) *cobra.Command { rootCmd := &cobra.Command{ Use: "virtual-worker", Short: "Virtual Tink Worker", - PreRunE: func(cmd *cobra.Command, args []string) error { + PreRunE: func(cmd *cobra.Command, _ []string) error { return createViper(logger, cmd) }, - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { retryInterval := viper.GetDuration("retry-interval") retries := viper.GetInt("max-retry") workerID := viper.GetString("id") diff --git a/cmd/virtual-worker/worker/container_manager.go b/cmd/virtual-worker/worker/container_manager.go index 4a9b8fa10..c4ff26629 100644 --- a/cmd/virtual-worker/worker/container_manager.go +++ b/cmd/virtual-worker/worker/container_manager.go @@ -30,7 +30,7 @@ type fakeManager struct { } func (m *fakeManager) sleep() { - jitter := time.Duration(m.r.Int31n(int32(m.sleepJitter.Milliseconds()))) * time.Millisecond + jitter := time.Duration(m.r.Int63n(m.sleepJitter.Milliseconds())) * time.Millisecond time.Sleep(jitter + m.sleepMinimum) } diff --git a/internal/agent/agent_test.go b/internal/agent/agent_test.go index 83ddd6dfd..6b5568013 100644 --- a/internal/agent/agent_test.go +++ b/internal/agent/agent_test.go @@ -105,7 +105,7 @@ func TestAgent_ConcurrentWorkflows(t *testing.T) { // Started is used to indicate the runtime has received the workflow. started := make(chan struct{}) rntime := agent.ContainerRuntimeMock{ - RunFunc: func(ctx context.Context, action workflow.Action) error { + RunFunc: func(ctx context.Context, _ workflow.Action) error { started <- struct{}{} <-ctx.Done() return nil @@ -412,7 +412,7 @@ message`, trnport := transport.Noop() rntime := agent.ContainerRuntimeMock{ - RunFunc: func(ctx context.Context, action workflow.Action) error { + RunFunc: func(_ context.Context, action workflow.Action) error { if res, ok := tc.Errors[action.ID]; ok { return failure.NewReason(res.Message, res.Reason) } @@ -424,7 +424,7 @@ message`, // to check for the last expected action. lastEventReceived := make(chan struct{}) recorder := event.RecorderMock{ - RecordEventFunc: func(contextMoqParam context.Context, event event.Event) error { + RecordEventFunc: func(_ context.Context, event event.Event) error { if cmp.Equal(event, tc.Events[len(tc.Events)-1]) { lastEventReceived <- struct{}{} } diff --git a/internal/agent/transport/file_test.go b/internal/agent/transport/file_test.go index d6e2e426f..ca2b29e85 100644 --- a/internal/agent/transport/file_test.go +++ b/internal/agent/transport/file_test.go @@ -46,7 +46,7 @@ func TestFile(t *testing.T) { defer cancel() handler := &transport.WorkflowHandlerMock{ - HandleWorkflowFunc: func(contextMoqParam context.Context, workflow workflow.Workflow, recorder event.Recorder) { + HandleWorkflowFunc: func(_ context.Context, workflow workflow.Workflow, _ event.Recorder) { if !cmp.Equal(expect, workflow) { t.Fatalf("Workflow diff:\n%v", cmp.Diff(expect, workflow)) } diff --git a/internal/agent/transport/grpc_test.go b/internal/agent/transport/grpc_test.go index 80660a6cb..fb158399c 100644 --- a/internal/agent/transport/grpc_test.go +++ b/internal/agent/transport/grpc_test.go @@ -46,7 +46,7 @@ func TestGRPC(t *testing.T) { ContextFunc: context.Background, } client := &workflowproto.WorkflowServiceClientMock{ - GetWorkflowsFunc: func(ctx context.Context, in *workflowproto.GetWorkflowsRequest, opts ...grpc.CallOption) (workflowproto.WorkflowService_GetWorkflowsClient, error) { + GetWorkflowsFunc: func(_ context.Context, _ *workflowproto.GetWorkflowsRequest, _ ...grpc.CallOption) (workflowproto.WorkflowService_GetWorkflowsClient, error) { return stream, nil }, } @@ -54,7 +54,7 @@ func TestGRPC(t *testing.T) { var wg sync.WaitGroup wg.Add(1) handler := &transport.WorkflowHandlerMock{ - HandleWorkflowFunc: func(contextMoqParam context.Context, workflow workflow.Workflow, recorder event.Recorder) { + HandleWorkflowFunc: func(_ context.Context, _ workflow.Workflow, _ event.Recorder) { defer wg.Done() close(responses) }, diff --git a/internal/cli/agent.go b/internal/cli/agent.go index e01f2eb07..d1529dfff 100644 --- a/internal/cli/agent.go +++ b/internal/cli/agent.go @@ -23,7 +23,7 @@ func NewAgent() *cobra.Command { // TODO(chrisdoherty4) Handle signals cmd := cobra.Command{ Use: "tink-agent", - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { zl, err := zap.NewProduction() if err != nil { return fmt.Errorf("init logger: %w", err) @@ -35,7 +35,7 @@ func NewAgent() *cobra.Command { return fmt.Errorf("create runtime: %w", err) } - conn, err := grpc.DialContext(cmd.Context(), opts.TinkServerAddr) + conn, err := grpc.NewClient(opts.TinkServerAddr) if err != nil { return fmt.Errorf("dial tink server: %w", err) } diff --git a/internal/client/client.go b/internal/client/client.go index ee6ee83c8..c143525e8 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -18,7 +18,7 @@ func NewClientConn(authority string, tlsEnabled bool, tlsInsecure bool) (*grpc.C creds = grpc.WithTransportCredentials(insecure.NewCredentials()) } - conn, err := grpc.Dial(authority, creds, grpc.WithStatsHandler(otelgrpc.NewClientHandler())) + conn, err := grpc.NewClient(authority, creds, grpc.WithStatsHandler(otelgrpc.NewClientHandler())) if err != nil { return nil, errors.Wrap(err, "dial tinkerbell server") } diff --git a/internal/deprecated/workflow/reconciler.go b/internal/deprecated/workflow/reconciler.go index 06a7f615d..e38d093da 100644 --- a/internal/deprecated/workflow/reconciler.go +++ b/internal/deprecated/workflow/reconciler.go @@ -10,7 +10,6 @@ import ( "github.com/tinkerbell/tink/api/v1alpha1" "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/errors" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/ptr" ctrl "sigs.k8s.io/controller-runtime" @@ -111,7 +110,6 @@ func (r *Reconciler) Reconcile(ctx context.Context, req reconcile.Request) (reco } func (r *Reconciler) handleHardwareAllowPXE(ctx context.Context, stored *v1alpha1.Workflow, hardware *v1alpha1.Hardware) error { - // We need to set allowPXE to true before a workflow runs. // We need to set allowPXE to false after a workflow completes successfully. @@ -206,7 +204,7 @@ func (r *Reconciler) processNewWorkflow(ctx context.Context, logger logr.Logger, } // netboot the hardware if requested - if stored.Spec.BootOpts.OneTimeNetboot { + if stored.Spec.BootOpts.OneTimeNetboot { //nolint:nestif // Will work on this complexity. // check if the hardware has a bmcRef if hardware.Spec.BMCRef == nil { return reconcile.Result{}, fmt.Errorf("hardware %s does not have a BMC, cannot perform one time netboot", hardware.Name) @@ -227,12 +225,10 @@ func (r *Reconciler) processNewWorkflow(ctx context.Context, logger logr.Logger, stored.Status.OneTimeNetboot.DeletionStatus = &v1alpha1.Status{Status: v1alpha1.StatusSuccess, Message: "previous existing one time netboot job deleted"} return reconcile.Result{Requeue: true}, nil + } else if errors.IsNotFound(err) { + stored.Status.OneTimeNetboot.DeletionStatus = &v1alpha1.Status{Status: v1alpha1.StatusSuccess, Message: "no existing one time netboot job found"} } else { - if apierrors.IsNotFound(err) { - stored.Status.OneTimeNetboot.DeletionStatus = &v1alpha1.Status{Status: v1alpha1.StatusSuccess, Message: "no existing one time netboot job found"} - } else { - return reconcile.Result{Requeue: true}, err - } + return reconcile.Result{Requeue: true}, err } } @@ -329,7 +325,7 @@ func toTemplateHardwareData(hardware v1alpha1.Hardware) templateHardwareData { return contract } -func (r *Reconciler) processRunningWorkflow(_ context.Context, stored *v1alpha1.Workflow) reconcile.Result { +func (r *Reconciler) processRunningWorkflow(_ context.Context, stored *v1alpha1.Workflow) reconcile.Result { //nolint:unparam // This is the way controller runtime works. // Check for global timeout expiration if r.nowFunc().After(stored.GetStartTime().Add(time.Duration(stored.Status.GlobalTimeout) * time.Second)) { stored.Status.State = v1alpha1.WorkflowStateTimeout diff --git a/internal/workflow/internal/reconcile_test.go b/internal/workflow/internal/reconcile_test.go index 6cccf8456..5becb75ad 100644 --- a/internal/workflow/internal/reconcile_test.go +++ b/internal/workflow/internal/reconcile_test.go @@ -10,7 +10,7 @@ import ( "github.com/rs/zerolog" tinkv1 "github.com/tinkerbell/tink/api/v1alpha2" "github.com/tinkerbell/tink/internal/ptr" - . "github.com/tinkerbell/tink/internal/workflow/internal" + . "github.com/tinkerbell/tink/internal/workflow/internal" //nolint:revive // Dot imports should not be used. Problem for another time though. corev1 "k8s.io/api/core/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime"