Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add general conditions::is_met helper to runtime::wait #669

Open
clux opened this issue Oct 24, 2021 · 3 comments · May be fixed by #679
Open

add general conditions::is_met helper to runtime::wait #669

clux opened this issue Oct 24, 2021 · 3 comments · May be fixed by #679
Labels
core generic apimachinery style work question Direction unclear; possibly a bug, possibly could be improved. runtime controller runtime related

Comments

@clux
Copy link
Member

clux commented Oct 24, 2021

would like to have a generic helper to the conditions module so we don't have to special case every condition matching thing in there. something like:

pub fn is_met<K: Resource>(condition: &str, required_value: &str) -> impl Condition<K> {
    todo!()
}

unfortunately, the only way we can do it atm AFAIKT is to require Serialize + Deserialize, then use the serde derives to parse the object as yaml/json, then safely try to check if the path .spec.conditions path exists and contains an object with cont[r#type] == condition and return true if condition + path exists and cond[r#type].status == required_value.

long term though, i think we could do a trait from codegen like HasConditions: HasSpec (we already have HasSpec and HasStatus) that lets you reach into that path and get a vector of maps because then we could use that trait and not have to do a pointless serialize+deserialize.

we could do the simple hack as a stop-gap, but we could also do a trait for it already in kube-core. wdyt?

@clux clux added question Direction unclear; possibly a bug, possibly could be improved. runtime controller runtime related core generic apimachinery style work labels Oct 24, 2021
@nightkr
Copy link
Member

nightkr commented Oct 24, 2021

Agreed with the general idea.. A few points though:

  1. Can't we just use DynamicObject internally, rather than going through the reserialization dance?
  2. IIRC value is a bool, not a string?
  3. Starting to think we might want to have a way to combine conditions…

@clux
Copy link
Member Author

clux commented Oct 24, 2021

  1. ooh, yeah, good call. we should implement this for DynamicObject initially :-)
  2. value is a strinigfied bool or tri-bool (possibly more), see https://docs.rs/k8s-openapi/0.13.1/k8s_openapi/api/apiserverinternal/v1alpha1/struct.StorageVersionCondition.html#structfield.status
  3. yeah, a combinator there shouldn't be too hard!

@nightkr
Copy link
Member

nightkr commented Oct 26, 2021

Ahh, fair enough.

nightkr added a commit to nightkr/kube-rs that referenced this issue Oct 26, 2021
Super unhappy with the API, but it's a start...

Fixes kube-rs#669

Signed-off-by: Teo Klestrup Röijezon <[email protected]>
@nightkr nightkr linked a pull request Oct 26, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core generic apimachinery style work question Direction unclear; possibly a bug, possibly could be improved. runtime controller runtime related
Projects
Status: Backlog
Development

Successfully merging a pull request may close this issue.

2 participants