search

knative / serving

Kubernetes-based, scale-to-zero, request-driven compute
https://knative.dev/docs/serving/
Apache License 2.0
5.53k stars 1.15k forks source link

Should upgrade test run with all KIngress? #8095

Open nak3 opened 4 years ago

nak3 commented 4 years ago

In what area(s)?

/area networking /area test-and-release

Describe the feature

As per title, we always use net-istio for the upgrade test(e2e-upgrade-tests.sh). It does not support to run it against Kourier or other Ingress solutions actually.

So I would like to request to run e2e-upgrade-tests.sh against other Ingress providers and see the status in test grid.

Is it not good idea?

If we will test against other Kingress, I think we would need following tasks:

  1. Update e2e-upgrade-tests.sh to install/configure other KIngress in install_latest_release().
  2. Add the test to test-infra's each ingress job.
nak3 commented 4 years ago

The background of this request is that downstream (using net-kourier) frequently fails on TestProbe executed by go_test_e2e -tags=preupgrade -timeout=20m ./test/upgrade. We are still not sure about the root cause, but I think the test results could be different among KIngresses as their Ingress reconciliation logic is different.

mattmoor commented 4 years ago

I'd like to be able to run it with any kingress, and I'd imagine the kourier folks would as well.

cc @tcnghia @dprotaso

github-actions[bot] commented 3 years ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

mattmoor commented 3 years ago

/lifecycle frozen

@nak3 any interest in trying to make this work post-submit for all kingress?

nak3 commented 3 years ago

/assign

Sure, I will try.

markusthoemmes commented 3 years ago

FWIW I did add an upgrade test to Kourier specifically and am planning to move that to knative/networking so each ingress impl. can test their upgrade behavior in isolation.

evankanderson commented 3 years ago

Is this still an issue?

If so, is this something which should go into the networking implementations or the serving repo? It would be nice to have a property that we didn't need bidirectional import/testing dependencies between the repos.

/triage accepted

evankanderson commented 3 years ago

/remove-triage accepted /triage needs-user-input

evankanderson commented 3 years ago

/remove-triage needs-user-input /triage accepted

It looks like this issue is stale. @nak3, is this still on the radar? Is this issue the best way to track it?

nak3 commented 1 year ago

Sorry for the late response. I missed this.

I think serving repo should have:

I tried to add the github action on serving's repo but it failed due to ytt and kapp issue. (I drop my assignment for now as it is difficult for me to fix the kapp issue...)

dprotaso commented 1 year ago

what's the kapp issue?

nak3 commented 1 year ago

Here is what I am testing - https://github.com/nak3/serving/pull/57 In short, it is something like:

        export INSTALL_SERVING_VERSION="latest-release"
        export INSTALL_ISTIO_VERSION="latest-release"
        knative_setup
        go test ./test/upgrade/

Then, it fails to run install_head_reuse_ingress. 

2023-04-20T09:39:45.8108074Z Apr 20 09:39:17.928 install_head_reuse_ingress [OUT] 9:39:17AM: update secret/webhook-certs (v1) namespace: 7b1820da-01eb-4aa7-8015-72bc0400e8d8
2023-04-20T09:39:45.8108904Z Apr 20 09:39:17.928 install_head_reuse_ingress [OUT] 9:39:17AM: update configmap/config-tracing (v1) namespace: 7b1820da-01eb-4aa7-8015-72bc0400e8d8
2023-04-20T09:39:45.8109418Z Apr 20 09:39:18.684 install_head_reuse_ingress [OUT]
2023-04-20T09:39:45.8110173Z Apr 20 09:39:18.684 install_head_reuse_ingress [ERR] kapp: Error: update configmap/config-tracing (v1) namespace: 7b1820da-01eb-4aa7-8015-72bc0400e8d8:
2023-04-20T09:39:45.8111249Z Apr 20 09:39:18.684 install_head_reuse_ingress [ERR]   Updating resource configmap/config-tracing (v1) namespace: 7b1820da-01eb-4aa7-8015-72bc0400e8d8:
2023-04-20T09:39:45.8112863Z Apr 20 09:39:18.684 install_head_reuse_ingress [ERR]     API server says: admission webhook "config.webhook.serving.knative.dev" denied the request: validation failed: the update modifies a key in "_example" which is probably not what you want. Instead, copy the respective setting to the top-level of the ConfigMap, directly below "data" (reason: BadRequest)
2023-04-20T09:39:45.8113707Z Apr 20 09:39:18.689 install_head_reuse_ingress [ERR] exit status 1
2023-04-20T09:39:45.7809644Z === CONT  TestServingUpgrades/Run/Steps/UpgradeWith/ServingHead
2023-04-20T09:39:45.7821713Z     shell.go:40: exit status 158
2023-04-20T09:39:45.7824298Z                 --- FAIL: TestServingUpgrades/Run/Steps/UpgradeWith/ServingHead (51.11s)
dprotaso commented 1 year ago

API server says: admission webhook "config.webhook.serving.knative.dev" denied the request: validation failed: the update modifies a key in "_example" which is probably not what you want. Instead, copy the respective setting to the top-level of the ConfigMap, directly below "data" (reason: BadRequest)

I believe there's a kapp debug flag that shows the diff that's being applied - you could infer what's modifying the example block in the config map.