kube icon indicating copy to clipboard operation
kube copied to clipboard

Condition creation & management helpers

Open clux opened this issue 4 years ago • 8 comments

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):

  • https://github.com/qualified/ephemeron/blob/b507360232fcdc3d0a1f97abc93fabdd41c9a5bf/src/resource/mod.rs#L40
  • https://github.com/qualified/ephemeron/blob/b507360232fcdc3d0a1f97abc93fabdd41c9a5bf/src/resource/schemas.rs#L23-L30

EDIT 2023:

  • cross reference designs with kstatus which have opinions on status and conditions
  • used by flux's runtime: https://pkg.go.dev/github.com/fluxcd/pkg/runtime#readme-working-with-conditions

clux avatar Feb 21 '21 21:02 clux

requiring custom deserializers/serializers.

Not required. Condition can be struct Condtion { type_: String, status: String } and #[derive(JsonSchema)] will work too.

I did that first, but it's awkward to work with. Also considered enum ConditionStatus { Unknown, True, False }, but Option<bool> felt natural. I ended up with that because custom de/serialization is simple and server side apply needed custom schema anyway.


By the way, I've been thinking of forking schemars. It might make sense because:

  • attributes for Kubernetes extensions (this can be used instead of schema_with for simple conditions)
  • attributes for validations (not all are useful outside of Kubernetes like +kubebuilder:validation:EmbeddedResource)
  • structural schema (not sure what's needed yet, but at least failing to compile is probably better)
  • the author doesn't seem active anymore, so PRs might not be merged
  • JsonSchema is misleading

kazk avatar Feb 21 '21 23:02 kazk

Hehe fair. That makes sense.

A schemars fork makes a lot of sense, agree with the points. I had considered getting a light-weight thing inside the derive macro crate at first, but then you got us 90% of the way there with schemars out of the box :-) But then it's hard to get the last 10% without a maintainer.

clux avatar Feb 22 '21 00:02 clux

Is there an example of how to set the status of a CRD object, including Conditions? I guess that example would be in kube-runtime, but my search brought me here.

webern avatar Jun 02 '21 21:06 webern

Oh, I noticed this:

#[kube(status = "FooStatus")]

Here: https://github.com/clux/controller-rs/blob/7a9c03ba0959663a9ac6e85706ed37261187b000/src/manager.rs#L29

Which helps me understand what's happening a lot better.

Edit: and further down in that file is the exact example of how to patch the status field.

webern avatar Jun 02 '21 21:06 webern

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 avatar Nov 03 '21 22:11 clux

I'm keen to try this over Christmas

felipesere avatar Nov 23 '22 09:11 felipesere

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 :-)

clux avatar Mar 10 '23 07:03 clux

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(())
}

juliusl avatar Aug 16 '23 23:08 juliusl