Serving: conformance shouldn't depend on scale-to-zero

Created on 13 Jan 2020  路  22Comments  路  Source: knative/serving

In what area(s)?

/area API
/area test-and-release

/kind good-first-issue

What version of Knative?

HEAD

Expected Behavior

Conformance should stick to the public API surface of knative/serving.

Actual Behavior

The new checks look at Pods and configmaps, and fail when run against knative/serving in simply another namespace:

--- FAIL: TestProbeRuntime (0.57s)
    --- FAIL: TestProbeRuntime/httpGet (30.29s)
        readiness_probe_test.go:79: Creating a new Service
        service.go:128: Creating a new Service. service probe-runtime-http-get-egjdyqtr
        crd.go:35:  resource {<nil> <nil> <*>{&TypeMeta{Kind:,APIVersion:,} &ObjectMeta{Name:probe-runtime-http-get-egjdyqtr,GenerateName:,Namespace:,SelfLink:,UID:,ResourceVersion:,Generation:0,CreationTimestamp:0001-01-01 00:00:00 +0000 UTC,DeletionTimestamp:<nil>,DeletionGracePeriodSeconds:nil,Labels:map[string]string{},Annotations:map[string]string{},OwnerReferences:[]OwnerReference{},Finalizers:[],ClusterName:,ManagedFields:[]ManagedFieldsEntry{},} {0 <nil> <nil> <nil> <nil> {0 <nil> <nil> <*>&ObjectMeta{Name:,GenerateName:,Namespace:,SelfLink:,UID:,ResourceVersion:,Generation:0,CreationTimestamp:0001-01-01 00:00:00 +0000 UTC,DeletionTimestamp:<nil>,DeletionGracePeriodSeconds:nil,Labels:map[string]string{},Annotations:map[string]string{},OwnerReferences:[]OwnerReference{},Finalizers:[],ClusterName:,ManagedFields:[]ManagedFieldsEntry{},}} {0 <nil>}} {{0 <nil>} {<nil>   <nil> <nil>} { }}} <nil>}
        service.go:143: Waiting for Service to transition to Ready. service probe-runtime-http-get-egjdyqtr
        service.go:148: Checking to ensure Service Status is populated for Ready service
        service.go:177: Getting latest objects Created by Service
        service.go:180: Successfully created Service probe-runtime-http-get-egjdyqtr
        readiness_probe_test.go:90: Waiting for "probe-runtime-http-get-egjdyqtr-47rh5-deployment" to scale to zero
        readiness_probe_test.go:91: Could not scale to zero: failed to get autoscaler configmap: configmaps "config-autoscaler" not found

Steps to Reproduce the Problem

You can reproduce this with github.com/mattmoor/mink (ko apply -R -f config on a clean K8s cluster) then:

go test -v -count=1 -tags=e2e ./test/conformance/... --ingressClass=contour.ingress.networking.knative.dev --resolvabledomain=true
areAPI areautoscale aretest-and-release good first issue help wanted kinbug kingood-first-issue

All 22 comments

@mattmoor I will work on this one :)

/assign @hpandeycodeit

@mattmoor

I am unable to build serving on my mac:

$ go test -v -count=1 -tags=e2e ./test/conformance/... --ingressClass=contour.ingress.networking.knative.dev --resolvabledomain=true
# runtime/cgo
_cgo_export.c:3:10: fatal error: 'stdlib.h' file not found
FAIL    _/Users/proj/serving/test/conformance/api/v1 [build failed]
FAIL    _/Users/proj/serving/test/conformance/api/v1alpha1 [build failed]
FAIL    _/Users/proj/serving/test/conformance/api/v1beta1 [build failed]
FAIL    _/Users/proj/serving/test/conformance/ingress [build failed]
FAIL    _/Users/proj/serving/test/conformance/runtime [build failed]

Is this because of missing 'stdlib.h' ?

/good-first-issue

@markusthoemmes:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

/good-first-issue

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.

@hpandeycodeit does xcode-select --install solve the problem?

@hpandeycodeit, are you working on this or we need to find another person to work on this?

bump - @hpandeycodeit let us know if you're available to continue your work on this issue

@dprotaso yes, apologies for the delay in response. I will create a PR soon

hey @hpandeycodeit let us know if you're still able to work on this issue. Otherwise feel free to unassign yourself to give someone else the opportunity

sorry about that. I have unassigned myself from this issue.

I can take a peek at this one.

/assign @beemarie

looking for a little more information on understanding what the issue is here/how to recreate. If I run the test from knative/serving, i see that it passes:

    TestProbeRuntime/exec: readiness_probe_test.go:89: Waiting for "probe-runtime-exec-mhvafrpa-f9d8r-deployment" to scale to zero
--- PASS: TestProbeRuntime (0.14s)
    --- PASS: TestProbeRuntime/tcpSocket (68.41s)
    --- PASS: TestProbeRuntime/httpGet (70.39s)
    --- PASS: TestProbeRuntime/exec (90.75s)

I see @mattmoor recommended using mink for recreating -- is that required? I see mink doesn't actually have any tests, so would I run them from knative/serving instead? Thanks!

@beemarie You need to move serving into a different namespace to reproduce this. I believe our e2e tests are setup to do this, but they also set an environment variable that keeps conformance working. If you unset that environment variable for conformance, things should blow up.

cc @houshengbo who did that work IIRC

Is there any progress being made here?

I doubt if this issue is still valid.

I used to submit a PR https://github.com/knative/serving/pull/7397 in Apr to fix https://github.com/knative/serving/issues/7283 (More general ns related issue), targeting to resolve the NS related issues.
All the prow jobs generate random NS, and use it to install serving and testing serving. We have already resolved this issue.

One example can be:
Check this PR: https://github.com/knative/serving/pull/8487. The integration tests link is here: https://prow.knative.dev/view/gcs/knative-prow/pr-logs/pull/knative_serving/8487/pull-knative-serving-integration-tests/1276517212642873344
The generated NS is e85f486b-e51d-4d8c-b991-af63a7b16987.
The test TestProbeRuntime/httpGet passed:

=== RUN   TestProbeRuntime
=== RUN   TestProbeRuntime/httpGet
=== PAUSE TestProbeRuntime/httpGet
=== RUN   TestProbeRuntime/tcpSocket
=== PAUSE TestProbeRuntime/tcpSocket
=== RUN   TestProbeRuntime/exec
=== PAUSE TestProbeRuntime/exec
=== CONT  TestProbeRuntime/httpGet
=== CONT  TestProbeRuntime/tcpSocket
=== CONT  TestProbeRuntime/exec
    TestProbeRuntime/httpGet: readiness_probe_test.go:78: Creating a new Service
    TestProbeRuntime/tcpSocket: readiness_probe_test.go:78: Creating a new Service
    TestProbeRuntime/httpGet: service.go:101: Creating a new Service service probe-runtime-http-get-aaswhscb
    TestProbeRuntime/tcpSocket: service.go:101: Creating a new Service service probe-runtime-tcp-socket-xgatwznj
    TestProbeRuntime/exec: readiness_probe_test.go:78: Creating a new Service
    TestProbeRuntime/exec: service.go:101: Creating a new Service service probe-runtime-exec-ksakqwpt
    TestProbeRuntime/tcpSocket: crd.go:35:  resource {<nil> <nil> <*>{&TypeMeta{Kind:,APIVersion:,} &ObjectMeta{Name:probe-runtime-tcp-socket-xgatwznj,GenerateName:,Namespace:,SelfLink:,UID:,ResourceVersion:,Generation:0,CreationTimestamp:0001-01-01 00:00:00 +0000 UTC,DeletionTimestamp:<nil>,DeletionGracePeriodSeconds:nil,Labels:map[string]string{},Annotations:map[string]string{},OwnerReferences:[]OwnerReference{},Finalizers:[],ClusterName:,ManagedFields:[]ManagedFieldsEntry{},} {{&ObjectMeta{Name:,GenerateName:,Namespace:,SelfLink:,UID:,ResourceVersion:,Generation:0,CreationTimestamp:0001-01-01 00:00:00 +0000 UTC,DeletionTimestamp:<nil>,DeletionGracePeriodSeconds:nil,Labels:map[string]string{},Annotations:map[string]string{},OwnerReferences:[]OwnerReference{},Finalizers:[],ClusterName:,ManagedFields:[]ManagedFieldsEntry{},}} {<nil>}} {{0 <nil> <nil>} { } {<nil> <nil> <nil>}}} <nil>}
    TestProbeRuntime/exec: crd.go:35:  resource {<nil> <nil> <*>{&TypeMeta{Kind:,APIVersion:,} &ObjectMeta{Name:probe-runtime-exec-ksakqwpt,GenerateName:,Namespace:,SelfLink:,UID:,ResourceVersion:,Generation:0,CreationTimestamp:0001-01-01 00:00:00 +0000 UTC,DeletionTimestamp:<nil>,DeletionGracePeriodSeconds:nil,Labels:map[string]string{},Annotations:map[string]string{},OwnerReferences:[]OwnerReference{},Finalizers:[],ClusterName:,ManagedFields:[]ManagedFieldsEntry{},} {{&ObjectMeta{Name:,GenerateName:,Namespace:,SelfLink:,UID:,ResourceVersion:,Generation:0,CreationTimestamp:0001-01-01 00:00:00 +0000 UTC,DeletionTimestamp:<nil>,DeletionGracePeriodSeconds:nil,Labels:map[string]string{},Annotations:map[string]string{},OwnerReferences:[]OwnerReference{},Finalizers:[],ClusterName:,ManagedFields:[]ManagedFieldsEntry{},}} {<nil>}} {{0 <nil> <nil>} { } {<nil> <nil> <nil>}}} <nil>}
    TestProbeRuntime/httpGet: crd.go:35:  resource {<nil> <nil> <*>{&TypeMeta{Kind:,APIVersion:,} &ObjectMeta{Name:probe-runtime-http-get-aaswhscb,GenerateName:,Namespace:,SelfLink:,UID:,ResourceVersion:,Generation:0,CreationTimestamp:0001-01-01 00:00:00 +0000 UTC,DeletionTimestamp:<nil>,DeletionGracePeriodSeconds:nil,Labels:map[string]string{},Annotations:map[string]string{},OwnerReferences:[]OwnerReference{},Finalizers:[],ClusterName:,ManagedFields:[]ManagedFieldsEntry{},} {{&ObjectMeta{Name:,GenerateName:,Namespace:,SelfLink:,UID:,ResourceVersion:,Generation:0,CreationTimestamp:0001-01-01 00:00:00 +0000 UTC,DeletionTimestamp:<nil>,DeletionGracePeriodSeconds:nil,Labels:map[string]string{},Annotations:map[string]string{},OwnerReferences:[]OwnerReference{},Finalizers:[],ClusterName:,ManagedFields:[]ManagedFieldsEntry{},}} {<nil>}} {{0 <nil> <nil>} { } {<nil> <nil> <nil>}}} <nil>}
    TestProbeRuntime/exec: service.go:116: Waiting for Service to transition to Ready. service probe-runtime-exec-ksakqwpt
    TestProbeRuntime/tcpSocket: service.go:116: Waiting for Service to transition to Ready. service probe-runtime-tcp-socket-xgatwznj
    TestProbeRuntime/httpGet: service.go:116: Waiting for Service to transition to Ready. service probe-runtime-http-get-aaswhscb
    TestProbeRuntime/exec: service.go:121: Checking to ensure Service Status is populated for Ready service
    TestProbeRuntime/tcpSocket: service.go:121: Checking to ensure Service Status is populated for Ready service
    TestProbeRuntime/exec: service.go:127: Getting latest objects Created by Service
    TestProbeRuntime/tcpSocket: service.go:127: Getting latest objects Created by Service
    TestProbeRuntime/exec: service.go:130: Successfully created Service probe-runtime-exec-ksakqwpt
    TestProbeRuntime/exec: readiness_probe_test.go:88: Waiting for "probe-runtime-exec-ksakqwpt-57q2r-deployment" to scale to zero
    TestProbeRuntime/tcpSocket: service.go:130: Successfully created Service probe-runtime-tcp-socket-xgatwznj
    TestProbeRuntime/tcpSocket: readiness_probe_test.go:88: Waiting for "probe-runtime-tcp-socket-xgatwznj-d2nn4-deployment" to scale to zero
    TestProbeRuntime/httpGet: service.go:121: Checking to ensure Service Status is populated for Ready service
    TestProbeRuntime/httpGet: service.go:127: Getting latest objects Created by Service
    TestProbeRuntime/httpGet: service.go:130: Successfully created Service probe-runtime-http-get-aaswhscb
    TestProbeRuntime/httpGet: readiness_probe_test.go:88: Waiting for "probe-runtime-http-get-aaswhscb-bkdt5-deployment" to scale to zero
--- PASS: TestProbeRuntime (0.14s)
    --- PASS: TestProbeRuntime/tcpSocket (82.88s)
    --- PASS: TestProbeRuntime/exec (82.96s)
    --- PASS: TestProbeRuntime/httpGet (83.36s)

@mattmoor @markusthoemmes @beemarie
We should be safe to close this one. Right?

From the comment from @mattmoor -- weren't you saying there is an environment variable that is set to keep the tests passing when knative is running in a different namespace? If that env var is unset, then this test should fail, if i'm understanding correctly.

Knative serving is installed under a NS.
When we run the tests, we need to specify the same NS to test on.
As long as installation and test use the same NS, there should be no problem for knative. Otherwise, there may be issues with knative or the test cases.

However, if we install knative serving under one NS, but test on another NS, the expected behavior is tests fail.

I think the point of the issue is that ConfigMaps and Secrets etc are not part of the Knative Serving API. The conformance tests thus shouldn鈥榯 rely on being able to fetch such data via Configmaps which I believe our ScaleToZero helpers do IIRC.

@mattmoor was that the reason to open this issue in the first place?

@mattmoor, @markusthoemmes - I made a comment in the other PR as well, but do we need a new issue to remove conformance test dependence on Secrets? It looks like AddRootCAtoTransport has this dependency when https is enabled. We can open a new issue for that if I'm understanding correctly.

Was this page helpful?
0 / 5 - 0 ratings