ContainerMissing is no longer reported in Condition when an invalid image path is specified

[jessiezcc]

/area API
/kind dev

Expected Behavior

Some recent change broke TC ContainerErrorMsg.
It expects to see LatestRevisionReady Condition to have Reason as ContainerMissing

Actual Behavior

The reason shows "Unknown" in my repro.
And in Brenda's repro, it shows as RevisionFailed.
The test was passing as of Jun.8th.
"
status:
conditions:
- lastTransitionTime: null
message: 'revision "prodrxavxzdc-00001" failed with message: MANIFEST_UNKNOWN:
"Failed to fetch "latest" from request "/v2/cf-elafros-dog/invalidhelloworld/manifests/latest"."'
reason: RevisionFailed
status: "False"
type: LatestRevisionReady"

Steps to Reproduce the Problem

1. go test -v ./test/e2e -tags=e2e -run ContainerErrorMsg --logverbose

Additional Info

I0615 14:29:41.736487 184415 request.go:874] Response Body: {"apiVersion":"serving.knative.dev/v1alpha1","kind":"Configuration","metadata":{"clusterName":"","creationTimestamp":"2018-06-15T21:29:31Z","generation":0,"labels":{"serving.knative.dev/route":"noodleburgjgpaysdk"},"name":"prodssxudvlo","namespace":"noodleburg","resourceVersion":"11307300","selfLink":"/apis/serving.knative.dev/v1alpha1/namespaces/noodleburg/configurations/prodssxudvlo","uid":"30dd9435-70e3-11e8-bae2-42010af00105"},"spec":{"generation":1,"revisionTemplate":{"metadata":{"creationTimestamp":null},"spec":{"concurrencyModel":"Multi","container":{"image":"gcr.io/kubertry-200221/invalidhelloworld","name":"","resources":{}}}}},"status":{"conditions":[{"lastTransitionTime":"2018-06-15T21:29:31Z","status":"Unknown","type":"LatestRevisionReady"},{"lastTransitionTime":"2018-06-15T21:29:31Z","status":"Unknown","type":"Ready"}],"latestCreatedRevisionName":"prodssxudvlo-00001","observedGeneration":1}}

[mattmoor]

/assign @mattmoor

[jessiezcc]

looks like all the error info are gone now, also checked with unauthorized container path, no longer see unauthorized msg as well

[mattmoor]

I believe that this is a test bug. We see the initialization of the conditions to Unknown and check their status immediately. We should predicate on .Status != corev1.ConditionUnknown.

I will send a fix for the tests and re-enable them soon.

[jessiezcc]

In my offline tests, I inserted sleep for 10 sec, still didn't see expected condition field values. Did you actually see the fields populated with desired values?

[mattmoor]

Ok, it looks like there's a mix of problems. We also now have a Reason: RevisionFailed instead of Reason: ContainerMissing (I suspect this is still the latter at the Revision level, but not at the Configuration level).

[mattmoor]

Yeah, if I launch sample/helloworld/sample.yaml via kubectl instead of ko I get this at the Revision level:

    - lastTransitionTime: 2018-06-18T23:02:42Z
      message: 'unsupported status code 404; body: Not Found'
      reason: ContainerMissing
      status: "False"
      type: Ready

... and this at the Configuration level:

    - lastTransitionTime: 2018-06-18T23:03:02Z
      message: 'revision "configuration-example-00001" failed with message: unsupported
        status code 404; body: Not Found'
      reason: RevisionFailed
      status: "False"
      type: Ready

I will check the spec to see if we're required to propagate the Reason.

[mattmoor]

Yeah, the Reason of ContainerMissing is only specified for revisions.

I am going to relax this check. cc @jonjohnsonjr