Run upstream e2e test with net-istio by nak3 · Pull Request #1014 · openshift-knative/serverless-operator

nak3 · 2021-06-10T02:53:18Z

/cc

openshift-ci · 2021-06-10T02:53:19Z

@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.

Details

In response to this:

/cc

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.

nak3 · 2021-06-10T05:47:55Z

/retest
istio-operator did not start.

nak3 · 2021-06-15T06:35:38Z

/test 4.7-upstream-e2e-aws-ocp-47

nak3 · 2021-06-15T10:49:37Z

domainmapping test hits this issue knative/serving@26c93b9

    stream.go:*54: E 07:41:55.348 domain-mapping-744987c5fb-zl6xs [knative.dev.serving.pkg.reconciler.domainmapping.Reconciler] [serving-tests/domain-mapping-bgboydtt.apps.ci-op-mcifrq*q-fbf8c.origin-ci-int-aws.dev.rhcloud.com] Returned an error err=failed to create Ingress: Ingress.networking.internal.knative.dev "domain-mapping-bgboydtt.apps.ciefb90bedf33484b06ac7d3ba94496ca7" is invalid: metadata.labels: Invalid value: "domain-mapping-bgboydtt.apps.ci-op-mcifrq*q-fbf8c.origin-ci-int-aws.dev.rhcloud.com": must be no more than 63 characters

nak3 · 2021-06-16T00:55:23Z

TestCustomOpenShiftRoute fails because it has fixed kourier and knative-serving-ingress values.

nak3 · 2021-06-17T12:33:23Z

grpc_test.go must be removed. It hangs and got timeout.

nak3 · 2021-06-23T04:31:09Z

Aletmanager prints warning https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/openshift-knative_serverless-operator/1014/pull-ci-openshift-knative-serverless-operator-main-4.7-upstream-e2e-aws-ocp-47/1407509003751133184/artifacts/upstream-e2e-aws-ocp-47/serverless-e2e/artifacts/build-TQsn6lF/alerts.json

Hmm... it should be fixed by #1036

nak3 · 2021-06-23T05:24:49Z

The original CRtest/v1alpha1/resources/operator.knative.dev_v1alpha1_knativeserving_cr.yaml sets to prometheus so we need to override.

nak3 · 2021-06-23T13:13:12Z

The test passed. Now trying to remove GODEBUG="x509ignoreCN=0".

nak3 · 2021-06-25T06:33:24Z

/retest

New failure with TestActivatorHAGraceful . I think it is flake.

nak3 · 2021-06-25T08:31:44Z

TestActivatorHAGraceful failed again due to Failed to delete pod activator-85d6cdc89-dr8bg: pods "activator-85d6cdc89-dr8bg" not found.

It will be improved by knative/serving@9054d8b 🤔

nak3 · 2021-06-27T05:49:06Z

/retest

TestActivatorHAGraceful works on my local even with openshift/release-v0.22.0 branch.

nak3 · 2021-06-28T04:06:03Z

TestActivatorHANonGraceful also has same problem 😥
https://prow.ci.openshift.org/view/gs/origin-ci-test/pr-logs/pull/openshift-knative_serverless-operator/1014/pull-ci-openshift-knative-serverless-operator-main-4.7-upstream-e2e-aws-ocp-47/1409337331164385280

nak3 · 2021-06-28T08:19:54Z

/retest

Test passed once. One more time.

nak3 · 2021-06-30T05:20:00Z

Serving 0.22 didn't have h2c issue but 0.23 got the issue log:

    http2_test.go:67: The endpoint http://hello-http2-with-por-fwbdxyth-serving-tests.apps.ci-op-17k619p9-fbf8c.origin-ci-int-aws.dev.rhcloud.com for Route hello-http2-with-por-fwbdxyth didn't serve the expected text "Hello, New World! How about donuts and coffee?": response: status: 426, body: , headers: map[Content-Length:[0] Date:[Wed, 30 Jun 2021 04:54:46 GMT] Server:[istio-envoy] X-Envoy-Upstream-Service-Time:[2] Zipkin_trace_id:[d47245bee119f7711504b28250809ca0]] did not pass checks: status = 426 426 Upgrade Required, want one of: [200]

I experienced it in openshift/knative-serving#751 but still not sure why only serving 0.23 happens this problem.

nak3 · 2021-06-30T15:23:22Z

/retest

TestRouteCreation failed. (It is the first time to fail it.)

    route_test.go:158: CheckSLO() error SLI for "TestRouteCreation" = 0.915663, wanted >= 1.000000

log

nak3 · 2021-07-01T02:51:41Z

Only two HA tests failed.
TestDomainMappingHA ... patch command overrides domainmapping's HA so needs to fix script.
TestControllerHA ... it needs openshift/knative-serving#836

nak3 · 2021-07-01T12:59:35Z

Umm... TestControllerHA still failed. openshift/knative-serving#836 might not be applied.

nak3 · 2021-07-01T22:48:31Z

/retest

infra

nak3 · 2021-07-02T00:25:41Z

/cc @markusthoemmes @mgencur @maschmid

This is ready for review. Could you please take a look?

skonto · 2021-07-02T08:05:24Z

@nak3 should we run downstream tests with this setup as well? Is this coming next? I would like to test metrics (ideally tracing too).

nak3 · 2021-07-02T08:49:24Z

Yes, I am going to add the mesh test for downstream with follow up PR. (Because it messes up script and hard to review I guess
😅 )

markusthoemmes · 2021-07-02T09:41:59Z

Makefile

+	# This is to run with FULL_MESH to verify the PR. Remove this and use "test-upstream-e2e-mesh".
+	FULL_MESH=true TEST_KNATIVE_KAFKA=false TEST_KNATIVE_E2E=true TEST_KNATIVE_UPGRADE=false ./test/upstream-e2e-tests.sh
+	# INSTALL_KAFKA=true TEST_KNATIVE_KAFKA=true TEST_KNATIVE_E2E=true TEST_KNATIVE_UPGRADE=false ./test/upstream-e2e-tests.sh


Should this go away before a merge?

Yes, correct. This is a temporary as pre-submit PR always run test-upstream-e2e-no-upgrade. I will revert it and trigger job against test-upstream-e2e-mesh.

markusthoemmes · 2021-07-02T09:43:17Z

test/serving.bash

+    # TODO: SRVKS-211: Can not run grpc and http2 tests.
+    rm ./test/e2e/grpc_test.go
+    rm ./test/e2e/http2_test.go
+    # Remove h2c test
+    sed -ie '46,50d' ./test/conformance/runtime/protocol_test.go


This means that we'll have to document gRPC/HTTP2 not working for mesh for now, right?

Yes, I will open the JIRA.

markusthoemmes · 2021-07-12T08:46:52Z

hack/lib/serverless.bash

+  config:
+    observability:
+      metrics.backend-destination: "none"


Is this required? Don't we do that "automatically"?

Umm... we have the default as "prometheus"

serverless-operator/test/v1alpha1/resources/operator.knative.dev_v1alpha1_knativeserving_cr.yaml

Line 39 in b368a3f

metrics.backend-destination: "prometheus"

so I think we need to override it with none. Sorry if I misunderstood the meaning of "automatically".

ahhhh we explicitly set it. Nevermind then, I was thinking in our Golang code, where we default to none currently, if istio is enabled.

markusthoemmes · 2021-07-12T08:47:09Z

hack/lib/serverless.bash

+  - name: domain-mapping
+    replicas: 2


Why do we need this? Don't we also need that in "usual" setups?

Yes, we have the replicas: 2 in "usual" setups.

serverless-operator/test/v1alpha1/resources/operator.knative.dev_v1alpha1_knativeserving_cr.yaml

Lines 7 to 8 in b368a3f

- name: domain-mapping

replicas: 2

But the spec.deployments is an array. So when we patch the annotation (inject and rewriteAppHTTPProbers) to spec.deployments for activator and autoscaler, domain-mapping's replicas: 2 is dropped 😓

Gotcha, makes sense!

markusthoemmes · 2021-07-12T08:48:13Z

test/serving.bash

+    # Use x509ignoreCN=0.
+    # This should not be necesssary if we could ceate certs with SAN. However, openssl command in the CI
+    # seems old version and so it does not have "-addext" option to add SAN.
+    export GODEBUG="x509ignoreCN=0"


We should be able to fix that. Can we update the openssl command in https://github.com/openshift-knative/serverless-operator/blob/main/openshift/ci-operator/build-image/Dockerfile?

I tried but the base image of registry.ci.openshift.org/openshift/release:golang-1.15 is CentOS 7 and the latest openssl command was still 1.10.

$ docker run build-image rpm -qf `which openssl` openssl-1.0.2k-21.el7_9.x86_64

markusthoemmes

/lgtm
/approve

/hold

Please revert the "old" make target and then we're good to go here :)

markusthoemmes

/lgtm
/approve

openshift-ci · 2021-07-12T11:27:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: markusthoemmes, nak3

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [markusthoemmes]

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

nak3 · 2021-07-13T05:24:40Z

/hold cancel

openshift-ci bot added the do-not-merge/work-in-progress label Jun 10, 2021

nak3 mentioned this pull request Jun 17, 2021

[SRVKS-752] Add NetworkPolicy for webhook on net-istio ingress #1027

Merged

nak3 changed the title ~~wip - Run e2e test with net-istio~~ wip - Run upstream e2e test with net-istio Jun 29, 2021

openshift-ci bot requested review from markusthoemmes, maschmid and mgencur July 2, 2021 00:25

markusthoemmes reviewed Jul 2, 2021

View reviewed changes

nak3 mentioned this pull request Jul 8, 2021

Run downstream test with net-istio & mTLS #1085

Merged

markusthoemmes reviewed Jul 12, 2021

View reviewed changes

Run e2e test with net-istio enabled

fb1a72f

markusthoemmes approved these changes Jul 12, 2021

View reviewed changes

openshift-ci bot added the do-not-merge/hold label Jul 12, 2021

openshift-ci bot assigned markusthoemmes Jul 12, 2021

openshift-ci bot added lgtm approved labels Jul 12, 2021

Revert makefile's temporary change

03de2e6

openshift-ci bot removed the lgtm label Jul 12, 2021

nak3 changed the title ~~wip - Run upstream e2e test with net-istio~~ Run upstream e2e test with net-istio Jul 12, 2021

openshift-ci bot removed the do-not-merge/work-in-progress label Jul 12, 2021

markusthoemmes approved these changes Jul 12, 2021

View reviewed changes

openshift-ci bot added the lgtm label Jul 12, 2021

openshift-ci bot removed the do-not-merge/hold label Jul 13, 2021

openshift-merge-robot merged commit 3eab11c into openshift-knative:main Jul 13, 2021

Conversation

nak3 commented Jun 10, 2021

Uh oh!

openshift-ci bot commented Jun 10, 2021

Uh oh!

nak3 commented Jun 10, 2021

Uh oh!

nak3 commented Jun 15, 2021

Uh oh!

nak3 commented Jun 15, 2021

Uh oh!

nak3 commented Jun 16, 2021

Uh oh!

nak3 commented Jun 17, 2021

Uh oh!

nak3 commented Jun 23, 2021

Uh oh!

nak3 commented Jun 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nak3 commented Jun 23, 2021

Uh oh!

nak3 commented Jun 25, 2021

Uh oh!

nak3 commented Jun 25, 2021

Uh oh!

nak3 commented Jun 27, 2021

Uh oh!

nak3 commented Jun 28, 2021

Uh oh!

nak3 commented Jun 28, 2021

Uh oh!

nak3 commented Jun 30, 2021

Uh oh!

nak3 commented Jun 30, 2021

Uh oh!

nak3 commented Jul 1, 2021

Uh oh!

nak3 commented Jul 1, 2021

Uh oh!

nak3 commented Jul 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nak3 commented Jul 2, 2021

Uh oh!

skonto commented Jul 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nak3 commented Jul 2, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nak3 Jul 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markusthoemmes left a comment

Choose a reason for hiding this comment

Uh oh!

markusthoemmes left a comment

nak3 commented Jun 23, 2021 •

edited

Loading

nak3 commented Jul 1, 2021 •

edited

Loading

skonto commented Jul 2, 2021 •

edited

Loading

nak3 Jul 12, 2021 •

edited

Loading