prow enhancements #13
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pohly 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
cncf-cla: yes
|
/test pull-sig-storage-csi-lib-utils |
|
/test pull-sig-storage-csi-release-tools |
This ensures that also new, currently unknown alpha gates are enabled when testing against a future Kubernetes versions. For all currently known Kubernetes versions we just use the minimal set of alpha gates, which ensures that we don't miss any of them in our documentation.
"grep -w" treated "serial-alpha" as two words and therefore CSI_PROW_TESTS sometimes ran too many tests.
|
/test pull-sig-storage-csi-release-tools |
|
/hold The version check is not quite right yet. |
k8s-ci-robot
added
the
do-not-merge/hold
The previous logic failed for canary jobs, those also deploy a recent driver. Instead of guessing what driver gets installed based on job parameters, check what really runs in the cluster and base the decision on that. We only need to maintain this blacklist for 1.0.x until we replace it with 1.1.0, then this entire hostpath_supports_block can be removed.
|
/hold cancel |
k8s-ci-robot
removed
the
do-not-merge/hold
|
KinD is failing in Prow for the "on-master" jobs (and only for those). Works for me locally. I would prefer to merge this PR as it is right now and investigate that failure separately, because then we can also merge some of the other PRs which aren't affected. |
|
/hold I'm starting to think that |
k8s-ci-robot
added
the
do-not-merge/hold
|
/hold cancel I've removed the |
k8s-ci-robot
removed
the
do-not-merge/hold
|
All four PRs which include this change ran okay: |
prow.sh
Outdated
| @@ -846,102 +855,99 @@ main () { | |||
| run_with_go "${CSI_PROW_GO_VERSION_BUILD}" make container || die "'make container' failed" | |||
| fi | |||
|
|
|||
| install_kind || die "installing kind failed" | |||
| start_cluster || die "starting the cluster failed" | |||
| if test_enabled "sanity" || test_enabled "parallel" || test_enabled "serial" || test_enabled "serial-alpha" || test_enabled "parallel-alpha"; then | |||
There was a problem hiding this comment.
How about having a couple of variables that can serve as whitelists of jobs that:
- require non-alpha cluster
- require alpha cluster
There was a problem hiding this comment.
You mean functions like job_needs_cluster, job_needs_alpha_cluster, job_needs_non_alpha_cluster instead of spelling that out via test_enabled? Yes, that would be a bit cleaner, albeit not necessarily shorter overall. I'll change it.
prow.sh
Outdated
| # Careful with AllAlpha=true: enabling all alpha features turned out | ||
| # to be problematic (kubelet died), so let's better list alpha gates | ||
| # explicitly. | ||
| configvar CSI_PROW_E2E_ALPHA_GATES_LATEST 'VolumeSnapshotDataSource=true' "alpha feature gates for latest Kubernetes" |
There was a problem hiding this comment.
ExpandCSIVolumes is also an alpha feature for 1.14.
When running only some tests, sometimes extra, unnecessarily work was done, like bringing up the cluster without alpha gates.
Not all environments have Docker. The simplifying assumption here is that if the Docker command is available, it's also usable.
When KinD fails in a Prow job, we need additional information to understand why it failed.
It turned out to not work. Instead of reverting the commit which introduced this, let's better document this explicitly.
|
/test pull-sig-storage-csi-release-tools |
|
All tests in the individual PRs passed, with just the known issue that hostpath v1.0.1 fails some csi-sanity tests. This will be fixed by updating the 1.13 deployment with hostpath v1.1.0. |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
add approvers to csi-lib-utils
This is related to kubernetes/test-infra#12088 but both can be merged independently.
/assign @msau42