Run downstream test with net-istio & mTLS#1085
Run downstream test with net-istio & mTLS#1085openshift-merge-robot merged 14 commits intoopenshift-knative:mainfrom nak3:add-net-istio-downstream
Conversation
|
@nak3: GitHub didn't allow me to request PR reviews from the following users: nak3. Note that only openshift-knative members and repo collaborators can review this PR, and authors cannot review their own PRs. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
hack/lib/mesh_resources/smcp.yaml
Outdated
| trafficControl: | ||
| inbound: | ||
| excludedPorts: | ||
| - 15020 |
There was a problem hiding this comment.
Can we add port 8444 here for metrics to work?
There was a problem hiding this comment.
Thank you @skonto I added 8444.
By the way, do you have an idea how to stop alert error - alerts.json.
We need to set metrics.backend-destination to "none" like #1014 ?
There was a problem hiding this comment.
Nope we should have the backend as is. The workaround that exposes the port should make things work.
nak3
changed the title
|
/hold @markusthoemmes @skonto @mgencur This is ready for review. Could you please take a look? |
|
As long as we dont lose coverage down some code path we are fine. I tested the setup locally. /lgtm |
markusthoemmes
left a comment
markusthoemmes
left a comment
There was a problem hiding this comment.
LGTM on the surface. Have we verified that the "normal" test targets are not affected by the pkg move?
| @@ -0,0 +1,14 @@ | |||
| apiVersion: networking.k8s.io/v1 | |||
There was a problem hiding this comment.
Should we ship this together with Knative Serving maybe? Or are we going to document that users have to apply this manually?
There was a problem hiding this comment.
I think that it should be alright to ship it with Knative Serving (When istio ingress class is enabled, the networkpolicy is applied.).
@skonto are you alright? I think we need to update the doc https://docs.openshift.com/container-platform/4.6/serverless/serverless-release-notes.html#serverless-rn-1-16-0_serverless-release-notes as well.
| replicas: 2 | ||
| config: | ||
| observability: | ||
| metrics.backend-destination: "none" |
| kind: NetworkPolicy | ||
| metadata: | ||
| name: allow-from-openshift-monitoring-ns | ||
| namespace: knative-serving |
There was a problem hiding this comment.
This lacks the provider label, so it'll be applied even with kourier, which we don't want.
This is a test so I am running both patterns. - test-upstream-e2e-no-upgrade -> test-e2e-with-mesh - test-e2e-with-kafka -> test-upstream-e2e-no-upgrade
|
/test 4.7-upstream-e2e-mesh-aws-ocp-47 |
|
|
/retest Hmm... the error should not be related to this PR. |
|
/hold cancel |
markusthoemmes
left a comment
markusthoemmes
left a comment
There was a problem hiding this comment.
/assign @maschmid
LGTM! Want to give Marek the right of the last word.
|
/lgtm |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: markusthoemmes, maschmid, nak3, skonto The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
5 participants
This patch adds
test-e2e-with-meshfor downstream test with net-istio&mTLS.servinge2e tests excludes the following tests for net-istio&mTLS:
servicemesh_test.go... test for kourier&istioservice_to_service_test.go... test for kourier&istioverify_http_and_https_test.go... test for OpenShift Route's TLS. net-istio&mTLS does not use Route's TLS.custom_route_test.go... test for OpenShift Route's escape hatch. Istio's Gateway is flexible and we don't need to support it for istio.verify_route_conflict_test.go... needs to add extra namespaceconflict-testandtestto SMMR.