refactor(topo): address linter violations and style in topo nodes (4/5)

topo/node/alpine/alpine.go

@@ -22,7 +22,7 @@ import (
 
 	"github.com/openconfig/kne/topo/node"
 	"google.golang.org/protobuf/proto"
-	"k8s.io/utils/pointer"
+	"k8s.io/utils/ptr"
 
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -123,7 +123,7 @@ func (n *Node) CreatePod(ctx context.Context) error {
 		Resources:       node.ToResourceRequirements(pb.Constraints),
 		ImagePullPolicy: "IfNotPresent",
 		SecurityContext: &corev1.SecurityContext{
-			Privileged: pointer.Bool(true),
+			Privileged: ptr.To(true),
 		},
 	}}
 
@@ -193,14 +193,14 @@ func (n *Node) CreatePod(ctx context.Context) error {
 				Resources:       node.ToResourceRequirements(pb.Constraints),
 				ImagePullPolicy: "IfNotPresent",
 				SecurityContext: &corev1.SecurityContext{
-					Privileged: pointer.Bool(true),
+					Privileged: ptr.To(true),
 				},
 				VolumeMounts: extraMounts,
 			}
 			alpineContainers = append(alpineContainers, containerSpec)
 		default:
 			// Only Dataplane container is supported as the custom container
-			return fmt.Errorf("Alpine supports only 1 custom container, %d provided.", numContainers)
+			return fmt.Errorf("alpine supports only 1 custom container, %d provided", numContainers)
 		}
 	}
 
@@ -223,11 +223,11 @@ func (n *Node) CreatePod(ctx context.Context) error {
 				},
 				ImagePullPolicy: "IfNotPresent",
 				SecurityContext: &corev1.SecurityContext{
-					Privileged: pointer.Bool(true),
+					Privileged: ptr.To(true),
 				},
 			}},
 			Containers:                    alpineContainers,
-			TerminationGracePeriodSeconds: pointer.Int64(0),
+			TerminationGracePeriodSeconds: ptr.To(int64(0)),
 			NodeSelector:                  map[string]string{},
 			Affinity: &corev1.Affinity{
 				PodAntiAffinity: &corev1.PodAntiAffinity{
@@ -261,7 +261,7 @@ func (n *Node) CreatePod(ctx context.Context) error {
 			MountPath: pb.Config.ConfigPath + "/" + pb.Config.ConfigFile,
 			ReadOnly:  true,
 		}
-		if vol.VolumeSource.ConfigMap != nil {
+		if vol.ConfigMap != nil {
 			vm.SubPath = pb.Config.ConfigFile
 		}
 		for i, c := range pod.Spec.Containers {

topo/node/alpine/alpine_test.go

@@ -29,7 +29,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kfake "k8s.io/client-go/kubernetes/fake"
-	"k8s.io/utils/pointer"
+	"k8s.io/utils/ptr"
 )
 
 func TestNew(t *testing.T) {
@@ -200,7 +200,7 @@ func TestCreatePod(t *testing.T) {
 				Requests: corev1.ResourceList{}},
 			ImagePullPolicy: "IfNotPresent",
 			SecurityContext: &corev1.SecurityContext{
-				Privileged: pointer.Bool(true),
+				Privileged: ptr.To(true),
 			},
 		},
 		wantDpCtr: corev1.Container{
@@ -212,7 +212,7 @@ func TestCreatePod(t *testing.T) {
 				Requests: corev1.ResourceList{}},
 			ImagePullPolicy: "IfNotPresent",
 			SecurityContext: &corev1.SecurityContext{
-				Privileged: pointer.Bool(true),
+				Privileged: ptr.To(true),
 			},
 			VolumeMounts: []corev1.VolumeMount{{Name: "files", MountPath: "/files"}},
 		},
@@ -246,7 +246,7 @@ func TestCreatePod(t *testing.T) {
 				Requests: corev1.ResourceList{}},
 			ImagePullPolicy: "IfNotPresent",
 			SecurityContext: &corev1.SecurityContext{
-				Privileged: pointer.Bool(true),
+				Privileged: ptr.To(true),
 			},
 			VolumeMounts: []corev1.VolumeMount{{
 				Name:      "startup-config-volume",
@@ -264,7 +264,7 @@ func TestCreatePod(t *testing.T) {
 				Requests: corev1.ResourceList{}},
 			ImagePullPolicy: "IfNotPresent",
 			SecurityContext: &corev1.SecurityContext{
-				Privileged: pointer.Bool(true),
+				Privileged: ptr.To(true),
 			},
 			VolumeMounts: []corev1.VolumeMount{{
 				Name:      "files",
@@ -297,7 +297,7 @@ func TestCreatePod(t *testing.T) {
 				Requests: corev1.ResourceList{}},
 			ImagePullPolicy: "IfNotPresent",
 			SecurityContext: &corev1.SecurityContext{
-				Privileged: pointer.Bool(true),
+				Privileged: ptr.To(true),
 			},
 		},
 	}}

topo/node/arista/arista.go

@@ -259,7 +259,7 @@ func (n *Node) CreateCRD(ctx context.Context) error {
 		},
 	}
 	for label, v := range proto.GetLabels() {
-		device.ObjectMeta.Labels[label] = v
+		device.Labels[label] = v
 	}
 	for _, service := range proto.GetServices() {
 		insidePort := service.Inside
@@ -396,7 +396,7 @@ func (n *Node) ConfigPush(ctx context.Context, r io.Reader) error {
 	}
 
 	if resp.Failed == nil {
-		log.Infof("%s - finished config push", n.Impl.Proto.Name)
+		log.Infof("%s - finished config push", n.Proto.Name)
 	}
 
 	return resp.Failed
@@ -422,7 +422,7 @@ func (n *Node) ResetCfg(ctx context.Context) error {
 	}
 
 	if resp.Failed == nil {
-		log.Infof("%s - finshed resetting config", n.Name())
+		log.Infof("%s - finished resetting config", n.Name())
 	}
 
 	return resp.Failed
@@ -504,7 +504,7 @@ func (n *Node) FixInterfaces() error {
 	for k, v := range n.Proto.Interfaces {
 		switch {
 		default:
-			return fmt.Errorf("Unrecognized interface name: %s", v.Name)
+			return fmt.Errorf("unrecognized interface name: %s", v.Name)
 		case !strings.HasPrefix(k, "eth"), ethIntfRe.MatchString(v.Name), mgmtIntfRe.MatchString(v.Name):
 		case v.Name == "":
 			n.Proto.Interfaces[k].Name = fmt.Sprintf("Ethernet%s", strings.TrimPrefix(k, "eth"))

topo/node/arista/arista_test.go

@@ -87,7 +87,7 @@ func TestNew(t *testing.T) {
 					},
 				},
 			},
-			wantErr: "Unrecognized interface name: Ethernet1/2/3/4",
+			wantErr: "unrecognized interface name: Ethernet1/2/3/4",
 		}, {
 			desc: "invalid eth intfs 2",
 			nImpl: &node.Impl{
@@ -97,7 +97,7 @@ func TestNew(t *testing.T) {
 					},
 				},
 			},
-			wantErr: "Unrecognized interface name: Ethernet",
+			wantErr: "unrecognized interface name: Ethernet",
 		}, {
 			desc: "invalid management intfs 1",
 			nImpl: &node.Impl{
@@ -109,7 +109,7 @@ func TestNew(t *testing.T) {
 					},
 				},
 			},
-			wantErr: "Unrecognized interface name: Management1/2/3",
+			wantErr: "unrecognized interface name: Management1/2/3",
 		}, {
 			desc: "invalid management intfs 2",
 			nImpl: &node.Impl{
@@ -119,7 +119,7 @@ func TestNew(t *testing.T) {
 					},
 				},
 			},
-			wantErr: "Unrecognized interface name: Management",
+			wantErr: "unrecognized interface name: Management",
 		}, {
 			desc: "default check with empty topo proto",
 			nImpl: &node.Impl{
@@ -457,7 +457,7 @@ func TestCRD(t *testing.T) {
 					Proto:      tt.proto,
 				},
 			}
-			node.Impl.Proto.Name = name
+			node.Proto.Name = name
 			err := node.CreateCRD(ctx)
 			if s := errdiff.Check(err, tt.wantErr); s != "" {
 				t.Errorf("New() unexpected err: %s", s)
@@ -623,7 +623,7 @@ func TestStatus(t *testing.T) {
 					Proto:      &topopb.Node{},
 				},
 			}
-			node.Impl.Proto.Name = name
+			node.Proto.Name = name
 			status, err := node.Status(ctx)
 			if s := errdiff.Check(err, tt.cantWatch); s != "" {
 				t.Errorf("Status() unexpected err: %s", s)

topo/node/cisco/cisco.go

@@ -37,7 +37,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/rest"
 	log "k8s.io/klog/v2"
-	"k8s.io/utils/pointer"
+	"k8s.io/utils/ptr"
 )
 
 const (
@@ -161,16 +161,16 @@ func (n *Node) Create(ctx context.Context) error {
 		initContainerImage = node.DefaultInitContainerImage
 	}
 	secContext := &corev1.SecurityContext{
-		Privileged: pointer.Bool(true),
+		Privileged: ptr.To(true),
 	}
 	tty := false
 	stdin := false
 	// XRd requires additional security context and the ability to create a tty
 	// terminal. This is not required for 8000e nodes.
 	if pb.Model == ModelXRD {
 		secContext = &corev1.SecurityContext{
-			Privileged: pointer.Bool(true),
-			RunAsUser:  pointer.Int64(0),
+			Privileged: ptr.To(true),
+			RunAsUser:  ptr.To(int64(0)),
 			Capabilities: &corev1.Capabilities{
 				Add: []corev1.Capability{"SYS_ADMIN"},
 			},
@@ -221,7 +221,7 @@ func (n *Node) Create(ctx context.Context) error {
 					},
 				},
 			}},
-			TerminationGracePeriodSeconds: pointer.Int64(0),
+			TerminationGracePeriodSeconds: ptr.To(int64(0)),
 			NodeSelector:                  map[string]string{},
 			Affinity: &corev1.Affinity{
 				PodAntiAffinity: &corev1.PodAntiAffinity{
@@ -243,7 +243,7 @@ func (n *Node) Create(ctx context.Context) error {
 		},
 	}
 	for label, v := range n.GetProto().GetLabels() {
-		pod.ObjectMeta.Labels[label] = v
+		pod.Labels[label] = v
 	}
 	if pb.Config.ConfigData != nil {
 		vol, err := n.CreateConfig(ctx)
@@ -256,7 +256,7 @@ func (n *Node) Create(ctx context.Context) error {
 			MountPath: pb.Config.ConfigPath + "/" + pb.Config.ConfigFile,
 			ReadOnly:  true,
 		}
-		if vol.VolumeSource.ConfigMap != nil {
+		if vol.ConfigMap != nil {
 			vm.SubPath = pb.Config.ConfigFile
 		}
 		for i, c := range pod.Spec.Containers {
@@ -277,10 +277,10 @@ func (n *Node) Create(ctx context.Context) error {
 }
 
 // DefaultNodeConstraints returns default node constraints for CISCO.
-// If the model for 8000e is specificied correctly it returns defaults for 8000e.
+// If the model for 8000e is specified correctly it returns defaults for 8000e.
 // Otherwise, it returns defaults for XRD by default.
 func (n *Node) DefaultNodeConstraints() node.Constraints {
-	if n.Impl == nil || n.Impl.Proto == nil {
+	if n.Impl == nil || n.Proto == nil {
 		return defaultXRDConstraints
 	}
 	switch n.GetProto().Model {
@@ -292,7 +292,7 @@ func (n *Node) DefaultNodeConstraints() node.Constraints {
 	return defaultXRDConstraints
 }
 
-// validateHostConstraints - Validates host contraints through the default node's implementation. It skips the validation optionally
+// validateHostConstraints - Validates host constraints through the default node's implementation. It skips the validation optionally
 // based on skipValidation flag which is useful for unit tests
 func validateHostConstraints(n *Node, skipValidation bool) error {
 	if skipValidation {
@@ -409,7 +409,10 @@ func getCiscoInterfaceID(pb *tpb.Node, eth string) (string, error) {
 		return pb.Interfaces[eth].Name, nil
 	}
 	// ethWithIDRegx.MatchString(eth) was successful, so no need to do extra check here
-	ethID, _ := strconv.Atoi(ethRegx.Split(eth, -1)[1])
+	ethID, err := strconv.Atoi(ethRegx.Split(eth, -1)[1])
+	if err != nil {
+		return "", fmt.Errorf("failed to parse interface ID from %q: %w", eth, err)
+	}
 	eid := ethID - 1
 	switch pb.Model {
 	case "8201":
@@ -657,8 +660,8 @@ func endTelnet(d *scraplinetwork.Driver) error {
 	// sending ctrl + ] (^]) to end telnet session gracefully. Otherwise, the next connection can be blocked.
 	endTelnet := string(byte(29)) + " quit\n"
 	log.Infof("Closing the connection by sending ctrl+] quit \n")
-	d.SendCommand(endTelnet)
-	return nil
+	_, err := d.SendCommand(endTelnet)
+	return err
 }
 
 func (n *Node) ResetCfg(ctx context.Context) error {
@@ -671,9 +674,9 @@ func (n *Node) ResetCfg(ctx context.Context) error {
 
 	var cmd string
 	if n.Proto.Model == ModelXRD {
-		// Copy the snooped management interface config from a know location and the startup config from
+		// Copy the snooped management interface config from a known location and the startup config from
 		// the mounted location so it can be applied. This is required to preserve the snooped management
-		// IP addres and since the "copy" xr_cli command can only access files on disk 0/1.
+		// IP address and since the "copy" xr_cli command can only access files on disk 0/1.
 		startup_config := n.Proto.Config.Env["XR_EVERY_BOOT_CONFIG"]
 		if startup_config == "" {
 			return status.Errorf(codes.InvalidArgument, "XR_EVERY_BOOT_CONFIG is not set")
@@ -748,7 +751,7 @@ func (n *Node) ConfigPush(ctx context.Context, r io.Reader) error {
 		return err
 	}
 	if resp.Failed == nil {
-		log.Infof("%s - finished config push", n.Impl.Proto.Name)
+		log.Infof("%s - finished config push", n.Proto.Name)
 	}
 
 	return resp.Failed
@@ -757,7 +760,7 @@ func (n *Node) ConfigPush(ctx context.Context, r io.Reader) error {
 func (n *Node) GenerateSelfSigned(context.Context) error {
 	// IOS XR automatically generates a self-signed certificate when gRPC is first enabled.
 	// If the startup configuration contains a gRPC configuration, or if the user configures
-	// gRPC after bootup, the self-signed cert will automatically be created and used.
+	// gRPC after boot up, the self-signed cert will automatically be created and used.
 	return status.Errorf(codes.Unimplemented, "certificate generation is not supported")
 }
 

topo/node/cisco/cisco_test.go

@@ -41,7 +41,10 @@ func init() {
 }
 
 func defaultNode(pb *tpb.Node) *tpb.Node {
-	node, _ := defaults(pb)
+	node, err := defaults(pb)
+	if err != nil {
+		panic(err)
+	}
 	return node
 }
 
@@ -945,8 +948,14 @@ func TestNodeStatus(t *testing.T) {
 				}()
 				podIsUpRegex = regexp.MustCompile("fake log") // this is the expected log from a fake pod
 			}
-			nImpl, _ := New(tt.ni)
-			n, _ := nImpl.(*Node)
+			nImpl, err := New(tt.ni)
+			if err != nil {
+				t.Fatalf("New() failed: %v", err)
+			}
+			n, ok := nImpl.(*Node)
+			if !ok {
+				t.Fatalf("nImpl is not a *Node")
+			}
 			status, err := n.Status(ctx)
 			if err != nil {
 				t.Errorf("Error is not expected for Node Status")

topo/node/drivenets/drivenets.go

@@ -121,7 +121,7 @@ var clientFn = func(c *rest.Config) (clientset.Interface, error) {
 }
 
 func (n *Node) Create(ctx context.Context) error {
-	if n.Impl.Proto.Model != modelCdnos {
+	if n.Proto.Model != modelCdnos {
 		return fmt.Errorf("cannot create an instance of an unknown model")
 	}
 	return n.cdnosCreate(ctx)
@@ -200,7 +200,7 @@ func (n *Node) cdnosCreate(ctx context.Context) error {
 }
 
 func (n *Node) Status(ctx context.Context) (node.Status, error) {
-	if n.Impl.Proto.Model != modelCdnos {
+	if n.Proto.Model != modelCdnos {
 		return node.StatusUnknown, fmt.Errorf("invalid model specified")
 	}
 	return n.cdnosStatus(ctx)
@@ -228,7 +228,7 @@ func (n *Node) cdnosStatus(ctx context.Context) (node.Status, error) {
 }
 
 func (n *Node) Delete(ctx context.Context) error {
-	if n.Impl.Proto.Model != modelCdnos {
+	if n.Proto.Model != modelCdnos {
 		return fmt.Errorf("unknown model")
 	}
 	return n.cdnosDelete(ctx)