WaitForReady should check for event type before trying to read generation number

[cardil]

Bug report

The code at location supplied behaves badly in a situation when it receives an event of type different from MODIFIED, in which the underlying object doesn't contain a status field yet.

That situation is completely normal, as ADDED event can be received first, and the resource at that point may not possess a status field yet.

client/pkg/wait/wait_for_ready.go

Lines 183 to 207 in e26d5f2

	// Check whether resource is in sync already (meta.generation == status.observedGeneration)
	inSync, err := generationCheck(event.Object)
	if err != nil {
	return false, false, err
	}
	// Skip events if generations has not yet been consolidated, regardless of type.
	// Wait for the next event to come in until the generations align
	if !inSync {
	continue
	}
	// Skip event if its not a MODIFIED event, as only MODIFIED events update the condition
	// we are looking for.
	// This will filter out all synthetic ADDED events that created bt the API server for
	// the initial state. See https://kubernetes.io/docs/reference/using-api/api-concepts/#the-resourceversion-parameter
	// for details:
	// "Get State and Start at Most Recent: Start a watch at the most recent resource version,
	// which must be consistent (i.e. served from etcd via a quorum read). To establish initial state,
	// the watch begins with synthetic “Added” events of all resources instances that exist at the starting
	// resource version. All following watch events are for all changes that occurred after the resource
	// version the watch started at."
	if event.Type != watch.Modified {
	continue
	}

Such situation I've observed while developing wait on ready based on dynamic client at knative-extensions/kn-plugin-event#38

The actual error that is being returned is similar to:

cannot extract status from &{map[metadata:map[creationTimestamp:<nil> generation:1 name:test-service] spec:map[template:map[metadata:map[creationTimestamp:<nil>] spec:map[containers:<nil>]]]]}

Expected behavior

When WaitForReady receives an event of type different from MODIFIED, it should skip it first, before checking anything in the underlying object structure.

Steps to reproduce the problem

cardil@7ca0769

Knative (serving/eventing) version

Nightly
0.24

[cardil]

/kind bug
/kind good-first-issue

[cardil]

/assign