Skip to content
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

Enable e2e test for Alpha Feature AnyVolumeDataSource #25709

Merged
merged 1 commit into from
Mar 23, 2022
Merged

Enable e2e test for Alpha Feature AnyVolumeDataSource #25709

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: 2 additions & 2 deletions config/jobs/kubernetes/sig-cloud-provider/gcp/gcp-gce.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ presubmits:
- --provider=gce
- --runtime-config=api/all=true
- --stage=gs://kubernetes-release-pull/ci/pull-kubernetes-e2e-gce-alpha-features
- --test_args=--ginkgo.focus=\[Feature:(GRPCContainerProbe|InPlacePodVerticalScaling|ProbeTerminationGracePeriod|APIServerTracing|StorageVersionAPI|PodPreset|StatefulSetMinReadySeconds|CustomResourceValidationExpressions)\]|Networking --ginkgo.skip=\[Feature:(SCTPConnectivity|Volumes|Networking-Performance)\]|IPv6|csi-hostpath-v0 --minStartupPods=8
- --test_args=--ginkgo.focus=\[Feature:(GRPCContainerProbe|InPlacePodVerticalScaling|ProbeTerminationGracePeriod|APIServerTracing|StorageVersionAPI|PodPreset|StatefulSetMinReadySeconds|CustomResourceValidationExpressions|AnyVolumeDataSource)\]|Networking --ginkgo.skip=\[Feature:(SCTPConnectivity|Volumes|Networking-Performance)\]|IPv6|csi-hostpath-v0 --minStartupPods=8
Copy link
Member

Choose a reason for hiding this comment

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

Is there a Kubernetes PR validating that this test passes?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see kubernetes/kubernetes#108664, but do you know if this test ran? I think there's a way to configure a Kubernetes PR to run with a prow config from a PR, but I can't recall the details.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andrewsykim Thank you for your review. IIUC, without --ginkgo.focus option for "[Feature:AnyVolumeDataSource]", the e2e tests containing "[Feature:AnyVolumeDataSource]" string are skipped. So, this PR is needed to make this job not to skip these tests. In the PR, the job is enabled with /test pull-kubernetes-e2e-gce-alpha-features, but the tests seem to be skipped.

Copy link
Member

Choose a reason for hiding this comment

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

This is kind of hacky, but could you temporary update the ginkgo tag in your PR to something else to have the test run and see if it passes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The last commit which changed this line was #24393, and it enables [Feature:CustomResourceValidationExpressions] for the tests in k/k by this way. So, I think that this is the way the tests for alpha features have been enabled (and I agree that it looks hacky).

Copy link
Member

Choose a reason for hiding this comment

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

I'm not disagreeing with the approach to enable the tests, just asking if we can have a successful run of the test before enabling this for all alpha jobs. The (hacky) approach I mentioned is just a temporary hack to run the test in presubmit by using a known filter that works (just pick any of the existing alpha filters that are used).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test's PR is tested manually like this. The test requires alpha features, or AnyVolumeDataSource feature gate, to be enabled, which means removing [Feature:AnyVolumeDataSource] will make this test run in other jobs which don't enable the feature gate and fail.

So, I will update the test's PR if it is really required before merging this PR, however, I don't think that it should be a blocker for this PR (This PR could be merged without any side effects and the test's PR can be merged later without any dependencies).

Copy link
Member

Choose a reason for hiding this comment

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

The test's PR is tested manually like kubernetes/kubernetes#108664 (comment). The test requires alpha features, or AnyVolumeDataSource feature gate, to be enabled, which means removing [Feature:AnyVolumeDataSource] will make this test run in other jobs which don't enable the feature gate and fail.

This seems sufficient, thanks

- --timeout=180m
image: gcr.io/k8s-staging-test-infra/kubekins-e2e:v20220316-9d252673a0-master
resources:
Expand Down Expand Up @@ -576,7 +576,7 @@ periodics:
- --gcp-zone=us-west1-b
- --provider=gce
- --runtime-config=api/all=true
- --test_args=--ginkgo.focus=\[Feature:(InPlacePodVerticalScaling|StorageVersionAPI|PodPreset|CustomResourceValidationExpressions)\]|Networking --ginkgo.skip=\[Feature:(SCTPConnectivity|Volumes|Networking-Performance)\]|IPv6|csi-hostpath-v0 --minStartupPods=8
- --test_args=--ginkgo.focus=\[Feature:(InPlacePodVerticalScaling|StorageVersionAPI|PodPreset|CustomResourceValidationExpressions|AnyVolumeDataSource)\]|Networking --ginkgo.skip=\[Feature:(SCTPConnectivity|Volumes|Networking-Performance)\]|IPv6|csi-hostpath-v0 --minStartupPods=8
- --timeout=180m
image: gcr.io/k8s-staging-test-infra/kubekins-e2e:v20220316-9d252673a0-master
resources:
Expand Down