test-infra
test-infra copied to clipboard
kubernetes
Enable in-tree gcepd driver for e2e tests
See https://github.com/kubernetes/kubernetes/pull/109541. Since cloud-provider-gcp installs the pd csi driver, it's safe to enable this now.
Tested manually, the best I could and it seemed to pass.
/assign @leiyiz
Probably you want to add this to a much broader set of tests - presubmit and another periodic. In this case you probably should use preset. However, I have a strong feeling that this is a no-op change.
https://github.com/kubernetes-sigs/kubetest2/blob/master/kubetest2-gce/deployer/common.go#L89 does not pass environment variables as it's unsupported in kubetest2.
See #26760
@jprzychodzen note these are tests that up until the beginning of the year ran with this e2e tests, and were disabled due to csi migration. So I'm not sure another periodic is necessary?
At any rate, the thing about environment variables and kubetest2 is annoying. I don't fully understand it, but I'll take your word for it. As you can see from https://github.com/kubernetes/kubernetes/pull/109541 the test setup depends on environment variables being passed in.
Maybe we should fix kubetest2? Or change e2e.test to detect if the pd csi driver is installed.
Ah, I see now - I was thinking that is passed to kubetest2 deployer part - gce deployer. This part does not support env variables. I don't know about ginko framework part. It might already support env variables so this change would do what you want.
In this case - could you create a preset with this env, add this preset to all e2e tests - there is presubmit that should have this env variable.
Regarding changing kubetest2 gce deployer - it seems like some kind of explicit decision from the comment, so I guess it would be to understand why it's this way before changing the code.
Hmm, looking at some internal GKE tests, it appears that indeed the env variable doesn't get plumbed through (we stop testing gcepd at 1.24 which is where https://github.com/kubernetes/kubernetes/pull/109541 was backported to IIRC.
So I think we do need a command line flag or a test config change?
I've raised https://github.com/kubernetes-sigs/kubetest2/issues/202 to start discussion and will work on adding a flag in the meantime.
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: leiyiz, mattcary
Once this PR has been reviewed and has the lgtm label, please ask for approval from mikedanese by writing /assign @mikedanese in a comment. For more information see:The Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
I've updated this PR for the new flag, but it will need to be backported to 1.24 before it will be usable from the prow job (and wait for a patch release to come out? the prow job uses test version 1.24.2 explicitly, so we'll have to update the patch release as well I think)
Sorry for the long time to respond, pre freeze time was intensive.
Probably the most reasonable approach forward with this flag is to migrate tests (and repository) to 1.25/master branch, which should happen ~soon, as a part of preparation to 1.25 release.
Makes sense. Are you planning on continuing to pin to a patch release, track release-1.25, or just track master?
Short term - pin it to 1.25.
Medium/long term - I need to rework repository bumping/tracking, and this will be part of the problem to solve.
@mattcary: PR needs rebase.
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.
@mattcary can we close this PR?
Right now test jobs are using ginko version based on cloud-provider-gcp file (which now points to 1.25.3).
If we're now pinned to 1.25, I think we can submit this (when it was on 1.24, the flag would be ignored IIRC).
Let me rebase and test it again.
In this case please first add this flag in a fork of current presubmit to prevent breaking presubmit (see #27618 and #27823 for a process)
The recent push has forked out the jobs as you requested.
Although, I'm not able to test the kubetest2 commands any longer. I can add a --gcp-project flag to avoid boskos, but it fails trying to make release-tars (there's no release-tars rule). Maybe I have a bad combination of kubetest2 and k8s versions? I've tried kubetest2 at head and k8s at head and release-1.25, but no luck.
Anyway I guess since I've forked the presubmits it will probably be faster to submit this and let them run, than figure out kubetest :-/
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: jprzychodzen, leiyiz, mattcary
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~config/jobs/kubernetes/cloud-provider-gcp/OWNERS~~ [jprzychodzen]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
@mattcary: Updated the job-config configmap in namespace default at cluster test-infra-trusted using the following files:
- key
cloud-provider-gcp-periodics.yamlusing fileconfig/jobs/kubernetes/cloud-provider-gcp/cloud-provider-gcp-periodics.yaml
In response to this:
See https://github.com/kubernetes/kubernetes/pull/109541. Since cloud-provider-gcp installs the pd csi driver, it's safe to enable this now.
Tested manually, the best I could and it seemed to pass.
/assign @leiyiz
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.