Add DomainMapping conformance test #9780

julz · 2020-10-12T06:56:44Z

Fourth part of #9713.

Now that (once #9752 lands) things work end-to-end, add a simple conformance test for DomainMapping.

Looks like this is the only thing outside of Ingress using the new alpha/beta conformance test stuff, so I hope I'm holding this right. Also if we want this to actually run in CI we'll need to deploy config/domain-mapping in test-infra.. gonna need some hand-holding on how that should work.

~~/hold for #9752.~~

/assign @mattmoor @dprotaso

markusthoemmes · 2020-10-12T06:59:06Z

pkg/apis/serving/v1alpha1/domainmapping_lifecycle.go

@@ -35,6 +35,11 @@ func (dm *DomainMapping) GetGroupVersionKind() schema.GroupVersionKind {
 	return SchemeGroupVersion.WithKind("DomainMapping")
 }

+// IsReady returns true if the DomainMapping is ready.
+func (dms *DomainMappingStatus) IsReady() bool {


I think we have moved away from this and are instead providing this method on the type itself, including a Generation check, like:

// IsReady returns true if the Status condition MetricConditionReady // is true and the latest spec has been observed. func (m *Metric) IsReady() bool { ms := m.Status return ms.ObservedGeneration == m.Generation && ms.GetCondition(MetricConditionReady).IsTrue() }

(this would also make the currently unused ReadyCondition constant used 😂 )

ah, I copied this from Route but sure that makes sense, lemme update

codecov · 2020-10-12T07:24:29Z

Codecov Report

Merging #9780 into master will increase coverage by 0.04%.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##           master    #9780      +/-   ##
==========================================
+ Coverage   87.75%   87.79%   +0.04%     
==========================================
  Files         180      180              
  Lines        8589     8590       +1     
==========================================
+ Hits         7537     7542       +5     
+ Misses        802      799       -3     
+ Partials      250      249       -1

Impacted Files	Coverage Δ
pkg/activator/net/throttler.go	`88.00% <50.00%> (-0.04%)`	⬇️
...g/apis/serving/v1alpha1/domainmapping_lifecycle.go	`76.19% <100.00%> (+5.60%)`	⬆️
pkg/queue/breaker.go	`96.42% <100.00%> (-0.09%)`	⬇️
pkg/reconciler/revision/reconcile_resources.go	`80.72% <0.00%> (-2.41%)`	⬇️
pkg/activator/net/revision_backends.go	`91.40% <0.00%> (+0.90%)`	⬆️
pkg/reconciler/autoscaling/kpa/scaler.go	`90.00% <0.00%> (+1.42%)`	⬆️
pkg/reconciler/configuration/configuration.go	`89.34% <0.00%> (+1.63%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1c9dcf0...fd1c4f2. Read the comment docs.

mattmoor · 2020-10-12T18:31:53Z

test/conformance/api/v1alpha1/domain_mapping_test.go

+	if !test.ServingFlags.EnableAlphaFeatures {
+		t.Skip("Alpha features not enabled")
+	}


We should enable this in .github/workflows/kind-e2e.yaml and ./test/e2e-tests.sh otherwise nothing is running this. I feel like we should be running this at least somewhere.

I'd kinda like to see the istio class annotation moved to be config-based before we start adding e2e tests that will light up folks testgrid.

added to kind-e2e in #9837, Im gonna ping on slack to try to get some hand-holding for how to go about trying to iterate on modifications to e2e-tests.sh :)

.. and added prow e2e in #9851. Think this may be ready to go now.

julz · 2020-10-16T05:55:14Z

Rebased on master so we can try this out with the new e2e test GitHub action 🤞.

julz · 2020-10-16T07:12:29Z

Neat: passes on contour and ingress, fails on kourier as expected bc I don't think we have RewriteHost implemented there yet. Do we want to exclude the kourier row for now or keep it in there as incentive to get that working @mattmoor?

julz · 2020-10-20T08:48:47Z

Rebased so we can try this with the prow-based e2e-tests now #9851 has landed 🤞.

julz · 2020-10-20T10:01:38Z

/retest

mattmoor · 2020-10-20T14:10:44Z

@julz yeah I'd add an exclude with TODO(#1234) for an issue to track this with kourier?

julz · 2020-10-20T14:20:40Z

good call @mattmoor, done.

mattmoor

/lgtm

knative-prow-robot · 2020-10-20T15:10:37Z

[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:

~~OWNERS~~ [mattmoor]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

julz · 2020-10-20T15:27:54Z

/unhold

knative-prow-robot assigned dprotaso and mattmoor Oct 12, 2020

knative-prow-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Oct 12, 2020

google-cla bot added the cla: yes Indicates the PR's author has signed the CLA. label Oct 12, 2020

knative-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 12, 2020

knative-prow-robot requested review from chaodaiG and coryrc October 12, 2020 06:56

knative-prow-robot added area/API API objects and controllers area/test-and-release It flags unit/e2e/conformance/perf test issues for product features labels Oct 12, 2020

markusthoemmes reviewed Oct 12, 2020

View reviewed changes

mattmoor reviewed Oct 12, 2020

View reviewed changes

julz mentioned this pull request Oct 13, 2020

DomainMapping CRD (Tracking Issue) #9713

Closed

14 tasks

knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 15, 2020

julz mentioned this pull request Oct 15, 2020

Run v1alpha1 tests (and deploy DomainMapping component) in github actions #9837

Merged

julz force-pushed the domainmappingconformance branch from a47e56b to 9d93fc7 Compare October 16, 2020 05:54

knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 16, 2020

julz changed the title ~~WIP: Add DomainMapping conformance test~~ Add DomainMapping conformance test Oct 16, 2020

knative-prow-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 16, 2020

julz mentioned this pull request Oct 19, 2020

Deploy DomainMappings and Enable Alpha tests in e2e tests #9851

Merged

julz added 5 commits October 20, 2020 09:44

Add DomainMapping conformance test

ab7fefe

Use IsReady on object rather than status

870e47e

Fix field name for consistency

f7e1d6f

Remove placeholder file

0b0555b

Use PollTimeout constant

8823a40

julz force-pushed the domainmappingconformance branch from 4adf445 to 8823a40 Compare October 20, 2020 08:47

Exclude Kourier for now

db0d36f

Use k8s-version because kingress isn't a row unfortunately

fd1c4f2

mattmoor approved these changes Oct 20, 2020

View reviewed changes

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 20, 2020

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 20, 2020

knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 20, 2020

knative-prow-robot merged commit 16e6096 into knative:master Oct 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add DomainMapping conformance test #9780

Add DomainMapping conformance test #9780

julz commented Oct 12, 2020 •

edited

Loading

markusthoemmes Oct 12, 2020

julz Oct 12, 2020

codecov bot commented Oct 12, 2020 •

edited

Loading

mattmoor Oct 12, 2020

julz Oct 16, 2020

julz Oct 20, 2020

julz commented Oct 16, 2020

julz commented Oct 16, 2020

julz commented Oct 20, 2020

julz commented Oct 20, 2020

mattmoor commented Oct 20, 2020

julz commented Oct 20, 2020

mattmoor left a comment

knative-prow-robot commented Oct 20, 2020

julz commented Oct 20, 2020

Add DomainMapping conformance test #9780

Add DomainMapping conformance test #9780

Conversation

julz commented Oct 12, 2020 • edited Loading

markusthoemmes Oct 12, 2020

Choose a reason for hiding this comment

julz Oct 12, 2020

Choose a reason for hiding this comment

codecov bot commented Oct 12, 2020 • edited Loading

Codecov Report

mattmoor Oct 12, 2020

Choose a reason for hiding this comment

julz Oct 16, 2020

Choose a reason for hiding this comment

julz Oct 20, 2020

Choose a reason for hiding this comment

julz commented Oct 16, 2020

julz commented Oct 16, 2020

julz commented Oct 20, 2020

julz commented Oct 20, 2020

mattmoor commented Oct 20, 2020

julz commented Oct 20, 2020

mattmoor left a comment

Choose a reason for hiding this comment

knative-prow-robot commented Oct 20, 2020

julz commented Oct 20, 2020

julz commented Oct 12, 2020 •

edited

Loading

codecov bot commented Oct 12, 2020 •

edited

Loading