add ability to use gcloud to boot a gke cluster for e2e integration #288
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
Hi @hantaowang. Thanks for your PR. I'm waiting for a kubernetes-sigs or kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
needs-ok-to-test
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
| @@ -59,6 +61,7 @@ var ( | |||
| const ( | |||
| pdImagePlaceholder = "gcr.io/gke-release/gcp-compute-persistent-disk-csi-driver" | |||
| k8sBuildBinDir = "_output/dockerized/bin/linux/amd64" | |||
| gkeTestClusterName = "gcp-pd-csi-driver-test-cluster" | |||
There was a problem hiding this comment.
Can we append some random suffix to it? That will let us run multiple test instances in parallel. Or if some previous test cluster failed to cleanup properly, then subsequent test runs would not get blocked.
There was a problem hiding this comment.
Unless I'm mistaken, the current kube-up script does not do this either. Are separate tests run through prow supposed to be run on separate projects, thus avoiding any concurrent and subsequent conflicts.
There was a problem hiding this comment.
Yes, technically with boskos project lending, there shouldn't be concurrent runs in the same project. However, I'm thinking more about the scenario where a previous run failed to properly clean up the cluster (for whatever reason).
The kube-up script first tries to delete any existing cluster before trying to create a new one to get around this issue. We may want to consider doing something similar for gke.
test/k8s-integration/main.go
Outdated
| } | ||
|
|
||
| if *teardownCluster { | ||
| defer func() { | ||
| err := clusterDown(k8sDir) | ||
| var err error | ||
| if *deploymentStrat == "gke" { |
There was a problem hiding this comment.
nit: this can also be better as a switch statement and should contain a default with an error.
It's possible to have teardownCluster set without bringUpCluster set. So it's not safe to assume *deploymentStrat != "gke" --> deploymentStrat == gce in this case.
|
@msau42 @davidz627 the latest commit should address your remaining comments. if there is nothing else, I will squash tomorrow morning for the final approval. |
hantaowang
force-pushed
the
gke-integration-tests
branch
from
631e197 to
531f5c7
Compare
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: davidz627, hantaowang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
Adds the following flags to
k8s-integration-test:deployment-strategy: If a cluster needs to be booted up, choose if it is done on GCE through thekube-ip.shscript or GKE through thegcloudutility. The GKE option is incompatible with the migration test and must use a cluster version supported by GKE. Feature gates cannot be turned on when GKE is used. The default value is "gce", and GKE can be turned on by setting it to "gke".gke-cluster-version: If GKE is used, this flag can be set to an available cluster version, which can be checked withgcloud container get-server-config. Also accepted values are "latest" and "stable". The deault value is "latest".