Drop the hardcoded cluster.local #4277
Conversation
Experimenting with Eventing's e2e and a non-standard cluster suffix, I noticed that this test failed. Once my change to support KinD for Eventing e2e lands, we can make that pseudo-randomize this to prevent bugs like this in the future.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 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
area/test-and-release
Codecov Report
@@ Coverage Diff @@
## master #4277 +/- ##
=======================================
Coverage 80.25% 80.25%
=======================================
Files 287 287
Lines 7887 7887
=======================================
Hits 6330 6330
Misses 1174 1174
Partials 383 383 Continue to review full report at Codecov.
|
|
cc @tcnghia FYI (since you are chasing this in Serving) |
knative-prow-robot
added
the
do-not-merge/hold
| @@ -91,7 +91,7 @@ func TestTriggerDependencyAnnotation(t *testing.T) { | |||
| if brokerClass == eventing.MTChannelBrokerClassValue { | |||
| pingSource.Spec.SourceSpec.Sink.URI = &apis.URL{ | |||
| Scheme: "http", | |||
| Host: fmt.Sprintf("broker-ingress.%s.svc.cluster.local", resources.SystemNamespace), | |||
| Host: fmt.Sprintf("broker-ingress.%s.svc", resources.SystemNamespace), | |||
There was a problem hiding this comment.
I think we should stop setting the URI directly and set the pingSource.Spec.SourceSpec.Sink.Ref to the Broker since we have all the bits here already, and would make this more resilient if we wanted to test against other brokers. Then we can drop the if clause and just do it in L87.
|
/unhold |
knative-prow-robot
removed
the
do-not-merge/hold
Experimenting with Eventing's e2e and a non-standard cluster suffix, I noticed that this test failed.
Once my change to support KinD for Eventing e2e lands, we can make that pseudo-randomize this to prevent bugs like this in the future.
/cc @vaikas
/assign @vaikas
This was introduced in the initial MT broker change, I believe. Perhaps we should replace this with proper Addressable resolution via the source, but this seemed like the smallest change to unblock the scenario I'm chasing. LMK which you prefer.