Skip to content

Commit 6234714

Browse files
mattcarymattcary
authored
Merge pull request #2186 from julianKatz/constants-in-own-package
Move constants.go into new pkg/constants
2 parents d285089 + 772b8d7 commit 6234714

22 files changed

+438
-421
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"k8s.io/klog/v2"
3131
"k8s.io/utils/strings/slices"
3232
"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/common"
33+
"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/constants"
3334
"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/deviceutils"
3435
gce "sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/gce-cloud-provider/compute"
3536
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 {
437438
return nil
438439
}
439440

440-
lssdCount := common.LocalSSDCountForDataCache
441-
if nodeName != common.TestNode {
441+
lssdCount := constants.LocalSSDCountForDataCache
442+
if nodeName != constants.TestNode {
442443
var err error
443444
lssdCount, err = driver.GetDataCacheCountFromNodeLabel(ctx, nodeName)
444445
if err != nil {

‎pkg/common/parameters.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"strings"
2323

2424
"k8s.io/klog/v2"
25+
"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/constants"
2526
)
2627

2728
const (
@@ -192,7 +193,7 @@ func (pp *ParameterProcessor) ExtractAndDefaultParameters(parameters map[string]
192193
// Set data cache mode default
193194
d := DataCacheParameters{}
194195
if enableDataCache && parameters[ParameterKeyDataCacheSize] != "" {
195-
d.DataCacheMode = DataCacheModeWriteThrough
196+
d.DataCacheMode = constants.DataCacheModeWriteThrough
196197
}
197198

198199
for k, v := range extraVolumeLabels {
@@ -320,7 +321,7 @@ func (pp *ParameterProcessor) ExtractAndDefaultParameters(parameters map[string]
320321

321322
p.MultiZoneProvisioning = paramEnableMultiZoneProvisioning
322323
if paramEnableMultiZoneProvisioning {
323-
p.Labels[MultiZoneLabel] = "true"
324+
p.Labels[constants.MultiZoneLabel] = "true"
324325
}
325326
case ParameterAccessMode:
326327
if v != "" {

‎pkg/common/parameters_test.go‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"testing"
2222

2323
"github.com/google/go-cmp/cmp"
24+
"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/constants"
2425
)
2526

2627
func TestExtractAndDefaultParameters(t *testing.T) {
@@ -380,14 +381,14 @@ func TestExtractAndDefaultParameters(t *testing.T) {
380381
ResourceTags: map[string]string{},
381382
},
382383
expectDataCacheParams: DataCacheParameters{
383-
DataCacheMode: DataCacheModeWriteThrough,
384+
DataCacheMode: constants.DataCacheModeWriteThrough,
384385
DataCacheSize: "1234",
385386
},
386387
},
387388
{
388389
name: "data cache parameters",
389390
enableDataCache: true,
390-
parameters: map[string]string{ParameterKeyType: "pd-balanced", ParameterKeyReplicationType: "none", ParameterKeyDiskEncryptionKmsKey: "foo/key", ParameterKeyLabels: "key1=value1,key2=value2", ParameterKeyDataCacheSize: "1234Gi", ParameterKeyDataCacheMode: DataCacheModeWriteBack},
391+
parameters: map[string]string{ParameterKeyType: "pd-balanced", ParameterKeyReplicationType: "none", ParameterKeyDiskEncryptionKmsKey: "foo/key", ParameterKeyLabels: "key1=value1,key2=value2", ParameterKeyDataCacheSize: "1234Gi", ParameterKeyDataCacheMode: constants.DataCacheModeWriteBack},
391392
labels: map[string]string{},
392393
expectParams: DiskParameters{
393394
DiskType: "pd-balanced",
@@ -401,27 +402,27 @@ func TestExtractAndDefaultParameters(t *testing.T) {
401402
ResourceTags: map[string]string{},
402403
},
403404
expectDataCacheParams: DataCacheParameters{
404-
DataCacheMode: DataCacheModeWriteBack,
405+
DataCacheMode: constants.DataCacheModeWriteBack,
405406
DataCacheSize: "1234",
406407
},
407408
},
408409
{
409410
name: "data cache parameters - enableDataCache is false",
410411
enableDataCache: false,
411-
parameters: map[string]string{ParameterKeyType: "pd-balanced", ParameterKeyReplicationType: "none", ParameterKeyDiskEncryptionKmsKey: "foo/key", ParameterKeyLabels: "key1=value1,key2=value2", ParameterKeyDataCacheSize: "1234Gi", ParameterKeyDataCacheMode: DataCacheModeWriteBack},
412+
parameters: map[string]string{ParameterKeyType: "pd-balanced", ParameterKeyReplicationType: "none", ParameterKeyDiskEncryptionKmsKey: "foo/key", ParameterKeyLabels: "key1=value1,key2=value2", ParameterKeyDataCacheSize: "1234Gi", ParameterKeyDataCacheMode: constants.DataCacheModeWriteBack},
412413
labels: map[string]string{},
413414
expectErr: true,
414415
},
415416
{
416417
name: "multi-zone-enable parameters, multi-zone label is set, multi-zone feature enabled",
417418
parameters: map[string]string{ParameterKeyType: "hyperdisk-ml", ParameterKeyEnableMultiZoneProvisioning: "true"},
418-
labels: map[string]string{MultiZoneLabel: "true"},
419+
labels: map[string]string{constants.MultiZoneLabel: "true"},
419420
enableMultiZone: true,
420421
expectParams: DiskParameters{
421422
DiskType: "hyperdisk-ml",
422423
ReplicationType: "none",
423424
Tags: map[string]string{},
424-
Labels: map[string]string{MultiZoneLabel: "true"},
425+
Labels: map[string]string{constants.MultiZoneLabel: "true"},
425426
ResourceTags: map[string]string{},
426427
MultiZoneProvisioning: true,
427428
},

‎pkg/common/utils.go‎

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import (
3838
"k8s.io/apimachinery/pkg/util/sets"
3939
volumehelpers "k8s.io/cloud-provider/volume/helpers"
4040
"k8s.io/klog/v2"
41+
"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/constants"
4142
)
4243

4344
const (
@@ -110,7 +111,7 @@ var (
110111
http.StatusConflict: codes.FailedPrecondition,
111112
}
112113

113-
validDataCacheMode = []string{DataCacheModeWriteBack, DataCacheModeWriteThrough}
114+
validDataCacheMode = []string{constants.DataCacheModeWriteBack, constants.DataCacheModeWriteThrough}
114115

115116
// Regular expressions for validating parent_id, key and value of a resource tag.
116117
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) {
165166

166167
func GenerateUnderspecifiedVolumeID(diskName string, isZonal bool) string {
167168
if isZonal {
168-
return fmt.Sprintf(volIDZonalFmt, UnspecifiedValue, UnspecifiedValue, diskName)
169+
return fmt.Sprintf(volIDZonalFmt, constants.UnspecifiedValue, constants.UnspecifiedValue, diskName)
169170
}
170-
return fmt.Sprintf(volIDRegionalFmt, UnspecifiedValue, UnspecifiedValue, diskName)
171+
return fmt.Sprintf(volIDRegionalFmt, constants.UnspecifiedValue, constants.UnspecifiedValue, diskName)
171172
}
172173

173174
func SnapshotIDToProjectKey(id string) (string, string, string, error) {
@@ -705,7 +706,7 @@ func VolumeIdAsMultiZone(volumeId string) (string, error) {
705706
if splitId[volIDToplogyKey] != "zones" {
706707
return "", fmt.Errorf("expected id to be zonal. Got: %s", volumeId)
707708
}
708-
splitId[volIDToplogyValue] = MultiZoneValue
709+
splitId[volIDToplogyValue] = constants.MultiZoneValue
709710
return strings.Join(splitId, "/"), nil
710711
}
711712

@@ -764,43 +765,43 @@ func ShortString(s string) string {
764765

765766
// GetHyperdiskAttachLimit returns the hyperdisk attach limit based on machine type prefix and vCPUs
766767
func GetHyperdiskAttachLimit(machineTypePrefix string, vCPUs int64) int64 {
767-
var limitMap []MachineHyperdiskLimit
768+
var limitMap []constants.MachineHyperdiskLimit
768769

769770
switch machineTypePrefix {
770771
case "c4":
771-
limitMap = C4MachineHyperdiskAttachLimitMap
772+
limitMap = constants.C4MachineHyperdiskAttachLimitMap
772773
case "c4d":
773-
limitMap = C4DMachineHyperdiskAttachLimitMap
774+
limitMap = constants.C4DMachineHyperdiskAttachLimitMap
774775
case "n4":
775-
limitMap = N4MachineHyperdiskAttachLimitMap
776+
limitMap = constants.N4MachineHyperdiskAttachLimitMap
776777
case "c4a":
777-
limitMap = C4AMachineHyperdiskAttachLimitMap
778+
limitMap = constants.C4AMachineHyperdiskAttachLimitMap
778779
case "a4x":
779-
limitMap = A4XMachineHyperdiskAttachLimitMap
780+
limitMap = constants.A4XMachineHyperdiskAttachLimitMap
780781
default:
781782
// Fallback to the most conservative Gen4 map for unknown types
782-
return MapNumber(vCPUs, C4DMachineHyperdiskAttachLimitMap)
783+
return MapNumber(vCPUs, constants.C4DMachineHyperdiskAttachLimitMap)
783784
}
784785

785786
return MapNumber(vCPUs, limitMap)
786787
}
787788

788789
// mapNumber maps the vCPUs to the appropriate hyperdisk limit
789-
func MapNumber(vCPUs int64, limitMap []MachineHyperdiskLimit) int64 {
790+
func MapNumber(vCPUs int64, limitMap []constants.MachineHyperdiskLimit) int64 {
790791
for _, limit := range limitMap {
791-
if vCPUs <= limit.max {
792-
return limit.value
792+
if vCPUs <= limit.Max {
793+
return limit.Value
793794
}
794795
}
795796
// Return the last value if vCPUs exceeds all max values
796797
if len(limitMap) > 0 {
797-
return limitMap[len(limitMap)-1].value
798+
return limitMap[len(limitMap)-1].Value
798799
}
799800
return 15
800801
}
801802

802803
func DiskTypeLabelKey(diskType string) string {
803-
return fmt.Sprintf("%s/%s", DiskTypeKeyPrefix, diskType)
804+
return fmt.Sprintf("%s/%s", constants.DiskTypeKeyPrefix, diskType)
804805
}
805806

806807
// IsUpdateIopsThroughputValuesAllowed checks if a disk type is hyperdisk,

‎pkg/common/volume_lock.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,6 @@ import (
2222
"k8s.io/apimachinery/pkg/util/sets"
2323
)
2424

25-
const (
26-
VolumeOperationAlreadyExistsFmt = "An operation with the given Volume ID %s already exists"
27-
)
28-
2925
// VolumeLocks implements a map with atomic operations. It stores a set of all volume IDs
3026
// with an ongoing operation.
3127
type VolumeLocks struct {

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

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
package common
17+
package constants
1818

1919
const (
2020
// Keys for Topology. This key will be shared amongst drivers from GCP
@@ -29,6 +29,9 @@ const (
2929

3030
UnspecifiedValue = "UNSPECIFIED"
3131

32+
// VolumeOperationAlreadyExistsFmt is the error message format for when a volume operation already exists
33+
VolumeOperationAlreadyExistsFmt = "An operation with the given Volume ID %s already exists"
34+
3235
// Keyword indicating a 'multi-zone' volumeHandle. Replaces "zones" in the volumeHandle:
3336
// eg: projects/{project}/zones/multi-zone/disks/{name} vs.
3437
// projects/{project}/zones/{zone}/disks/{name}
@@ -69,45 +72,45 @@ const (
6972
// doc https://cloud.google.com/compute/docs/general-purpose-machines
7073
// MachineHyperdiskLimit represents the mapping between max vCPUs and hyperdisk (balanced) attach limit
7174
type MachineHyperdiskLimit struct {
72-
max int64
73-
value int64
75+
Max int64
76+
Value int64
7477
}
7578

7679
// C4 Machine Types - Hyperdisk Balanced Limits
7780
var C4MachineHyperdiskAttachLimitMap = []MachineHyperdiskLimit{
78-
{max: 2, value: 7},
79-
{max: 4, value: 15},
80-
{max: 24, value: 31},
81-
{max: 48, value: 63},
82-
{max: 96, value: 127},
81+
{Max: 2, Value: 7},
82+
{Max: 4, Value: 15},
83+
{Max: 24, Value: 31},
84+
{Max: 48, Value: 63},
85+
{Max: 96, Value: 127},
8386
}
8487

8588
// C4D Machine Types - Hyperdisk Balanced Limits
8689
var C4DMachineHyperdiskAttachLimitMap = []MachineHyperdiskLimit{
87-
{max: 2, value: 3},
88-
{max: 4, value: 7},
89-
{max: 8, value: 15},
90-
{max: 96, value: 31},
91-
{max: 192, value: 63},
92-
{max: 384, value: 127},
90+
{Max: 2, Value: 3},
91+
{Max: 4, Value: 7},
92+
{Max: 8, Value: 15},
93+
{Max: 96, Value: 31},
94+
{Max: 192, Value: 63},
95+
{Max: 384, Value: 127},
9396
}
9497

9598
// N4 Machine Types - Hyperdisk Balanced Limits
9699
var N4MachineHyperdiskAttachLimitMap = []MachineHyperdiskLimit{
97-
{max: 8, value: 15},
98-
{max: 80, value: 31},
100+
{Max: 8, Value: 15},
101+
{Max: 80, Value: 31},
99102
}
100103

101104
// C4A Machine Types - Hyperdisk Balanced Limits
102105
var C4AMachineHyperdiskAttachLimitMap = []MachineHyperdiskLimit{
103-
{max: 2, value: 7},
104-
{max: 8, value: 15},
105-
{max: 48, value: 31},
106-
{max: 72, value: 63},
106+
{Max: 2, Value: 7},
107+
{Max: 8, Value: 15},
108+
{Max: 48, Value: 31},
109+
{Max: 72, Value: 63},
107110
}
108111

109112
// A4X Machine Types - Hyperdisk Balanced Limits. The max here is actually the GPU count (not CPU, like the others).
110113
var A4XMachineHyperdiskAttachLimitMap = []MachineHyperdiskLimit{
111-
{max: 1, value: 63},
112-
{max: 2, value: 127},
114+
{Max: 1, Value: 63},
115+
{Max: 2, Value: 127},
113116
}

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"google.golang.org/grpc/status"
3131
"k8s.io/klog/v2"
3232
"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/common"
33+
"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/constants"
3334

3435
"k8s.io/apimachinery/pkg/util/sets"
3536
)
@@ -102,12 +103,12 @@ func (cloud *FakeCloudProvider) GetDefaultZone() string {
102103
}
103104

104105
func (cloud *FakeCloudProvider) RepairUnderspecifiedVolumeKey(ctx context.Context, project string, volumeKey *meta.Key) (string, *meta.Key, error) {
105-
if project == common.UnspecifiedValue {
106+
if project == constants.UnspecifiedValue {
106107
project = cloud.project
107108
}
108109
switch volumeKey.Type() {
109110
case meta.Zonal:
110-
if volumeKey.Zone != common.UnspecifiedValue {
111+
if volumeKey.Zone != constants.UnspecifiedValue {
111112
return project, volumeKey, nil
112113
}
113114
for diskVolKey, d := range cloud.disks {
@@ -118,7 +119,7 @@ func (cloud *FakeCloudProvider) RepairUnderspecifiedVolumeKey(ctx context.Contex
118119
}
119120
return "", nil, notFoundError()
120121
case meta.Regional:
121-
if volumeKey.Region != common.UnspecifiedValue {
122+
if volumeKey.Region != constants.UnspecifiedValue {
122123
return project, volumeKey, nil
123124
}
124125
r, err := common.GetRegionFromZones([]string{cloud.zone})

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import (
4141
"k8s.io/klog/v2"
4242
"k8s.io/utils/strings/slices"
4343
"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/common"
44+
"sigs.k8s.io/gcp-compute-persistent-disk-csi-driver/pkg/constants"
4445
)
4546

4647
const (
@@ -292,7 +293,7 @@ func (cloud *CloudProvider) listInstancesForProject(service *computev1.Service,
292293
// by the volume key and return a volume key with a correct zone
293294
func (cloud *CloudProvider) RepairUnderspecifiedVolumeKey(ctx context.Context, project string, volumeKey *meta.Key) (string, *meta.Key, error) {
294295
klog.V(5).Infof("Repairing potentially underspecified volume key %v", volumeKey)
295-
if project == common.UnspecifiedValue {
296+
if project == constants.UnspecifiedValue {
296297
project = cloud.project
297298
}
298299
region, err := common.GetRegionFromZones([]string{cloud.zone})
@@ -302,7 +303,7 @@ func (cloud *CloudProvider) RepairUnderspecifiedVolumeKey(ctx context.Context, p
302303
switch volumeKey.Type() {
303304
case meta.Zonal:
304305
foundZone := ""
305-
if volumeKey.Zone == common.UnspecifiedValue {
306+
if volumeKey.Zone == constants.UnspecifiedValue {
306307
// list all zones, try to get disk in each zone
307308
zones, err := cloud.ListZones(ctx, region)
308309
if err != nil {
@@ -334,7 +335,7 @@ func (cloud *CloudProvider) RepairUnderspecifiedVolumeKey(ctx context.Context, p
334335
}
335336
return project, volumeKey, nil
336337
case meta.Regional:
337-
if volumeKey.Region == common.UnspecifiedValue {
338+
if volumeKey.Region == constants.UnspecifiedValue {
338339
volumeKey.Region = region
339340
}
340341
return project, volumeKey, nil
@@ -468,8 +469,8 @@ func validAccessMode(want, got string) bool {
468469
return true
469470
}
470471
switch want {
471-
case common.GCEReadOnlyManyAccessMode, common.GCEReadWriteOnceAccessMode:
472-
return got == common.GCEReadWriteManyAccessMode
472+
case constants.GCEReadOnlyManyAccessMode, constants.GCEReadWriteOnceAccessMode:
473+
return got == constants.GCEReadWriteManyAccessMode
473474
// For RWX, no other access mode is valid.
474475
default:
475476
return false

0 commit comments

Comments
 (0)