[WIP] TEST-ONLY change net-contour image to cors changes only #15231

izabelacg · 2024-05-22T12:46:56Z

Testing only

change net-contour controller to a previous version to rule out other changes like updating dependencies: knative-extensions/net-contour@a0f7039
removed set of tests for CORS policy from the suite

knative-prow · 2024-05-22T12:47:03Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: izabelacg
Once this PR has been reviewed and has the lgtm label, please assign skonto for approval. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

codecov · 2024-05-22T12:52:31Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.77%. Comparing base (a0a1ac7) to head (e08f84b).
Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #15231      +/-   ##
==========================================
+ Coverage   84.76%   84.77%   +0.01%     
==========================================
  Files         218      218              
  Lines       13473    13473              
==========================================
+ Hits        11420    11422       +2     
+ Misses       1687     1685       -2     
  Partials      366      366

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

izabelacg · 2024-05-22T17:07:11Z

@dprotaso I have created this PR to help understand what is impacting serving tests in contour-latest_serving_main. Currently this PR has a previous version of net-contour but still with the more recent CORS change (changing only the httpproxy related to external routes added by knative-extensions/net-contour#1088) but I removed the tests added by #15188

It looks like the tests pass now, so the CORS ones might be messing with the rest. I don't understand why yet. Should I investigate this further or it's fine to focus on #15226 instead?

dprotaso · 2024-05-22T17:20:02Z

I don't understand why yet. Should I investigate this further or it's fine to focus on #15226 instead?

Let's figure out why those tests interfere with others. Now I'm curious. Then let's do #15226.

This reverts commit 482376f.

izabelacg · 2024-05-22T19:39:12Z

/test contour-latest_serving_main

izabelacg · 2024-05-22T20:35:13Z

Update:
The attempt to disable the cors config is resulting in error, and the net-contour-controller pod is left in an error test. I will test a different command and see if tests pass.

#toggle_feature cors-policy "" config-contour || fail_test
# instead will do the below to test
PATCH='[{"op": "remove", "path": "/data/cors-policy"}]'
kubectl patch cm config-contour -n "${SYSTEM_NAMESPACE}" --type=json -p "${PATCH}"

The net-contour-controller logs are shown below. I'll take a look later to understand why it crashes and doesn't give a more helpful error message:

{"severity":"debug","timestamp":"2024-05-22T20:30:34.083Z","logger":"net-contour-controller","caller":"ingress/controller.go:153","message":"Creating event broadcaster","commit":"4357467-dirty"}
{"severity":"info","timestamp":"2024-05-22T20:30:34.095Z","logger":"net-contour-controller","caller":"sharedmain/main.go:283","message":"Starting configuration manager...","commit":"4357467-dirty"}
E0522 20:30:34.108329       1 runtime.go:79] Observed a panic: "invalid memory address or nil pointer dereference" (runtime error: invalid memory address or nil pointer dereference)
goroutine 103 [running]:
k8s.io/apimachinery/pkg/util/runtime.logPanic({0x1a771a0, 0x2e8c9f0})
        k8s.io/apimachinery@v0.29.3/pkg/util/runtime/runtime.go:75 +0x85
k8s.io/apimachinery/pkg/util/runtime.HandleCrash({0x0, 0x0, 0xc0002dd180?})
        k8s.io/apimachinery@v0.29.3/pkg/util/runtime/runtime.go:49 +0x6b
panic({0x1a771a0?, 0x2e8c9f0?})
        runtime/panic.go:770 +0x132
knative.dev/net-contour/pkg/reconciler/contour/config.NewContourFromConfigMap(0xc000470120)
        knative.dev/net-contour/pkg/reconciler/contour/config/contour.go:84 +0x299
reflect.Value.call({0x1a23040?, 0x1e3a3f8?, 0x13?}, {0x1d31a70, 0x4}, {0xc000652c60, 0x1, 0x1?})
        reflect/value.go:596 +0xca6
reflect.Value.Call({0x1a23040?, 0x1e3a3f8?, 0xc0003a8660?}, {0xc00059a460?, 0xc0005c14a0?, 0x2f24da0?})
        reflect/value.go:380 +0xb9
knative.dev/pkg/configmap.(*UntypedStore).OnConfigChanged(0xc00031d5e0, 0xc000470120)
        knative.dev/pkg@v0.0.0-20240416145024-0f34a8815650/configmap/store.go:141 +0x12f
knative.dev/pkg/configmap.(*ManualWatcher).OnChange(0xc00011a330, 0xc000470120)
        knative.dev/pkg@v0.0.0-20240416145024-0f34a8815650/configmap/manual_watcher.go:72 +0xf7
knative.dev/pkg/configmap/informer.(*InformedWatcher).addConfigMapEvent(...)
        knative.dev/pkg@v0.0.0-20240416145024-0f34a8815650/configmap/informer/informed_watcher.go:220
knative.dev/pkg/configmap/informer.(*syncedCallback).Call(0xc000149140, {0x1d00580?, 0xc000470120?}, {0xc0003a8660, 0xe})
        knative.dev/pkg@v0.0.0-20240416145024-0f34a8815650/configmap/informer/synced_callback.go:94 +0x37
knative.dev/pkg/configmap/informer.(*InformedWatcher).Start.func1({0x1d00580?, 0xc000470120?})
        knative.dev/pkg@v0.0.0-20240416145024-0f34a8815650/configmap/informer/informed_watcher.go:158 +0x3f
k8s.io/client-go/tools/cache.ResourceEventHandlerFuncs.OnAdd(...)
        k8s.io/client-go@v0.29.3/tools/cache/controller.go:239
k8s.io/client-go/tools/cache.(*processorListener).run.func1()
        k8s.io/client-go@v0.29.3/tools/cache/shared_informer.go:972 +0x13e
k8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1(0x30?)
        k8s.io/apimachinery@v0.29.3/pkg/util/wait/backoff.go:226 +0x33
k8s.io/apimachinery/pkg/util/wait.BackoffUntil(0xc000331f70, {0x1fed020, 0xc000303500}, 0x1, 0xc0000ac9c0)
        k8s.io/apimachinery@v0.29.3/pkg/util/wait/backoff.go:227 +0xaf
k8s.io/apimachinery/pkg/util/wait.JitterUntil(0xc00059a770, 0x3b9aca00, 0x0, 0x1, 0xc0000ac9c0)
        k8s.io/apimachinery@v0.29.3/pkg/util/wait/backoff.go:204 +0x7f
k8s.io/apimachinery/pkg/util/wait.Until(...)
        k8s.io/apimachinery@v0.29.3/pkg/util/wait/backoff.go:161
k8s.io/client-go/tools/cache.(*processorListener).run(0xc000326e10)
        k8s.io/client-go@v0.29.3/tools/cache/shared_informer.go:966 +0x69
k8s.io/apimachinery/pkg/util/wait.(*Group).Start.func1()
        k8s.io/apimachinery@v0.29.3/pkg/util/wait/wait.go:72 +0x52
created by k8s.io/apimachinery/pkg/util/wait.(*Group).Start in goroutine 98
        k8s.io/apimachinery@v0.29.3/pkg/util/wait/wait.go:70 +0x73
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x17516f9]

goroutine 103 [running]:
k8s.io/apimachinery/pkg/util/runtime.HandleCrash({0x0, 0x0, 0xc0002dd180?})
        k8s.io/apimachinery@v0.29.3/pkg/util/runtime/runtime.go:56 +0xcd
panic({0x1a771a0?, 0x2e8c9f0?})
        runtime/panic.go:770 +0x132
knative.dev/net-contour/pkg/reconciler/contour/config.NewContourFromConfigMap(0xc000470120)
        knative.dev/net-contour/pkg/reconciler/contour/config/contour.go:84 +0x299
reflect.Value.call({0x1a23040?, 0x1e3a3f8?, 0x13?}, {0x1d31a70, 0x4}, {0xc000652c60, 0x1, 0x1?})
        reflect/value.go:596 +0xca6
reflect.Value.Call({0x1a23040?, 0x1e3a3f8?, 0xc0003a8660?}, {0xc00059a460?, 0xc0005c14a0?, 0x2f24da0?})
        reflect/value.go:380 +0xb9
knative.dev/pkg/configmap.(*UntypedStore).OnConfigChanged(0xc00031d5e0, 0xc000470120)
        knative.dev/pkg@v0.0.0-20240416145024-0f34a8815650/configmap/store.go:141 +0x12f
knative.dev/pkg/configmap.(*ManualWatcher).OnChange(0xc00011a330, 0xc000470120)
        knative.dev/pkg@v0.0.0-20240416145024-0f34a8815650/configmap/manual_watcher.go:72 +0xf7
knative.dev/pkg/configmap/informer.(*InformedWatcher).addConfigMapEvent(...)
        knative.dev/pkg@v0.0.0-20240416145024-0f34a8815650/configmap/informer/informed_watcher.go:220
knative.dev/pkg/configmap/informer.(*syncedCallback).Call(0xc000149140, {0x1d00580?, 0xc000470120?}, {0xc0003a8660, 0xe})
        knative.dev/pkg@v0.0.0-20240416145024-0f34a8815650/configmap/informer/synced_callback.go:94 +0x37
knative.dev/pkg/configmap/informer.(*InformedWatcher).Start.func1({0x1d00580?, 0xc000470120?})
        knative.dev/pkg@v0.0.0-20240416145024-0f34a8815650/configmap/informer/informed_watcher.go:158 +0x3f
k8s.io/client-go/tools/cache.ResourceEventHandlerFuncs.OnAdd(...)
        k8s.io/client-go@v0.29.3/tools/cache/controller.go:239
k8s.io/client-go/tools/cache.(*processorListener).run.func1()
        k8s.io/client-go@v0.29.3/tools/cache/shared_informer.go:972 +0x13e
k8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1(0x30?)
        k8s.io/apimachinery@v0.29.3/pkg/util/wait/backoff.go:226 +0x33
k8s.io/apimachinery/pkg/util/wait.BackoffUntil(0xc000331f70, {0x1fed020, 0xc000303500}, 0x1, 0xc0000ac9c0)
        k8s.io/apimachinery@v0.29.3/pkg/util/wait/backoff.go:227 +0xaf
k8s.io/apimachinery/pkg/util/wait.JitterUntil(0xc00059a770, 0x3b9aca00, 0x0, 0x1, 0xc0000ac9c0)
        k8s.io/apimachinery@v0.29.3/pkg/util/wait/backoff.go:204 +0x7f
k8s.io/apimachinery/pkg/util/wait.Until(...)
        k8s.io/apimachinery@v0.29.3/pkg/util/wait/backoff.go:161
k8s.io/client-go/tools/cache.(*processorListener).run(0xc000326e10)
        k8s.io/client-go@v0.29.3/tools/cache/shared_informer.go:966 +0x69
k8s.io/apimachinery/pkg/util/wait.(*Group).Start.func1()
        k8s.io/apimachinery@v0.29.3/pkg/util/wait/wait.go:72 +0x52
created by k8s.io/apimachinery/pkg/util/wait.(*Group).Start in goroutine 98
        k8s.io/apimachinery@v0.29.3/pkg/util/wait/wait.go:70 +0x73

This reverts commit dff8c10.

izabelacg · 2024-05-22T21:22:56Z

/test contour-latest_serving_main

izabelacg · 2024-05-22T21:27:35Z

Update:
I noticed the pod error state occurs when parsing errors happen with other fields in the config-map as well. However, the logs show a nil pointer error and that is a legit bug to be addressed in this net-contour PR: knative-extensions/net-contour#1095

Once we confirm the serving tests are passing after properly disabling the cors feature, I will open a MR in serving to unblock things. Then, later, we can work on moving the e2e tests to net-contour.

izabelacg · 2024-05-23T15:18:18Z

Closing this, as it will be Implemented by #15235

change net-contour image to cors changes only - a0f70393

482376f

knative-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 22, 2024

knative-prow bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label May 22, 2024

knative-prow bot requested review from ReToCode and skonto May 22, 2024 12:47

izabelacg added 2 commits May 22, 2024 10:49

remove cors tests

9edb396

remove cors tests from e2e script

bc6580c

add cors-policy back with logging and different location

720ff9d

knative-prow bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 22, 2024

izabelacg added 2 commits May 22, 2024 14:19

Revert "change net-contour image to cors changes only - a0f70393"

599c17f

This reverts commit 482376f.

comment all tests except cors

dff8c10

knative-prow bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 22, 2024

izabelacg added 2 commits May 22, 2024 16:36

Revert "comment all tests except cors"

1a24189

This reverts commit dff8c10.

fix command to disable cors policy for Contour tests

d39d42a

knative-prow bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 22, 2024

adjust command to disable cors policy for Contour tests

e08f84b

izabelacg mentioned this pull request May 22, 2024

Fix nil pointer dereference error when parsing config map knative-extensions/net-contour#1095

Merged

izabelacg closed this May 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] TEST-ONLY change net-contour image to cors changes only #15231

[WIP] TEST-ONLY change net-contour image to cors changes only #15231

izabelacg commented May 22, 2024 •

edited

Loading

knative-prow bot commented May 22, 2024

codecov bot commented May 22, 2024 •

edited

Loading

izabelacg commented May 22, 2024 •

edited

Loading

dprotaso commented May 22, 2024

izabelacg commented May 22, 2024

izabelacg commented May 22, 2024

izabelacg commented May 22, 2024

izabelacg commented May 22, 2024

izabelacg commented May 23, 2024

[WIP] TEST-ONLY change net-contour image to cors changes only #15231

[WIP] TEST-ONLY change net-contour image to cors changes only #15231

Conversation

izabelacg commented May 22, 2024 • edited Loading

Testing only

knative-prow bot commented May 22, 2024

codecov bot commented May 22, 2024 • edited Loading

Codecov Report

izabelacg commented May 22, 2024 • edited Loading

dprotaso commented May 22, 2024

izabelacg commented May 22, 2024

izabelacg commented May 22, 2024

izabelacg commented May 22, 2024

izabelacg commented May 22, 2024

izabelacg commented May 23, 2024

izabelacg commented May 22, 2024 •

edited

Loading

codecov bot commented May 22, 2024 •

edited

Loading

izabelacg commented May 22, 2024 •

edited

Loading