Watcher breaks if any object cannot be deserialized

[nightkr]

Current and expected behavior

Let's say that we define the CRD spec as follows (this is just illustrative of the problem, I haven't tried compiling this...):

#[derive(CustomResource)]
#[kube(group = "dinner.com", version = "v1alpha1", kind = "Food")]
pub struct FoodSpec {
    price: i8,
}

And apply the generated CRD, as well as two objects of the specified type:

apiVersion: dinner.com/v1alpha1
kind: Food
metadata:
  name: eggs
spec:
  price: 10
---
apiVersion: dinner.com/v1alpha1
kind: Food
metadata:
  name: spam
spec:
  price: -5

So far, everything works fine and our watcher reports the object just fine.

However, eventually some smartass decides that negative prices don't make sense, and changes the FoodSpec definition:

#[derive(CustomResource)]
#[kube(group = "dinner.com", version = "v1alpha1", kind = "Food")]
pub struct FoodSpec {
    price: u8,
}

Now we have some orphaned objects that would no longer validate, but which have effectively been grandfathered in. watcher sees the error and fails immediately, swallowing all objects if eggs could have been parsed just fine. This percolates down the ecosystem, and causes Controller to fail in the same way.

Possible solution

I'm not sure. Maybe we watcher should return some kind of Result<K, Undeserializable>? Then again, that would probably hurt ergonomics quite a bit... How should reflector handle this case? We might also want to split between "object is A-OK", "metadata is OK, but the rest is u̵͚͘ͅn̶̡̩̎̑r̵̺̦̎ḙ̴̲͐ă̴͇͍̌ḑ̵́̂a̴̻̔͋b̵̤̳̈́̔l̵̙̪̏͠ĕ̶͔́ͅ", and "z̶̜͛̅̕a̴̛̘͋̐l̴̢̘̹̂g̵̩͖̏̎ō̸̰ ̵̣͔̖͛̄c̴͖̒ö̷̰́ṃ̷̧̎͗é̷̠̟̟̐s̶̖̖̬̓͌͆ ̵̫͈́̈́f̴̢̼̓o̵̗̔͠r̵̥͐͌ ̴͓̽̍ù̷͎̑s̴̺͍̙͋ ̵̫̻̦̕a̶̡̓l̶̙̃̑͗l̵̝̊͝".

This will probably also need to change in Api::list and Api::watch somehow...

Additional context

No response

Environment

This is a client issue.

Configuration and features

No response

Affected crates

kube-client, kube-runtime

Would you like to work on fixing this bug?

yes

[clux]

Now we have some orphaned objects that would no longer validate, but which have effectively been grandfathered in. watcher sees the error and fails immediately, swallowing all objects if eggs could have been parsed just fine. This percolates down the ecosystem, and causes Controller to fail in the same way.

I think it is probably better to take advantage of kubernetes versioning properly here, rather than build us into a type-corner, because the behaviour there of changing the schema to something not supporting the existing types is pretty bad behaviour.

There are a few things kubernetes has that can help us here if we use crd versioning better, in particular kubectl convert, and ConversionReview.

ConversionReview (afaikt) is an optional part of an admission controllers that is in charge of migrating between versions. We don't have an example of how to do this, nor do we have structs for it (and they are not in k8s-openapi - so we likely have to handroll them like AdmissionReview). So maybe we should try to instead support this, and let people who perform illegal updates to the CRD deal with their own problems.

Maybe down the line kubernetes will be able to help with the migrations for you with schemas (it already does pruning).

[nightkr]

Agreed that we need that too, and that creating a new version is the correct way to deal with this kind of breakage.