search

kubernetes-csi / csi-release-tools

shared build and test files used by kubernetes-csi projects
Apache License 2.0
22 stars 73 forks source link

Enable driver features to be toggled off/on #32

Open gnufied opened 5 years ago

gnufied commented 5 years ago

Rather than defaulting to always on features, I think we should allow tests suites to toggle them. Something like - 

CSI_ENABLED_FEATURES="controllerExpansion=true,nodeExpansion=true"

This will enable tests like https://github.com/kubernetes-csi/external-resizer/pull/53/files to enable certain features without having to update csi-release-tools. Also because features can differ between versions it will allow .prow.sh file in individual branches to have their own configuration. I am not sure if this is preferable to having if-elses built into prow.sh though.

gnufied commented 5 years ago

cc @pohly @msau42

pohly commented 5 years ago

This is about the test driver config, right?

https://github.com/kubernetes-csi/csi-release-tools/blob/0affdf95a80bfad3e254d93c2e80a3397f81e2f5/prow.sh#L742-L765

The content of that file must match the deployment of the host path driver, which is determined by CSI_PROW_HOSTPATH_VERSION (controlled by prow.sh) and CSI_PROW_KUBERNETES_VERSION (sometimes controlled by the Prow job config), and what the E2E suite expects, which is determined by CSI_PROW_E2E_VERSION (again controlled by prow.sh). That this isn't entirely controlled by prow.sh is unfortunate, but any other solution that I could think of had the same issue.

What I don't understand is why you want to enable certain features without having to update csi-release-tools. Is it because that is more work? It is more work, but it has the advantage that testing is consistent.

If we want testing that is different for different releases, then csi-release-tools needs to be forked.

fejta-bot commented 4 years ago

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta. /lifecycle stale

msau42 commented 4 years ago

I think the issue @gnufied is referring to the issue of new features and maybe sidecars are introduced in newer releases. But the test config is the same across all releases. We currently add a lot of release-specific logic into prow.sh to toggle on and off focus/skip flags because of this, which I don't really like because it adds a lot of toil every new release to update these settings and then sync it across all repos.

What if we add the test config to the per-release hostpath deployments? That should solve some of the difficulties because the test will be configured to run based on the hostpath deployment version, which indicates what the driver supports.

It could also potentially help us move towards a model where prow.sh is not so hardcoded with hostpath driver, and could be used for other drivers too.

/lifecycle-frozen

msau42 commented 4 years ago

/lifecycle frozen

pohly commented 4 years ago

Michelle Au notifications@github.com writes:

What if we add the test config to the per-release hostpath deployments? That should solve some of the difficulties because the test will be configured to run based on the hostpath deployment version, which indicates what the driver supports.

And the content of that per-Kubernetes-release test config then is for the storage test suite from that Kubernetes release? That could work.