Skip to content

[QUESTION/HELP] Benign "Bug" in pkg/client/tools.go and Linter Configuration Interest #1600

@nolandseigler

Description

@nolandseigler

Question / Where do you need Help?

Hello! Long time user, first time source code explorer here.

While spending some time learning about the codebase through the lens of investigating #1538, I found a benign "bug" here

k3d/pkg/client/tools.go

Lines 65 to 67 in 115c8ac

if err != nil {
return fmt.Errorf("failed to retrieve container runtime information: %w", err)
}
with an impossible error condition. I then used the Makefile to install the tools and run make lint. I believe issue was not caught due to two reasons. The first reason is that it seems like the linter is not running on anything in pkg/ and the second reason is that nilness is not enabled for govet.

If you all are open to modifying linter settings, I would like to assist with this effort to cover more code, tune the linter, and fix whatever issues pop up.

Here is some output with minimal changes. It certainly would need more tuning and I acknowledge that much of this linting can be considered the opinion of the linter author. Some of the deprecations are also of interest.

golangci-lint run
pkg/client/host.go:138:13: nilness: tautological condition: non-nil != nil (govet)
        if scanner != nil && execErr != nil {
                   ^
pkg/client/tools.go:65:10: nilness: impossible condition: nil != nil (govet)
                if err != nil {
                       ^
pkg/config/config_test.go:290:13: unusedwrite: unused write to field HostIP (govet)
        exposedAPI.HostIP = "0.0.0.0"
                   ^
pkg/config/config_test.go:291:13: unusedwrite: unused write to field HostPort (govet)
        exposedAPI.HostPort = "6443"
                   ^
pkg/client/cluster.go:185:17: QF1008: could remove embedded field "Cluster" from selector (staticcheck)
                clusterConfig.Cluster.Nodes = append(clusterConfig.Cluster.Nodes, registryNode)
                              ^
pkg/client/cluster.go:202:84: QF1008: could remove embedded field "Cluster" from selector (staticcheck)
                        if err := RegistryConnectNetworks(ctx, runtime, regNode, []string{clusterConfig.Cluster.Network.Name}); err != nil {
                                                                                                        ^
pkg/client/cluster.go:457:3: QF1003: could use tagged switch on node.Role (staticcheck)
                if node.Role == k3d.ServerRole {
                ^
pkg/client/cluster.go:574:30: QF1008: could remove embedded field "Node" from selector (staticcheck)
                cluster.ServerLoadBalancer.Node.FillRuntimeLabels()
                                           ^
pkg/client/cluster.go:576:31: QF1008: could remove embedded field "Node" from selector (staticcheck)
                        cluster.ServerLoadBalancer.Node.RuntimeLabels[k] = v
                                                   ^
pkg/client/cluster.go:596:30: QF1008: could remove embedded field "Node" from selector (staticcheck)
                cluster.ServerLoadBalancer.Node.HookActions = append(cluster.ServerLoadBalancer.Node.HookActions, writeLbConfigAction)
                                           ^
pkg/client/node.go:538:13: ST1005: error strings should not end with punctuation or newlines (staticcheck)
                                        return fmt.Errorf("[Node %s] Cannot activate DNS fix (K3D_FIX_DNS) when there's a file mounted at /etc/resolv.conf!", node.Name)
                                               ^
pkg/client/node.go:543:12: ST1005: error strings should not end with punctuation or newlines (staticcheck)
                                return fmt.Errorf("Cannot enable DNS fix, as Host Gateway IP is missing!")
                                       ^
pkg/client/node.go:623:2: QF1003: could use tagged switch on node.Role (staticcheck)
        if node.Role == k3d.AgentRole { // TODO: check here AND in CLI or only here?
        ^
pkg/client/registry_test.go:64:5: QF1001: could apply De Morgan's law (staticcheck)
        if !(strings.TrimSpace(string(cm)) == strings.TrimSpace(expectedYAMLString)) {
           ^
pkg/config/process.go:64:19: QF1008: could remove embedded field "PortMapping" from selector (staticcheck)
                cluster.KubeAPI.PortMapping.Binding.HostPort = k3sPort
                                ^
pkg/config/process.go:71:37: QF1008: could remove embedded field "Cluster" from selector (staticcheck)
        for _, node := range clusterConfig.Cluster.Nodes {
                                           ^
pkg/config/transform.go:121:6: S1009: should omit nil check; len() for nil slices is defined as zero (staticcheck)
                if simpleConfig.Options.K3dOptions.Loadbalancer.ConfigOverrides != nil && len(simpleConfig.Options.K3dOptions.Loadbalancer.ConfigOverrides) > 0 {
                   ^
pkg/runtimes/docker/container.go:56:7: SA1019: client.IsErrNotFound is deprecated: use [cerrdefs.IsNotFound] instead. (staticcheck)
                        if client.IsErrNotFound(err) {
                           ^
pkg/runtimes/docker/container.go:118:13: ST1023: should omit type io.Writer from declaration; it will be inferred from the right-hand side (staticcheck)
        var writer io.Writer = io.Discard
                   ^
pkg/runtimes/docker/container.go:130:62: SA1019: types.Container is deprecated: use [container.Summary]. (staticcheck)
func getNodeContainer(ctx context.Context, node *k3d.Node) (*types.Container, error) {
                                                             ^
pkg/runtimes/docker/container.go:155:15: ST1005: error strings should not be capitalized (staticcheck)
                return nil, fmt.Errorf("Failed to list containers: %+v", err)
                            ^
pkg/runtimes/docker/container.go:159:15: ST1005: error strings should not be capitalized (staticcheck)
                return nil, fmt.Errorf("Failed to get a single container for name '%s'. Found: %d", node.Name, len(containers))
                            ^
pkg/runtimes/docker/container.go:163:15: ST1005: error strings should not be capitalized (staticcheck)
                return nil, fmt.Errorf("Didn't find container for node '%s'", node.Name)
                            ^
pkg/runtimes/docker/container.go:188:7: SA1019: client.IsErrNotFound is deprecated: use [cerrdefs.IsNotFound] instead. (staticcheck)
                        if client.IsErrNotFound(err) {
                           ^
pkg/runtimes/docker/node.go:167:77: SA1019: types.Container is deprecated: use [container.Summary]. (staticcheck)
func getContainersByLabel(ctx context.Context, labels map[string]string) ([]types.Container, error) {
                                                                            ^
pkg/runtimes/docker/node.go:196:68: SA1019: types.ContainerJSON is deprecated: use [container.InspectResponse]. It will be removed in the next release. (staticcheck)
func getContainerDetails(ctx context.Context, containerID string) (types.ContainerJSON, error) {
                                                                   ^
pkg/runtimes/docker/node.go:200:10: SA1019: types.ContainerJSON is deprecated: use [container.InspectResponse]. It will be removed in the next release. (staticcheck)
                return types.ContainerJSON{}, fmt.Errorf("failed to create docker client. %w", err)
                       ^
pkg/runtimes/docker/node.go:206:10: SA1019: types.ContainerJSON is deprecated: use [container.InspectResponse]. It will be removed in the next release. (staticcheck)
                return types.ContainerJSON{}, fmt.Errorf("failed to get details for container '%s': %w", containerID, err)
                       ^
pkg/runtimes/docker/node.go:255:37: QF1008: could remove embedded field "ContainerJSONBase" from selector (staticcheck)
        running = containerInspectResponse.ContainerJSONBase.State.Running
                                           ^
pkg/runtimes/docker/node.go:256:41: QF1008: could remove embedded field "ContainerJSONBase" from selector (staticcheck)
        stateString = containerInspectResponse.ContainerJSONBase.State.Status
                                               ^
pkg/runtimes/docker/node.go:290:31: QF1008: could remove embedded field "ContainerJSONBase" from selector (staticcheck)
        if !containerInspectResponse.ContainerJSONBase.State.Running {
                                     ^
pkg/runtimes/docker/node.go:380:2: QF1007: could merge conditional assignment into variable declaration (staticcheck)
        attachStdin := false
        ^
pkg/runtimes/docker/translate.go:185:37: SA1019: types.Container is deprecated: use [container.Summary]. (staticcheck)
func TranslateContainerToNode(cont *types.Container) (*k3d.Node, error) {
                                    ^
pkg/runtimes/docker/translate.go:215:2: QF1007: could merge conditional assignment into variable declaration (staticcheck)
        restart := false
        ^
pkg/runtimes/docker/translate.go:258:3: QF1003: could use tagged switch on k (staticcheck)
                if k == k3d.LabelServerAPIHostIP {
                ^
pkg/runtimes/docker/util.go:89:18: SA1019: archive.CopyInfoSourcePath is deprecated: use [archive.CopyInfoSourcePath] instead. (staticcheck)
        srcInfo, err := archive.CopyInfoSourcePath(src, false)
                        ^
pkg/runtimes/docker/util.go:94:21: SA1019: archive.TarResource is deprecated: use [archive.TarResource] instead. (staticcheck)
        srcArchive, err := archive.TarResource(srcInfo)
                           ^
pkg/runtimes/docker/util.go:100:14: SA1019: archive.CopyInfo is deprecated: use [archive.CopyInfo] instead. (staticcheck)
        destInfo := archive.CopyInfo{Path: dest}
                    ^
pkg/runtimes/docker/util.go:106:35: SA1019: archive.PrepareArchiveCopy is deprecated: use [archive.PrepareArchiveCopy] instead. (staticcheck)
        destDir, preparedArchive, err := archive.PrepareArchiveCopy(srcArchive, srcInfo, destInfo)
                                         ^
pkg/runtimes/docker/util.go:174:6: SA1019: client.IsErrNotFound is deprecated: use [cerrdefs.IsNotFound] instead. (staticcheck)
                if client.IsErrNotFound(err) {
                   ^
pkg/util/filter.go:118:3: QF1003: could use tagged switch on node.Role (staticcheck)
                if node.Role == k3d.ServerRole {
                ^
41 issues:
* govet: 4
* staticcheck: 37

Scope of your Question

  • Is your question related to a specific version of k3d (or k3s)?

This question is related to the main branch and general code for this project standards.

Metadata

Metadata

Assignees

No one assigned

    Labels

    questionFurther information is requested

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions