Skip to content

Add test support for regional PD #621

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 12, 2020
Merged

Add test support for regional PD #621

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions test/k8s-integration/cluster.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ func gkeLocationArgs(gceZone, gceRegion string) (locationArg, locationVal string
return
}

func isRegionalGKECluster(gceZone, gceRegion string) bool {
return len(gceRegion) > 0
}

func clusterDownGCE(k8sDir string) error {
cmd := exec.Command(filepath.Join(k8sDir, "hack", "e2e-internal", "e2e-down.sh"))
err := runCommand("Bringing Down E2E Cluster on GCE", cmd)
Expand Down
9 changes: 9 additions & 0 deletions test/k8s-integration/config/sc-regional.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
apiVersion: storage.k8s.io/v1
kind: StorageClass
metadata:
name: csi-gcepd-regional
provisioner: pd.csi.storage.gke.io
parameters:
type: pd-standard
replication-type: regional-pd
volumeBindingMode: WaitForFirstConsumer
3 changes: 2 additions & 1 deletion test/k8s-integration/config/test-config-template.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ DriverInfo:
debug:
nouid32:
SupportedSizeRange:
Min: 5Gi
Min: {{.MinimumVolumeSize}}
Max: 64Ti
TopologyKeys:
- topology.gke.io/zone
NumAllowedTopologies: {{.NumAllowedTopologies}}
30 changes: 20 additions & 10 deletions test/k8s-integration/driver-config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,13 @@ import (
)

type driverConfig struct {
StorageClassFile string
StorageClass string
SnapshotClassFile string
Capabilities []string
SupportedFsType []string
StorageClassFile string
StorageClass string
SnapshotClassFile string
Capabilities []string
SupportedFsType []string
MinimumVolumeSize string
NumAllowedTopologies int
}

const (
Expand Down Expand Up @@ -98,12 +100,20 @@ func generateDriverConfigFile(platform, pkgDir, storageClassFile, snapshotClassF
absSnapshotClassFilePath = filepath.Join(pkgDir, testConfigDir, snapshotClassFile)
}

minimumVolumeSize := "5Gi"
numAllowedTopologies := 1
if storageClassFile == regionalPDStorageClass {
minimumVolumeSize = "200Gi"
numAllowedTopologies = 2
}
params := driverConfig{
StorageClassFile: filepath.Join(pkgDir, testConfigDir, storageClassFile),
StorageClass: storageClassFile[:strings.LastIndex(storageClassFile, ".")],
SnapshotClassFile: absSnapshotClassFilePath,
SupportedFsType: fsTypes,
Capabilities: caps,
StorageClassFile: filepath.Join(pkgDir, testConfigDir, storageClassFile),
StorageClass: storageClassFile[:strings.LastIndex(storageClassFile, ".")],
SnapshotClassFile: absSnapshotClassFilePath,
SupportedFsType: fsTypes,
Capabilities: caps,
MinimumVolumeSize: minimumVolumeSize,
NumAllowedTopologies: numAllowedTopologies,
}

// Write config file
Expand Down
37 changes: 32 additions & 5 deletions test/k8s-integration/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ var (
gkeReleaseChannel = flag.String("gke-release-channel", "", "GKE release channel to be used for cluster deploy. One of 'rapid', 'stable' or 'regular'")
gkeTestClusterName = flag.String("gke-cluster-name", "gcp-pd-csi-driver-test-cluster", "GKE cluster name")
gkeNodeVersion = flag.String("gke-node-version", "", "GKE cluster worker node version")
isRegionalCluster = flag.Bool("is-regional-cluster", false, "tell the test that a regional cluster is being used. Should be used for running on an existing regional cluster (ie, --bringup-cluster=false). The test will fail if a zonal GKE cluster is created when this flag is true")

// Test infrastructure flags
boskosResourceType = flag.String("boskos-resource-type", "gce-project", "name of the boskos resource type to reserve")
Expand All @@ -74,6 +75,7 @@ const (
k8sOutOfDockerBuildBinDir = "_output/bin"
externalDriverNamespace = "gce-pd-csi-driver"
managedDriverNamespace = "kube-system"
regionalPDStorageClass = "sc-regional.yaml"
)

func init() {
Expand All @@ -97,11 +99,11 @@ func main() {
ensureVariable(deployOverlayName, false, "'deploy-overlay-name' must not be set when using GKE managed driver")
}

if *deployOverlayName != "noauth" {
ensureVariable(saFile, true, "service-account-file is a required flag")
}
if !*useGKEManagedDriver {
ensureVariable(deployOverlayName, true, "deploy-overlay-name is a required flag")
if *deployOverlayName != "noauth" {
ensureVariable(saFile, true, "service-account-file is a required flag")
}
}

ensureVariable(testFocus, true, "test-focus is a required flag")
Expand All @@ -122,6 +124,9 @@ func main() {
ensureVariable(kubeFeatureGates, false, "kube-feature-gates set but not bringing up new cluster")
} else {
ensureVariable(imageType, true, "image type is a required flag. Available options include 'cos' and 'ubuntu'")
if *isRegionalCluster {
klog.Error("is-regional-cluster can only be set when using an existing cluster")
}
}

if *platform == "windows" {
Expand Down Expand Up @@ -269,6 +274,14 @@ func handle() error {
testDir = k8sDir
}

if *deploymentStrat == "gke" {
gkeRegional := isRegionalGKECluster(*gceZone, *gceRegion)
if *isRegionalCluster && !gkeRegional {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we enforce that only one of gceZone and gceRegion could be set? Then we know the cluster is regional if len(*gceRegion) > 0 and we don't need the --is-regional-cluster flag

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still need --is-regional-cluster for when you're using a pre-existing cluster---you don't set the region or zone in that case and just use your current kubeconfig.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that we already check that only one of these is set in main.go:112

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha sounds good

nit: maybe make this field targeted to pre-existing clusters only, i.e. only make the flag settable if gceZone and gceRegion are unset, and update the flag description. Otherwise it could get confusing for someone new to understand when is-regional-cluster should be set just from looking at the flag description

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea will do.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(added in the bringupCluster check)

return fmt.Errorf("--is-regional-cluster set but deployed GKE cluster would be zonal")
}
*isRegionalCluster = gkeRegional
}

// Create a cluster either through GKE or GCE
if *bringupCluster {
var err error = nil
Expand Down Expand Up @@ -418,10 +431,24 @@ func handle() error {

// Run the tests using the testDir kubernetes
if len(*storageClassFiles) != 0 {
storageClasses := strings.Split(*storageClassFiles, ",")
applicableStorageClassFiles := []string{}
for _, rawScFile := range strings.Split(*storageClassFiles, ",") {
scFile := strings.TrimSpace(rawScFile)
if len(scFile) == 0 {
continue
}
if scFile == regionalPDStorageClass && !*isRegionalCluster {
klog.Warningf("Skipping regional StorageClass in zonal cluster")
continue
}
applicableStorageClassFiles = append(applicableStorageClassFiles, scFile)
}
if len(applicableStorageClassFiles) == 0 {
return fmt.Errorf("No applicable storage classes found")
}
var ginkgoErrors []string
var testOutputDirs []string
for _, scFile := range storageClasses {
for _, scFile := range applicableStorageClassFiles {
outputDir := strings.TrimSuffix(scFile, ".yaml")
testOutputDirs = append(testOutputDirs, outputDir)
if err = runCSITests(*platform, pkgDir, testDir, *testFocus, testSkip, scFile,
Expand Down
9 changes: 8 additions & 1 deletion test/run-k8s-integration-ci.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,21 @@ readonly gke_release_channel=${GKE_RELEASE_CHANNEL:-""}
readonly teardown_driver=${GCE_PD_TEARDOWN_DRIVER:-true}
readonly gke_node_version=${GKE_NODE_VERSION:-}
readonly run_intree_plugin_tests=${RUN_INTREE_PLUGIN_TESTS:-false}

storage_classes=sc-standard.yaml,sc-balanced.yaml,sc-ssd.yaml

if [[ -n $gce_region ]] ; then
storage_classes="${storage_classes}",sc-regional
fi

export GCE_PD_VERBOSITY=9

make -C "${PKGDIR}" test-k8s-integration

base_cmd="${PKGDIR}/bin/k8s-integration-test \
--run-in-prow=true --service-account-file=${E2E_GOOGLE_APPLICATION_CREDENTIALS} \
--do-driver-build=${do_driver_build} --teardown-driver=${teardown_driver} --boskos-resource-type=${boskos_resource_type} \
--storageclass-files=sc-standard.yaml,sc-balanced.yaml,sc-ssd.yaml --snapshotclass-file=pd-volumesnapshotclass.yaml \
--storageclass-files="${storage_classes}" --snapshotclass-file=pd-volumesnapshotclass.yaml \
--deployment-strategy=${deployment_strategy} --test-version=${test_version} \
--num-nodes=3 --image-type=${image_type}"

Expand Down