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

Condition creation & management helpers #427

Open
clux opened this issue Feb 21, 2021 · 8 comments
Open

Condition creation & management helpers #427

clux opened this issue Feb 21, 2021 · 8 comments
Labels
core generic apimachinery style work derive kube-derive proc_macro related help wanted Not immediately prioritised, please help!

Comments

@clux
Copy link
Member

clux commented Feb 21, 2021

Seeing that kubernetes is doubling down on conditions:

  • nice support in kubectl describe already
  • now new support in kubectl wait --for=condition=Available

We should have some nice helpers for them (particularly as they are a bit awkward to use). Adding to new utils label.

The ephemeron conditions show one problem; tri-state status
requiring custom deserializers/serializers. (Though maybe it's possible to expose a canonical enum?)

Another: required custom schemas (when using server side apply):

EDIT 2023:

@clux clux added the utils extra utility modules label Feb 21, 2021
@kazk

This comment has been minimized.

@clux

This comment has been minimized.

@webern

This comment has been minimized.

@webern

This comment has been minimized.

@clux clux added core generic apimachinery style work and removed utils extra utility modules labels Jun 26, 2021
@clux clux changed the title Condition helpers Condition creation helpers Nov 3, 2021
@clux clux changed the title Condition creation helpers Condition creation & management helpers Nov 3, 2021
@clux
Copy link
Member Author

clux commented Nov 3, 2021

Bringing back this discussion a bit. Hid some of the previous comments because they were off-topic or outdated.

The example created in #432 solves the basic need by providing a simple way to use meta::v1::Condition.

However, I do think there's a potential here for some more ergonomic creation helpers for conditions. Not something that is urgent, but this could be an interesting future project.

Idea: have an ergonomic wrapper derive for conditions where you select what type of common fields you want.

#[derive(kube::Condition, Serialize, Debug, PartialEq, Clone)]
pub enum MyCondition {
    #[kube(transition, message, reason)]
    PodReady,
    #[kube(transition, generation, reason)]
    Available,
}

where the enum name would give the type, the attributes would fill in the basics.

it could generate:

  • helper methods MyCondition::is_pod_ready + MyCondition::is_available
  • constructors MyCondition::new_pod_ready(message, reason) + MyCondition::new_available(generation, reason)
  • serialisation wrappers to make working with conditions a little more ergonomic (particularly the tri-state string bool)
  • implement JsonSchema for the enum / provide a way to use with #[schemars(schema_with = ...)]

Anyway posting this as an idea right now. Feedback welcome.

@clux clux added derive kube-derive proc_macro related question Direction unclear; possibly a bug, possibly could be improved. labels Nov 3, 2021
@clux clux assigned clux and unassigned clux Nov 23, 2022
@felipesere
Copy link
Contributor

I'm keen to try this over Christmas

@clux clux added help wanted Not immediately prioritised, please help! and removed question Direction unclear; possibly a bug, possibly could be improved. labels Nov 23, 2022
@clux
Copy link
Member Author

clux commented Mar 10, 2023

Putting down some reference docs for Condition as it had a KEP for 1.19 to standardise it so we should follow that:

I dunno if you still feel like doing this @felipesere but it would be awesome :-)

@juliusl
Copy link

juliusl commented Aug 16, 2023

It seems like there's been a lot of background on this subject, and so I had a bit of a hard time figuring out how to actually do this. It turned out to be a lot more straight-forward than I had expected (ignoring any prior historical context and recommended practices).

There doesn't seem to be an example that handles this particular situation so I thought I would contribute one for anyone running into this thread via SEO,

/// Updates or inserts a pod condition into pod/status,
///
/// **Note**: This does not do any validation on the `status` field of the Condition
/// 
async fn handle_pod_conditions(
    client: Client,
    ns: impl AsRef<str>,
    name: impl AsRef<str>,
    (ty, status, message, reason): (
        impl Into<String>,
        impl Into<String>,
        Option<&str>,
        Option<&str>,
    ),
    mut conditions: Vec<PodCondition>,
) -> Result<()> {
    let condition = ty.into();
    let status = status.into();

    // **Gotcha** Json Merge can only merge maps, some considerations..
    // 
    // 1) Need to enumerate the existing conditions and check if the condition type being inserted is already there, otherwise you'll create an infinite loop
    // 2) If your condition type exists, it's important to not update it unless the Status has changed, otherwise you'll create an infinite loop
    // 
    if let Some(condition) = conditions.iter_mut().find(|c| c.type_ == condition) {
        if condition.status != status {
            condition.last_transition_time = Some(Time(Utc::now()));
            condition.status = status;
            condition.message = message.map(str::to_string);
            condition.reason = reason.map(str::to_string);
        } else {
            // Nothing to update, just return early
            return Ok(());
        }
    } else {
        conditions.push(PodCondition {
            last_probe_time: None,
            last_transition_time: Some(Time(Utc::now())),
            message: message.map(str::to_string),
            reason: reason.map(str::to_string),
            status,
            type_: condition,
        });
    }

    // **Gotcha** Cannot use Api::all for this type of Patch operation
    let pods: Api<Pod> = Api::namespaced(client.clone(), ns.as_ref());
    let pp = PatchParams::default();
    let data = serde_json::json!({
        "status": {
            "conditions": conditions
        }
    });

    let _ = pods
        .patch_status(name.as_ref(), &pp, &Patch::Merge(data))
        .await?;

    Ok(())
}

async fn example_usage(client: Client, mut pod: Pod) -> Result<()> {
    if let (Some(name), Some(ns)) = (pod.metadata.name.as_ref(), pod.metadata.namespace.as_ref()) {
        if let Some(mut status) = pod.status.take() {
            if let Some(conditions) = status.conditions.take() {
                handle_pod_conditions(
                    client,
                    ns,
                    name,
                    (
                        "ExampleCondition", // Condition Type
                        "True", // Condition Status
                        None, // Message
                        Some("ConditionsAreCool"), // Reason
                    ),
                    conditions,
                )
                .await?;
            }
        }
    }

    Ok(())
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core generic apimachinery style work derive kube-derive proc_macro related help wanted Not immediately prioritised, please help!
Projects
Status: Defining
Development

No branches or pull requests

5 participants