generalization of status generation UID processing

[joel-bluedata]

We're doing the same thing over and over for each new CR we handle. It's about time to factor this out. This involves:

• The status gen check (handleNewCluster etc.) for an incoming CR in the handler.
• The status gen update at the end of the handler (in the defer).
• The status gen check and validation in the validator.

I had some comments on PR #208 about that first bullet point, which I will reproduce here:

Much of the "handleNewWhatever" function can be generalized... convert the CR into an unstructured object, pull out the object UID and status generation UID, and do stuff. The code to get the UID and status gen is a little extensive but whatever, that would be fine if it actually cleaned up the rest of the codebase.

A few things nagging at me about that:

• Maybe instead of making this shared code generically deal with a passed-in object, it should rely on the caller to pass in the object UID and any status gen. This would simplify the shared code a lot and seems sensible at first glance, although it does reduce the utility of the shared code.
• The cr.Status.State update (and specific event logging) would probably need to be done in the caller, so that part wouldn't get pushed into the shared code and it would need to be driven by certain condition checks in the caller.
• For a cluster CR, handleNewCluster does things other than check the status gen; it also ensures an app reference halfway through that process. Probably something else that would be done in the caller under certain conditions after return from the shared code.

At this point I'm not sure how much simpler the calling code will be. It may be still be worthwhile to factor some of this into shared code though. The other big issue however is that the whole status gen processing relies not only on this code that checks incoming CRs, but also some stuff in the validator and some stuff in the deferred func at the end of the handler. Proper generalizing of status gen UID handling should encapsulate all of that.

[joel-bluedata]

And I'll put here my experiment in generalizing handleNewFoo (as CheckStatusGen below), taking any CR object as input. As mentioned above I'm not sure this is the way to go but I don't want to lose this yet. This is NOT code that works/compiles/whatever, just my sketch on what I think it might be like.

import (
    "k8s.io/apimachinery/pkg/runtime"
    "k8s.io/apimachinery/pkg/types"
)

func getInterfaceValue(
    obj map[string]interface{},
    propname string,
    required bool,
    propPathDescription string,
    objDescription string,
) (interface{}, error) {

    valueInterface, hasValue := obj[propname]
    if !hasValue {
        if !required {
            return nil, nil
        }
        missingPropErr := fmt.Errorf(
            "%s property is missing from %s object",
            propPathDescription,
            objDescription,
        )
        return nil, missingPropErr
    }
    return valueInterface, nil
}

func getObjValue (
    obj map[string]interface{},
    propname string,
    required bool,
    propPathDescription string,
    objDescription string,
) (map[string]interface{}, error) {

    valueInterface, missingValueErr := getInterfaceValue(
        obj,
        propname,
        required,
        propPathDescription,
        objDescription,
    )
    if missingValueErr != nil {
        return nil, missingValueErr
    }
    value, valueTypeOk := valueInterface.(map[string]interface{})
    if !valueTypeOk {
        valueTypeErr := fmt.Errorf(
            "%s property of %s object has unexpected value",
            propPathDescription,
            objDescription,
        )
        return nil, valueTypeErr
    }
    return value, nil
}

func getStringValue (
    obj map[string]interface{},
    propname string,
    required bool,
    propPathDescription string,
    objDescription string,
) (*string, error) {

    valueInterface, missingValueErr := getInterfaceValue(
        obj,
        propname,
        required,
        propPathDescription,
        objDescription,
    )
    if missingValueErr != nil {
        return nil, missingValueErr
    }
    value, valueTypeOk := valueInterface.(string)
    if !valueTypeOk {
        valueTypeErr := fmt.Errorf(
            "%s property of %s object has unexpected value",
            propPathDescription,
            objDescription,
        )
        return nil, valueTypeErr
    }
    return &value, nil
}

func getUIDValue (
    obj map[string]interface{},
    propname string,
    required bool,
    propPathDescription string,
    objDescription string,
) (*types.UID, error) {

    valueInterface, missingValueErr := getInterfaceValue(
        obj,
        propname,
        required,
        propPathDescription,
        objDescription,
    )
    if missingValueErr != nil {
        return nil, missingValueErr
    }
    value, valueTypeOk := valueInterface.(types.UID)
    if !valueTypeOk {
        valueTypeErr := fmt.Errorf(
            "%s property of %s object has unexpected value",
            propPathDescription,
            objDescription,
        )
        return nil, valueTypeErr
    }
    return &value, nil
}

func CheckStatusGen(
    reqLogger logr.Logger,
    cr interface{},
    crDescription string,
    statusGens *StatusGens,
) error {

    // First convert the structured CR object to a map of properties.
    crPropMap, crPropMapErr :=
        runtime.DefaultUnstructuredConverter.ToUnstructured(cr)
    if crPropMapErr != nil {
        return crPropMapErr
    }

    // Find the object's UID. This is required!
    metadata, metadataErr := getObjValue(
        crPropMap,
        "metadata",
        true,
        "metadata",
        crDescription,
    )
    if metadataErr != nil {
        return metadataErr
    }
    uid, uidErr := getUIDValue(
        metadata,
        "uid",
        true,
        "metadata/uid",
        crDescription,
    )
    if uidErr != nil {
        return uidErr
    }

    // And let's get the incoming status generation UID, if any. This is not
    // required to exist yet (for newly created objects).
    var *string incomingSGU = nil
    status, statusErr := getObjValue(
        crPropMap,
        "status",
        false,
        "status",
        crDescription,
    )
    if statusErr != nil {
        return statusErr
    }
    if status != nil {
        sgu, sguErr := getUIDValue(
            status,
            "generation_uid",
            false,
            "status/generation_uid",
            crDescription,
        )
        if sguErr != nil {
            return sguErr
        }
        if sgu != nil {
            incomingSGU = sgu
        }
    }

    // See if we already know an SGU value for this object.
    prevSGU, hasPrevSGU := statusGens.ReadStatusGen(*uid)

    // If so, then probably we have nothing to do here. However we should
    // sanity-check that the SGU is what we expect... it REALLY should be, but
    // if there is a bug/race in the client code or unexpected behavior of
    // the K8s API consistency then it might not be.
    if hasPrevSGU {
        if (incomingSGU != nil) && (prevSGU.UID == *incomingSGU) {
            // Everything is as we expect.
            return nil
        }
        // SGU mismatch. Return an error.
        shared.LogInfof(
            reqLogger,
            cr,
            shared.EventReasonNoEvent,
            "ignoring %s object with stale status generation UID; will retry",
            crDescription,
        )
        mismatchErr := fmt.Errorf(
            "%s incoming UID %s != last known UID %s",
            crDescription,
            incoming,
            prevSGU.UID,
        )
        return mismatchErr
    }

    // So we don't have an expected SGU value. If there is also no incoming
    // SGU value, this is a newly-created object... nothing to do here.
    if incomingSGU == nil {
        return nil
    }

    // Final case: there's an SGU in the object but we haven't cached it yet.
    // Do so now!
    shared.LogInfof(
        reqLogger,
        cr,
        shared.EventReasonNoEvent,
        "previously-unseen %s with incoming status generation UID %s",
        crDescription,
        *incomingSGU,
    )
    statusGens.WriteStatusGen(*uid, *incomingSGU)
    statusGens.ValidateStatusGen(*uid)
    return nil
}