Run v1alpha1 tests (and deploy DomainMapping component) in github actions #9837
Conversation
knative-prow-robot
added
size/M
Codecov Report
@@ Coverage Diff @@
## master #9837 +/- ##
==========================================
- Coverage 87.72% 87.64% -0.09%
==========================================
Files 180 180
Lines 8593 8593
==========================================
- Hits 7538 7531 -7
- Misses 805 808 +3
- Partials 250 254 +4
Continue to review full report at Codecov.
|
|
/retest |
|
v1alpha1 was deleted in #9545 |
|
oh, you have a new CRD with v1alpha1 version I guess the infra should not have been ripped out if that was a possibility. |
| # This is explicitly added since we don't want to run every single matrix element twice. | ||
| - k8s-version: v1.19.1 | ||
| test-suite: ./test/conformance/api/v1alpha1 | ||
| test-flags: "--enable-alpha" |
There was a problem hiding this comment.
I'd expect us to just add this to existing configurations. I don't think that duplicating works properly based on prior experimentation 🤔
There was a problem hiding this comment.
looks like it does in https://github.com/julz/serving/pull/2/checks?check_run_id=1260072669
There was a problem hiding this comment.
(it works because test-suite: ./test/conformance/api/v1alpha1 isn't in the existing matrix, so it's a new configuration, but there's really no point adding test-suite: ./test/conformance/api/v1alpha to the matrix since there's nothing in there that doesn't need the flag)
There was a problem hiding this comment.
Yeah, I was actually just coming back to leave that comment 🤣
I think what we really want is to add ./test/conformance/api/v1alpha1 to the test-suite list and then have a separate block with:
- test-suite: ./test/conformance/api/v1alpha1
test-flags: --enable-alpha
This should give us a new leg for all K8s versions, which has the test-flags. If there are combinations that don't work, I believe we can exclude: them (likely with TODO(#1234) 🙏 ), but my preference would be to add this for as many as we can here.
There was a problem hiding this comment.
that's fine with me, I was guessing we'd want to limit the test explosion and only add for one combination (like we do with ingress), will update
There was a problem hiding this comment.
When I added these we had alpha and beta, so it's sort of a return to that, but with many fewer tests (this is doing to be so fast 😂 )
knative-prow-robot
added
the
needs-rebase
knative-prow-robot
removed
the
needs-rebase
| @@ -43,6 +44,8 @@ jobs: | |||
| kind-version: v0.9.0 | |||
| kind-image-sha: sha256:98cf5288864662e37115e362b23e4369c8c4a408f99cbc06e58ac30ddc721600 | |||
| kingress: contour | |||
| - test-suite: ./test/conformance/api/v1alpha1 | |||
There was a problem hiding this comment.
nit: I'd put some space between the two blocks, since they act on different dimensions of the matrix
|
/retest ControllerHA seems to be acting up 🤔 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: julz, mattmoor 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 |
knative-prow-robot
added
the
approved
Since we'll be adding some tests to this suite gated by the enable-alpha flag as part of the DomainMapping work.
I think I'm holding GitHub actions right, but it's my first time trying so 🤷.
Example running on my fork here: https://github.com/julz/serving/runs/1258795911?check_suite_focus=true.
/assign @mattmoor @markusthoemmes