Merge pull request #2186 from julianKatz/constants-in-own-package

Move constants.go into new pkg/constants

Author: mattcary

cmd/gce-pd-csi-driver/main.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/klog/v2"
 	"k8s.io/utils/strings/slices"
 	"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/common"
+	"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/constants"
 	"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/deviceutils"
 	gce "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/gce-cloud-provider/compute"
 	metadataservice "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/gce-cloud-provider/metadata"
@@ -437,8 +438,8 @@ func setupDataCache(ctx context.Context, nodeName string, nodeId string) error {
 		return nil
 	}
 
-	lssdCount := common.LocalSSDCountForDataCache
-	if nodeName != common.TestNode {
+	lssdCount := constants.LocalSSDCountForDataCache
+	if nodeName != constants.TestNode {
 		var err error
 		lssdCount, err = driver.GetDataCacheCountFromNodeLabel(ctx, nodeName)
 		if err != nil {

pkg/common/parameters.go

@@ -22,6 +22,7 @@ import (
 	"strings"
 
 	"k8s.io/klog/v2"
+	"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/constants"
 )
 
 const (
@@ -192,7 +193,7 @@ func (pp *ParameterProcessor) ExtractAndDefaultParameters(parameters map[string]
 	// Set data cache mode default
 	d := DataCacheParameters{}
 	if enableDataCache && parameters[ParameterKeyDataCacheSize] != "" {
-		d.DataCacheMode = DataCacheModeWriteThrough
+		d.DataCacheMode = constants.DataCacheModeWriteThrough
 	}
 
 	for k, v := range extraVolumeLabels {
@@ -320,7 +321,7 @@ func (pp *ParameterProcessor) ExtractAndDefaultParameters(parameters map[string]
 
 			p.MultiZoneProvisioning = paramEnableMultiZoneProvisioning
 			if paramEnableMultiZoneProvisioning {
-				p.Labels[MultiZoneLabel] = "true"
+				p.Labels[constants.MultiZoneLabel] = "true"
 			}
 		case ParameterAccessMode:
 			if v != "" {

pkg/common/parameters_test.go

@@ -21,6 +21,7 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/constants"
 )
 
 func TestExtractAndDefaultParameters(t *testing.T) {
@@ -380,14 +381,14 @@ func TestExtractAndDefaultParameters(t *testing.T) {
 				ResourceTags: map[string]string{},
 			},
 			expectDataCacheParams: DataCacheParameters{
-				DataCacheMode: DataCacheModeWriteThrough,
+				DataCacheMode: constants.DataCacheModeWriteThrough,
 				DataCacheSize: "1234",
 			},
 		},
 		{
 			name:            "data cache parameters",
 			enableDataCache: true,
-			parameters:      map[string]string{ParameterKeyType: "pd-balanced", ParameterKeyReplicationType: "none", ParameterKeyDiskEncryptionKmsKey: "foo/key", ParameterKeyLabels: "key1=value1,key2=value2", ParameterKeyDataCacheSize: "1234Gi", ParameterKeyDataCacheMode: DataCacheModeWriteBack},
+			parameters:      map[string]string{ParameterKeyType: "pd-balanced", ParameterKeyReplicationType: "none", ParameterKeyDiskEncryptionKmsKey: "foo/key", ParameterKeyLabels: "key1=value1,key2=value2", ParameterKeyDataCacheSize: "1234Gi", ParameterKeyDataCacheMode: constants.DataCacheModeWriteBack},
 			labels:          map[string]string{},
 			expectParams: DiskParameters{
 				DiskType:             "pd-balanced",
@@ -401,27 +402,27 @@ func TestExtractAndDefaultParameters(t *testing.T) {
 				ResourceTags: map[string]string{},
 			},
 			expectDataCacheParams: DataCacheParameters{
-				DataCacheMode: DataCacheModeWriteBack,
+				DataCacheMode: constants.DataCacheModeWriteBack,
 				DataCacheSize: "1234",
 			},
 		},
 		{
 			name:            "data cache parameters - enableDataCache is false",
 			enableDataCache: false,
-			parameters:      map[string]string{ParameterKeyType: "pd-balanced", ParameterKeyReplicationType: "none", ParameterKeyDiskEncryptionKmsKey: "foo/key", ParameterKeyLabels: "key1=value1,key2=value2", ParameterKeyDataCacheSize: "1234Gi", ParameterKeyDataCacheMode: DataCacheModeWriteBack},
+			parameters:      map[string]string{ParameterKeyType: "pd-balanced", ParameterKeyReplicationType: "none", ParameterKeyDiskEncryptionKmsKey: "foo/key", ParameterKeyLabels: "key1=value1,key2=value2", ParameterKeyDataCacheSize: "1234Gi", ParameterKeyDataCacheMode: constants.DataCacheModeWriteBack},
 			labels:          map[string]string{},
 			expectErr:       true,
 		},
 		{
 			name:            "multi-zone-enable parameters, multi-zone label is set, multi-zone feature enabled",
 			parameters:      map[string]string{ParameterKeyType: "hyperdisk-ml", ParameterKeyEnableMultiZoneProvisioning: "true"},
-			labels:          map[string]string{MultiZoneLabel: "true"},
+			labels:          map[string]string{constants.MultiZoneLabel: "true"},
 			enableMultiZone: true,
 			expectParams: DiskParameters{
 				DiskType:              "hyperdisk-ml",
 				ReplicationType:       "none",
 				Tags:                  map[string]string{},
-				Labels:                map[string]string{MultiZoneLabel: "true"},
+				Labels:                map[string]string{constants.MultiZoneLabel: "true"},
 				ResourceTags:          map[string]string{},
 				MultiZoneProvisioning: true,
 			},

pkg/common/utils.go

@@ -38,6 +38,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/sets"
 	volumehelpers "k8s.io/cloud-provider/volume/helpers"
 	"k8s.io/klog/v2"
+	"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/constants"
 )
 
 const (
@@ -110,7 +111,7 @@ var (
 		http.StatusConflict:        codes.FailedPrecondition,
 	}
 
-	validDataCacheMode = []string{DataCacheModeWriteBack, DataCacheModeWriteThrough}
+	validDataCacheMode = []string{constants.DataCacheModeWriteBack, constants.DataCacheModeWriteThrough}
 
 	// Regular expressions for validating parent_id, key and value of a resource tag.
 	regexParent = regexp.MustCompile(`(^[1-9][0-9]{0,31}$)|(^[a-z][a-z0-9-]{4,28}[a-z0-9]$)`)
@@ -165,9 +166,9 @@ func KeyToVolumeID(volKey *meta.Key, project string) (string, error) {
 
 func GenerateUnderspecifiedVolumeID(diskName string, isZonal bool) string {
 	if isZonal {
-		return fmt.Sprintf(volIDZonalFmt, UnspecifiedValue, UnspecifiedValue, diskName)
+		return fmt.Sprintf(volIDZonalFmt, constants.UnspecifiedValue, constants.UnspecifiedValue, diskName)
 	}
-	return fmt.Sprintf(volIDRegionalFmt, UnspecifiedValue, UnspecifiedValue, diskName)
+	return fmt.Sprintf(volIDRegionalFmt, constants.UnspecifiedValue, constants.UnspecifiedValue, diskName)
 }
 
 func SnapshotIDToProjectKey(id string) (string, string, string, error) {
@@ -705,7 +706,7 @@ func VolumeIdAsMultiZone(volumeId string) (string, error) {
 	if splitId[volIDToplogyKey] != "zones" {
 		return "", fmt.Errorf("expected id to be zonal. Got: %s", volumeId)
 	}
-	splitId[volIDToplogyValue] = MultiZoneValue
+	splitId[volIDToplogyValue] = constants.MultiZoneValue
 	return strings.Join(splitId, "/"), nil
 }
 
@@ -764,43 +765,43 @@ func ShortString(s string) string {
 
 // GetHyperdiskAttachLimit returns the hyperdisk attach limit based on machine type prefix and vCPUs
 func GetHyperdiskAttachLimit(machineTypePrefix string, vCPUs int64) int64 {
-	var limitMap []MachineHyperdiskLimit
+	var limitMap []constants.MachineHyperdiskLimit
 
 	switch machineTypePrefix {
 	case "c4":
-		limitMap = C4MachineHyperdiskAttachLimitMap
+		limitMap = constants.C4MachineHyperdiskAttachLimitMap
 	case "c4d":
-		limitMap = C4DMachineHyperdiskAttachLimitMap
+		limitMap = constants.C4DMachineHyperdiskAttachLimitMap
 	case "n4":
-		limitMap = N4MachineHyperdiskAttachLimitMap
+		limitMap = constants.N4MachineHyperdiskAttachLimitMap
 	case "c4a":
-		limitMap = C4AMachineHyperdiskAttachLimitMap
+		limitMap = constants.C4AMachineHyperdiskAttachLimitMap
 	case "a4x":
-		limitMap = A4XMachineHyperdiskAttachLimitMap
+		limitMap = constants.A4XMachineHyperdiskAttachLimitMap
 	default:
 		// Fallback to the most conservative Gen4 map for unknown types
-		return MapNumber(vCPUs, C4DMachineHyperdiskAttachLimitMap)
+		return MapNumber(vCPUs, constants.C4DMachineHyperdiskAttachLimitMap)
 	}
 
 	return MapNumber(vCPUs, limitMap)
 }
 
 // mapNumber maps the vCPUs to the appropriate hyperdisk limit
-func MapNumber(vCPUs int64, limitMap []MachineHyperdiskLimit) int64 {
+func MapNumber(vCPUs int64, limitMap []constants.MachineHyperdiskLimit) int64 {
 	for _, limit := range limitMap {
-		if vCPUs <= limit.max {
-			return limit.value
+		if vCPUs <= limit.Max {
+			return limit.Value
 		}
 	}
 	// Return the last value if vCPUs exceeds all max values
 	if len(limitMap) > 0 {
-		return limitMap[len(limitMap)-1].value
+		return limitMap[len(limitMap)-1].Value
 	}
 	return 15
 }
 
 func DiskTypeLabelKey(diskType string) string {
-	return fmt.Sprintf("%s/%s", DiskTypeKeyPrefix, diskType)
+	return fmt.Sprintf("%s/%s", constants.DiskTypeKeyPrefix, diskType)
 }
 
 // IsUpdateIopsThroughputValuesAllowed checks if a disk type is hyperdisk,

pkg/common/volume_lock.go

@@ -22,10 +22,6 @@ import (
 	"k8s.io/apimachinery/pkg/util/sets"
 )
 
-const (
-	VolumeOperationAlreadyExistsFmt = "An operation with the given Volume ID %s already exists"
-)
-
 // VolumeLocks implements a map with atomic operations. It stores a set of all volume IDs
 // with an ongoing operation.
 type VolumeLocks struct {

pkg/common/constants.go pkg/constants/constants.gopkg/common/constants.go renamed to pkg/constants/constants.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package common
+package constants
 
 const (
 	// Keys for Topology. This key will be shared amongst drivers from GCP
@@ -29,6 +29,9 @@ const (
 
 	UnspecifiedValue = "UNSPECIFIED"
 
+	// VolumeOperationAlreadyExistsFmt is the error message format for when a volume operation already exists
+	VolumeOperationAlreadyExistsFmt = "An operation with the given Volume ID %s already exists"
+
 	// Keyword indicating a 'multi-zone' volumeHandle. Replaces "zones" in the volumeHandle:
 	// eg: projects/{project}/zones/multi-zone/disks/{name} vs.
 	// projects/{project}/zones/{zone}/disks/{name}
@@ -69,45 +72,45 @@ const (
 // doc https://cloud.google.com/compute/docs/general-purpose-machines
 // MachineHyperdiskLimit represents the mapping between max vCPUs and hyperdisk (balanced) attach limit
 type MachineHyperdiskLimit struct {
-	max   int64
-	value int64
+	Max   int64
+	Value int64
 }
 
 // C4 Machine Types - Hyperdisk Balanced Limits
 var C4MachineHyperdiskAttachLimitMap = []MachineHyperdiskLimit{
-	{max: 2, value: 7},
-	{max: 4, value: 15},
-	{max: 24, value: 31},
-	{max: 48, value: 63},
-	{max: 96, value: 127},
+	{Max: 2, Value: 7},
+	{Max: 4, Value: 15},
+	{Max: 24, Value: 31},
+	{Max: 48, Value: 63},
+	{Max: 96, Value: 127},
 }
 
 // C4D Machine Types - Hyperdisk Balanced Limits
 var C4DMachineHyperdiskAttachLimitMap = []MachineHyperdiskLimit{
-	{max: 2, value: 3},
-	{max: 4, value: 7},
-	{max: 8, value: 15},
-	{max: 96, value: 31},
-	{max: 192, value: 63},
-	{max: 384, value: 127},
+	{Max: 2, Value: 3},
+	{Max: 4, Value: 7},
+	{Max: 8, Value: 15},
+	{Max: 96, Value: 31},
+	{Max: 192, Value: 63},
+	{Max: 384, Value: 127},
 }
 
 // N4 Machine Types - Hyperdisk Balanced Limits
 var N4MachineHyperdiskAttachLimitMap = []MachineHyperdiskLimit{
-	{max: 8, value: 15},
-	{max: 80, value: 31},
+	{Max: 8, Value: 15},
+	{Max: 80, Value: 31},
 }
 
 // C4A Machine Types - Hyperdisk Balanced Limits
 var C4AMachineHyperdiskAttachLimitMap = []MachineHyperdiskLimit{
-	{max: 2, value: 7},
-	{max: 8, value: 15},
-	{max: 48, value: 31},
-	{max: 72, value: 63},
+	{Max: 2, Value: 7},
+	{Max: 8, Value: 15},
+	{Max: 48, Value: 31},
+	{Max: 72, Value: 63},
 }
 
 // A4X Machine Types - Hyperdisk Balanced Limits. The max here is actually the GPU count (not CPU, like the others).
 var A4XMachineHyperdiskAttachLimitMap = []MachineHyperdiskLimit{
-	{max: 1, value: 63},
-	{max: 2, value: 127},
+	{Max: 1, Value: 63},
+	{Max: 2, Value: 127},
 }

pkg/gce-cloud-provider/compute/fake-gce.go

@@ -30,6 +30,7 @@ import (
 	"google.golang.org/grpc/status"
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/common"
+	"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/constants"
 
 	"k8s.io/apimachinery/pkg/util/sets"
 )
@@ -102,12 +103,12 @@ func (cloud *FakeCloudProvider) GetDefaultZone() string {
 }
 
 func (cloud *FakeCloudProvider) RepairUnderspecifiedVolumeKey(ctx context.Context, project string, volumeKey *meta.Key) (string, *meta.Key, error) {
-	if project == common.UnspecifiedValue {
+	if project == constants.UnspecifiedValue {
 		project = cloud.project
 	}
 	switch volumeKey.Type() {
 	case meta.Zonal:
-		if volumeKey.Zone != common.UnspecifiedValue {
+		if volumeKey.Zone != constants.UnspecifiedValue {
 			return project, volumeKey, nil
 		}
 		for diskVolKey, d := range cloud.disks {
@@ -118,7 +119,7 @@ func (cloud *FakeCloudProvider) RepairUnderspecifiedVolumeKey(ctx context.Contex
 		}
 		return "", nil, notFoundError()
 	case meta.Regional:
-		if volumeKey.Region != common.UnspecifiedValue {
+		if volumeKey.Region != constants.UnspecifiedValue {
 			return project, volumeKey, nil
 		}
 		r, err := common.GetRegionFromZones([]string{cloud.zone})

pkg/gce-cloud-provider/compute/gce-compute.go

@@ -41,6 +41,7 @@ import (
 	"k8s.io/klog/v2"
 	"k8s.io/utils/strings/slices"
 	"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/common"
+	"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/constants"
 )
 
 const (
@@ -292,7 +293,7 @@ func (cloud *CloudProvider) listInstancesForProject(service *computev1.Service,
 // by the volume key and return a volume key with a correct zone
 func (cloud *CloudProvider) RepairUnderspecifiedVolumeKey(ctx context.Context, project string, volumeKey *meta.Key) (string, *meta.Key, error) {
 	klog.V(5).Infof("Repairing potentially underspecified volume key %v", volumeKey)
-	if project == common.UnspecifiedValue {
+	if project == constants.UnspecifiedValue {
 		project = cloud.project
 	}
 	region, err := common.GetRegionFromZones([]string{cloud.zone})
@@ -302,7 +303,7 @@ func (cloud *CloudProvider) RepairUnderspecifiedVolumeKey(ctx context.Context, p
 	switch volumeKey.Type() {
 	case meta.Zonal:
 		foundZone := ""
-		if volumeKey.Zone == common.UnspecifiedValue {
+		if volumeKey.Zone == constants.UnspecifiedValue {
 			// list all zones, try to get disk in each zone
 			zones, err := cloud.ListZones(ctx, region)
 			if err != nil {
@@ -334,7 +335,7 @@ func (cloud *CloudProvider) RepairUnderspecifiedVolumeKey(ctx context.Context, p
 		}
 		return project, volumeKey, nil
 	case meta.Regional:
-		if volumeKey.Region == common.UnspecifiedValue {
+		if volumeKey.Region == constants.UnspecifiedValue {
 			volumeKey.Region = region
 		}
 		return project, volumeKey, nil
@@ -468,8 +469,8 @@ func validAccessMode(want, got string) bool {
 		return true
 	}
 	switch want {
-	case common.GCEReadOnlyManyAccessMode, common.GCEReadWriteOnceAccessMode:
-		return got == common.GCEReadWriteManyAccessMode
+	case constants.GCEReadOnlyManyAccessMode, constants.GCEReadWriteOnceAccessMode:
+		return got == constants.GCEReadWriteManyAccessMode
 	// For RWX, no other access mode is valid.
 	default:
 		return false