Ensure comments end with period as godoc suggests

.golangci.yml

@@ -114,34 +114,34 @@ linters:
     - perfsprint
     - revive
     - gosec
-    - gochecknoglobals  # TODO FIXME
+    - godot # End all godoc comments with period
   disable:
     - govet # We already run with `make verify/govet`
     # We do not use
     - gomoddirectives # We need `replace` in `go.mod`
     - depguard # We don't guard against dependencies
-    - testpackage # Requires separate test package
-    - funlen # Long names happen
-    - maintidx # Maintainability Index
+    - testpackage # Require separate test package to catch leaky unexported dependencies
+    - funlen # Long func names happen
+    - varnamelen # Long var names happen
+    - maintidx # Maintainability index
     - cyclop # Cyclomatic complexity
+    - gocognit # Cognitive complexity
     - wsl # Too strict of a whitespace linter
     # TODO Investigate FIXME
-    - dupl # TODO FIXME (tests + non-tests)
-    - err113 # TODO FIXME
-    - wrapcheck # Many false positives
-    - varnamelen # Many false positives
-    - mnd # Many false positives
-    - gomnd # Many false positives
-    - paralleltest # TODO FIXME (Too many tests that aren't parallelized right now)
-    - nestif # Many false positives
-    - gocognit # Many false positives
-    - godox # TODO FIXME
-    # Ask team
+    - err113 # TODO FIXME do not create errors dynamically from scratch. Instead wrap static (package-level) error. 2 points
+    - wrapcheck # TODO ^^
+    - gochecknoglobals  # TODO FIXME A whole 2 point task on its own
+    - paralleltest # TODO FIXME (Too many tests that aren't parallelized right now, probably half a day to attempt to mark the ones we can)
+    - nestif # TODO whole refactoring/readability task, should split up
+    - godox # TODO FIXME audit our project TODOs
+    # TODO Q: Consult with team
     - lll # Limit line length
-    - godot # TODO Ask team if we want to end all godoc comments with period
-    - gofumpt # TODO Ask team if we want to rely on gofumpt
-    - ireturn # ask team if we want to accept interfaces return concrete types
-    - nlreturn # Meh, ask team newline return
-    - nonamedreturns # Ask team, maybe nolint a few of the places it actually leads to cleaner code
-    - exhaustruct # Seems REALLY painful for kubernetes structs...
-    - interfacebloat # Cloud and Mounter have 15 instead of 10... Smell or necessary? We can nolint
+    - gofumpt # Rely on gofumpt's stricter formatting opinions
+    - ireturn # Always accept interfaces return concrete types (gopherism)
+    - nlreturn # Always have emptyline before return
+    - nonamedreturns # No using named returns in functions. Maybe nolint a few of the places it actually leads to cleaner code
+    - exhaustruct # Forces you to explicitly instantiate all structs. REALLY painful for kubernetes structs...
+    - interfacebloat # Cloud and Mounter have 15 methods instead of linter's recommended 10... Smell or necessary? We can nolint specific ones
+    - mnd # Magic Number Detection. Many false positives, still worth with nolint?
+    - gomnd # Many false positives, still worth with nolint?
+    - dupl # Tracks code duplication. Brutal amount of duplication in tests. False positives in non-tests.

cmd/hooks/prestop.go

@@ -47,7 +47,7 @@ const clusterAutoscalerTaint = "ToBeDeletedByClusterAutoscaler"
 const v1KarpenterTaint = "karpenter.sh/disrupted"
 const v1beta1KarpenterTaint = "karpenter.sh/disruption"
 
-// drainTaints includes taints used by K8s or autoscalers that signify node draining or pod eviction
+// drainTaints includes taints used by K8s or autoscalers that signify node draining or pod eviction.
 var drainTaints = map[string]struct{}{
 	v1.TaintNodeUnschedulable: {}, // Kubernetes common eviction taint (kubectl drain)
 	clusterAutoscalerTaint:    {},

pkg/cloud/cloud.go

@@ -41,7 +41,7 @@ import (
 	"k8s.io/klog/v2"
 )
 
-// AWS volume types
+// AWS volume types.
 const (
 	// VolumeTypeIO1 represents a provisioned IOPS SSD type of volume.
 	VolumeTypeIO1 = "io1"
@@ -111,13 +111,13 @@ const (
 	MaxTagValueLength = 256
 )
 
-// Defaults
+// Defaults.
 const (
 	// DefaultVolumeSize represents the default volume size.
 	DefaultVolumeSize int64 = 100 * util.GiB
 )
 
-// Tags
+// Tags.
 const (
 	// VolumeNameTagKey is the key value that refers to the volume's name.
 	VolumeNameTagKey = "CSIVolumeName"
@@ -127,11 +127,11 @@ const (
 	KubernetesTagKeyPrefix = "kubernetes.io"
 	// AWSTagKeyPrefix is the prefix of the key value that is reserved for AWS.
 	AWSTagKeyPrefix = "aws:"
-	// AwsEbsDriverTagKey is the tag to identify if a volume/snapshot is managed by ebs csi driver
+	// AwsEbsDriverTagKey is the tag to identify if a volume/snapshot is managed by ebs csi driver.
 	AwsEbsDriverTagKey = "ebs.csi.aws.com/cluster"
 )
 
-// Batcher
+// Batcher.
 const (
 	volumeIDBatcher volumeBatcherType = iota
 	volumeTagBatcher
@@ -162,23 +162,23 @@ var (
 	ErrAlreadyExists = errors.New("resource already exists")
 
 	// ErrMultiSnapshots is returned when multiple snapshots are found
-	// with the same ID
+	// with the same ID.
 	ErrMultiSnapshots = errors.New("multiple snapshots with the same name found")
 
-	// ErrInvalidMaxResults is returned when a MaxResults pagination parameter is between 1 and 4
+	// ErrInvalidMaxResults is returned when a MaxResults pagination parameter is between 1 and 4.
 	ErrInvalidMaxResults = errors.New("maxResults parameter must be 0 or greater than or equal to 5")
 
-	// ErrVolumeNotBeingModified is returned if volume being described is not being modified
+	// ErrVolumeNotBeingModified is returned if volume being described is not being modified.
 	ErrVolumeNotBeingModified = errors.New("volume is not being modified")
 
-	// ErrInvalidArgument is returned if parameters were rejected by cloud provider
+	// ErrInvalidArgument is returned if parameters were rejected by cloud provider.
 	ErrInvalidArgument = errors.New("invalid argument")
 
-	// ErrInvalidRequest is returned if parameters were rejected by driver
+	// ErrInvalidRequest is returned if parameters were rejected by driver.
 	ErrInvalidRequest = errors.New("invalid request")
 )
 
-// Set during build time via -ldflags
+// Set during build time via -ldflags.
 var driverVersion string
 
 var invalidParameterErrorCodes = map[string]struct{}{
@@ -192,7 +192,7 @@ var invalidParameterErrorCodes = map[string]struct{}{
 	"ValidationError":             {},
 }
 
-// Disk represents a EBS volume
+// Disk represents a EBS volume.
 type Disk struct {
 	VolumeID         string
 	CapacityGiB      int32
@@ -202,7 +202,7 @@ type Disk struct {
 	Attachments      []string
 }
 
-// DiskOptions represents parameters to create an EBS volume
+// DiskOptions represents parameters to create an EBS volume.
 type DiskOptions struct {
 	CapacityBytes          int64
 	Tags                   map[string]string
@@ -222,20 +222,20 @@ type DiskOptions struct {
 	SnapshotID string
 }
 
-// ModifyDiskOptions represents parameters to modify an EBS volume
+// ModifyDiskOptions represents parameters to modify an EBS volume.
 type ModifyDiskOptions struct {
 	VolumeType string
 	IOPS       int32
 	Throughput int32
 }
 
-// ModifyTagsOptions represents parameter to modify the tags of an existing EBS volume
+// ModifyTagsOptions represents parameter to modify the tags of an existing EBS volume.
 type ModifyTagsOptions struct {
 	TagsToAdd    map[string]string
 	TagsToDelete []string
 }
 
-// Snapshot represents an EBS volume snapshot
+// Snapshot represents an EBS volume snapshot.
 type Snapshot struct {
 	SnapshotID     string
 	SourceVolumeID string
@@ -244,19 +244,19 @@ type Snapshot struct {
 	ReadyToUse     bool
 }
 
-// ListSnapshotsResponse is the container for our snapshots along with a pagination token to pass back to the caller
+// ListSnapshotsResponse is the container for our snapshots along with a pagination token to pass back to the caller.
 type ListSnapshotsResponse struct {
 	Snapshots []*Snapshot
 	NextToken string
 }
 
-// SnapshotOptions represents parameters to create an EBS volume
+// SnapshotOptions represents parameters to create an EBS volume.
 type SnapshotOptions struct {
 	Tags       map[string]string
 	OutpostArn string
 }
 
-// ec2ListSnapshotsResponse is a helper struct returned from the AWS API calling function to the main ListSnapshots function
+// ec2ListSnapshotsResponse is a helper struct returned from the AWS API calling function to the main ListSnapshots function.
 type ec2ListSnapshotsResponse struct {
 	Snapshots []types.Snapshot
 	NextToken *string
@@ -333,7 +333,7 @@ type cloud struct {
 var _ Cloud = &cloud{}
 
 // NewCloud returns a new instance of AWS cloud
-// It panics if session is invalid
+// It panics if session is invalid.
 func NewCloud(region string, awsSdkDebugLog bool, userAgentExtra string, batching bool) (Cloud, error) {
 	c := newEC2Cloud(region, awsSdkDebugLog, userAgentExtra, batching)
 	return c, nil
@@ -705,7 +705,7 @@ func (c *cloud) CreateDisk(ctx context.Context, volumeName string, diskOptions *
 	return &Disk{CapacityGiB: size, VolumeID: volumeID, AvailabilityZone: zone, SnapshotID: snapshotID, OutpostArn: outpostArn}, nil
 }
 
-// execBatchDescribeVolumesModifications executes a batched DescribeVolumesModifications API call
+// execBatchDescribeVolumesModifications executes a batched DescribeVolumesModifications API call.
 func execBatchDescribeVolumesModifications(svc EC2API, input []string) (map[string]*types.VolumeModification, error) {
 	klog.V(7).InfoS("execBatchDescribeVolumeModifications", "volumeIds", input)
 	request := &ec2.DescribeVolumesModificationsInput{
@@ -861,7 +861,7 @@ func (c *cloud) DeleteDisk(ctx context.Context, volumeID string) (bool, error) {
 	return true, nil
 }
 
-// execBatchDescribeInstances executes a batched DescribeInstances API call
+// execBatchDescribeInstances executes a batched DescribeInstances API call.
 func execBatchDescribeInstances(svc EC2API, input []string) (map[string]*types.Instance, error) {
 	klog.V(7).InfoS("execBatchDescribeInstances", "instanceIds", input)
 	request := &ec2.DescribeInstancesInput{
@@ -1401,7 +1401,7 @@ func (c *cloud) ListSnapshots(ctx context.Context, volumeID string, maxResults i
 	}, nil
 }
 
-// Helper method converting EC2 snapshot type to the internal struct
+// Helper method converting EC2 snapshot type to the internal struct.
 func (c *cloud) ec2SnapshotResponseToStruct(ec2Snapshot types.Snapshot) *Snapshot {
 	snapshotSize := *ec2Snapshot.VolumeSize
 	snapshot := &Snapshot{
@@ -1557,7 +1557,7 @@ func (c *cloud) getSnapshot(ctx context.Context, request *ec2.DescribeSnapshotsI
 	}
 }
 
-// listSnapshots returns all snapshots based from a request
+// listSnapshots returns all snapshots based from a request.
 func (c *cloud) listSnapshots(ctx context.Context, request *ec2.DescribeSnapshotsInput) (*ec2ListSnapshotsResponse, error) {
 	var snapshots []types.Snapshot
 	var nextToken *string
@@ -1643,7 +1643,7 @@ func isAWSErrorInvalidAttachmentNotFound(err error) bool {
 }
 
 // isAWSErrorModificationNotFound returns a boolean indicating whether the given
-// error is an AWS InvalidVolumeModification.NotFound error
+// error is an AWS InvalidVolumeModification.NotFound error.
 func isAWSErrorModificationNotFound(err error) bool {
 	return isAWSError(err, "InvalidVolumeModification.NotFound")
 }
@@ -1657,7 +1657,7 @@ func isAWSErrorSnapshotNotFound(err error) bool {
 
 // isAWSErrorIdempotentParameterMismatch returns a boolean indicating whether the
 // given error is an AWS IdempotentParameterMismatch error.
-// This error is reported when the two request contains same client-token but different parameters
+// This error is reported when the two request contains same client-token but different parameters.
 func isAWSErrorIdempotentParameterMismatch(err error) bool {
 	return isAWSError(err, "IdempotentParameterMismatch")
 }
@@ -1794,7 +1794,7 @@ func (c *cloud) getLatestVolumeModification(ctx context.Context, volumeID string
 }
 
 // randomAvailabilityZone returns a random zone from the given region
-// the randomness relies on the response of DescribeAvailabilityZones
+// the randomness relies on the response of DescribeAvailabilityZones.
 func (c *cloud) randomAvailabilityZone(ctx context.Context) (string, error) {
 	request := &ec2.DescribeAvailabilityZonesInput{}
 	response, err := c.ec2.DescribeAvailabilityZones(ctx, request)
@@ -1810,7 +1810,7 @@ func (c *cloud) randomAvailabilityZone(ctx context.Context) (string, error) {
 	return zones[0], nil
 }
 
-// AvailabilityZones returns availability zones from the given region
+// AvailabilityZones returns availability zones from the given region.
 func (c *cloud) AvailabilityZones(ctx context.Context) (map[string]struct{}, error) {
 	response, err := c.ec2.DescribeAvailabilityZones(ctx, &ec2.DescribeAvailabilityZonesInput{})
 	if err != nil {

pkg/cloud/cloud_test.go

@@ -1451,7 +1451,7 @@ func TestCreateDisk(t *testing.T) {
 	}
 }
 
-// Test client error IdempotentParameterMismatch by forcing it to progress twice
+// Test client error IdempotentParameterMismatch by forcing it to progress twice.
 func TestCreateDiskClientToken(t *testing.T) {
 	t.Parallel()
 

pkg/cloud/devicemanager/allocator.go

@@ -46,7 +46,7 @@ var _ NameAllocator = &nameAllocator{}
 // It does this by using a list of legal EBS device names from device_names.go
 //
 // likelyBadNames is a map of names that have previously returned an "in use" error when attempting to mount to them
-// These names are unlikely to result in a successful mount, and may be permanently unavailable, so use them last
+// These names are unlikely to result in a successful mount, and may be permanently unavailable, so use them last.
 func (d *nameAllocator) GetNext(existingNames ExistingNames, likelyBadNames *sync.Map) (string, error) {
 	for _, name := range deviceNames {
 		_, existing := existingNames[name]

pkg/cloud/devicemanager/manager.go

@@ -44,7 +44,7 @@ func (d *Device) Release(force bool) {
 	}
 }
 
-// Taint marks the device as no longer reusable
+// Taint marks the device as no longer reusable.
 func (d *Device) Taint() {
 	d.isTainted = true
 }
@@ -194,7 +194,7 @@ func (d *deviceManager) release(device *Device) error {
 }
 
 // getDeviceNamesInUse returns the device to volume ID mapping
-// the mapping includes both already attached and being attached volumes
+// the mapping includes both already attached and being attached volumes.
 func (d *deviceManager) getDeviceNamesInUse(instance *types.Instance) map[string]string {
 	nodeID := aws.ToString(instance.InstanceId)
 	inUse := map[string]string{}

pkg/cloud/handlers.go

@@ -29,7 +29,7 @@ import (
 	"k8s.io/klog/v2"
 )
 
-// RecordRequestsHandler is added to the Complete chain; called after any request
+// RecordRequestsHandler is added to the Complete chain; called after any request.
 func RecordRequestsMiddleware() func(*middleware.Stack) error {
 	return func(stack *middleware.Stack) error {
 		return stack.Finalize.Add(middleware.FinalizeMiddlewareFunc("RecordRequestsMiddleware", func(ctx context.Context, input middleware.FinalizeInput, next middleware.FinalizeHandler) (output middleware.FinalizeOutput, metadata middleware.Metadata, err error) {
@@ -60,7 +60,7 @@ func RecordRequestsMiddleware() func(*middleware.Stack) error {
 
 // LogServerErrorsMiddleware is a middleware that logs server errors received when attempting to contact the AWS API
 // A specialized middleware is used instead of the SDK's built-in retry logging to allow for customizing the verbosity
-// of throttle errors vs server/unknown errors, to prevent flooding the logs with throttle error
+// of throttle errors vs server/unknown errors, to prevent flooding the logs with throttle error.
 func LogServerErrorsMiddleware() func(*middleware.Stack) error {
 	return func(stack *middleware.Stack) error {
 		return stack.Finalize.Add(middleware.FinalizeMiddlewareFunc("LogServerErrorsMiddleware", func(ctx context.Context, input middleware.FinalizeInput, next middleware.FinalizeHandler) (output middleware.FinalizeOutput, metadata middleware.Metadata, err error) {

pkg/cloud/metadata/ec2.go

@@ -29,13 +29,13 @@ import (
 )
 
 const (
-	// OutpostArnEndpoint is the ec2 instance metadata endpoint to query to get the outpost arn
+	// OutpostArnEndpoint is the ec2 instance metadata endpoint to query to get the outpost arn.
 	OutpostArnEndpoint string = "outpost-arn"
 
-	// enisEndpoint is the ec2 instance metadata endpoint to query the number of attached ENIs
+	// enisEndpoint is the ec2 instance metadata endpoint to query the number of attached ENIs.
 	EnisEndpoint string = "network/interfaces/macs"
 
-	// blockDevicesEndpoint is the ec2 instance metadata endpoint to query the number of attached block devices
+	// blockDevicesEndpoint is the ec2 instance metadata endpoint to query the number of attached block devices.
 	BlockDevicesEndpoint string = "block-device-mapping"
 )
 

pkg/cloud/metadata/metadata.go

@@ -24,7 +24,7 @@ import (
 	"k8s.io/klog/v2"
 )
 
-// Metadata is info about the ec2 instance on which the driver is running
+// Metadata is info about the ec2 instance on which the driver is running.
 type Metadata struct {
 	InstanceID             string
 	InstanceType           string
@@ -83,7 +83,7 @@ func retrieveK8sMetadata(k8sAPIClient KubernetesAPIClient) (*Metadata, error) {
 	return KubernetesAPIInstanceInfo(clientset)
 }
 
-// Override the region on a Metadata object if it is non-empty
+// Override the region on a Metadata object if it is non-empty.
 func (m *Metadata) overrideRegion(region string) *Metadata {
 	if region != "" {
 		m.Region = region

pkg/cloud/volume_limits.go

@@ -160,7 +160,7 @@ func GetReservedSlotsForInstanceType(it string) int {
 
 // https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-store-volumes.html
 // IMDS does not provide NVMe instance store data; we'll just list all instances here
-// g5.48xlarge is not added to this table as it is in the maxVolumeLimits
+// g5.48xlarge is not added to this table as it is in the maxVolumeLimits.
 var nvmeInstanceStoreVolumes = map[string]int{
 	"c1.medium":       1,
 	"c1.xlarge":       4,
@@ -513,7 +513,7 @@ var nvmeInstanceStoreVolumes = map[string]int{
 
 // https://aws.amazon.com/ec2/instance-types
 // Despite the dl1.24xlarge having Gaudi Accelerators describe instance types considers them GPUs as such that instance type is in this table
-// g5.48xlarge is not added to this table as it is in the maxVolumeLimits
+// g5.48xlarge is not added to this table as it is in the maxVolumeLimits.
 var gpuInstanceGpus = map[string]int{
 	"dl1.24xlarge":  8,
 	"g3.16xlarge":   4,