watcher on 0.92 does not get all objects during Init

[jfeldman325]

Current and expected behavior

in 0.92 if you initialize a watcher with an Api object created with Api::all the watcher only reflects objects withing the current namespace into the store. Example:

    let subscriber = pod_writer
        .subscribe()
        .expect("subscribers can only be created from shared stores");
    // (2.5): create a watch stream
    let config = Config::default().concurrency(0);
    let new_client = Client::try_default().await.unwrap();
    let pods: Api<Pod> = Api::all(new_client);

    let pod_watch = watcher(pods, watcher::Config::default())
        .default_backoff()
        .reflect_shared(pod_writer)
        .modify(|pod| {
            pod.managed_fields_mut().clear();
            pod.spec.as_mut().unwrap().containers = vec![]; // We don't need any of this info, reduce memory footprint
        });

This snippit in 0.92 once the reflector returns as ready only contains pods from the current namespace when calling reader.state() , reverting to 0.91 fixes the issue and returns pods across all namespaces.

Possible solution

No response

Additional context

No response

Environment

Client Version: v1.28.4
Kustomize Version: v5.0.4-0.20230601165947-6ce0bf390ce3
Server Version: v1.24.12

Configuration and features

No response

Affected crates

No response

Would you like to work on fixing this bug?

None

[clux]

uhh...wtf. this is actually not just related to reflectors (or the unstable stream sharing api that's included above), but seemingly watchers in general. tried using the pod_watcher example and modified the Api::namespaced to an Api::all and i am only getting a subset of the pods...

thanks for reporting this!

[jfeldman325]

uhh...wtf. this is actually not just related to reflectors (or the unstable stream sharing api that's included above), but seemingly watchers in general. tried using the pod_watcher example and modified the Api::namespaced to an Api::all and i am only getting a subset of the pods...

thanks for reporting this!

No problem! It was very confusing to debug since using the api object to simply list all still returns the expected result!

[clux]

Confirming that this is a result of one of the many watcher changes in 0.92, yes.

The bug in the InitPage watcher state specifically going straight to InitListed even though we just iterated through one page.

kube/kube-runtime/src/watcher.rs

Lines 490 to 505 in f553c4e

	State::InitPage {
	continue_token,
	mut objects,
	last_bookmark,
	} => {
	if let Some(next) = objects.pop_front() {
	return (Some(Ok(Event::InitApply(next))), State::InitPage {
	continue_token,
	objects,
	last_bookmark,
	});
	}
	if let Some(resource_version) = last_bookmark {
	// we have drained the last page - move on to next stage
	return (Some(Ok(Event::InitDone)), State::InitListed { resource_version });
	}

it should be starting a new api.list from the previous continue_token (basically a fix is to check for whether continue_token.is_some() before returning InitListed.

[clux]

Will have to do a patch release for this. It breaks store completeness and is a pretty bad bug. Possibly even yank worthy after a patch release.

As for why we never saw this:

• unit tests did not catch this because they typically do a single page out of laziness and then check that regular watch events follow (which they do).
• all the controllers i manually e2e tested, run on frequently updated objects, meaning the issue would "self-resolve" in some sense (eventually you'd get all objects)
• mock test should have caught this, but it had a hole in its tests that only verified the continue token once (where it didn't matter)

we do need a better test case for this long term though. separate issue.