kubernetes-sigs / kubebuilder-declarative-pattern

A toolkit for building declarative operators with kubebuilder
Apache License 2.0
256 stars 85 forks source link

Extend kstatusAggregator to support custom/unconventional resources #365

Closed tatehanawalt closed 8 months ago

tatehanawalt commented 9 months ago

What would you like to be added:

kstatusAggregator, kstatusAggregator.BuildStatus extended to support aggregate status where a subset of resources are custom/unconventional, through user supplied GVK specific compute and abnormal conditions handler methods.

Why is this needed:

kstatusAggregator.BuildStatus aggregates status over underlying resources, but does not support handling of custom/unconventional resources.

Users can implement their own BuildStatus method, but that requires users to implement an entire BuildStatus method. kstatusAggregator.BuildStatus can be minimally modified to support custom/unconventional resources without requiring users to implement an entirely custom BuildStatus method.

Example: Our operator creates resources, some of which are maintained by a 3rd party operator which does not appear to follow abnormal-true conventions. Additionally, errors of these 3rd party resources may be present in the resource status, without corresponding status conditions reflecting those errors.

How we achieved this in practice: kstatusAggregator.BuildStatus derives status for individual resources in calls to Compute for a given resource.

Compute is an implementation of GetConditionsFn.

kstatusAggregator can support custom/unconventional resources by allowing users to specify GVK specific GetConditionsFn methods which would be called in place of Compute.

This is akin to allowing users to define something like the legacyTypes map, where a handler method can be specified for specific GVKs per the user's requirements.

If no compute method is specified for a specific GVK string, the existing compute method is used in the same way it is currently called. This makes the proposed BuildStatus changes reverse compatible.

kstatusAggregator.BuildStatus also uses a method to extract abnormal conditions of a given resource. In order for kstatusAggregator.BuildStatus to support custom/unconventional resources, users must also be able (but not required) to specify a method to derive AbnormalConditions for a given GVK.

type AbnormalConditionsMethod func (ctx context.Context, unstruct *unstructured.Unstructured) []status.Condition

type kstatusAggregator struct {
    // Map of GVK specific 'Compute' methods. Methods find the status of 
    // a given unstructured resource. 
    //
    // The returned result contains the status of the resource, which will be
    // one of
    //   - InProgress
    //   - Current
    //   - Failed
    //   - Terminating
    //
    // It also contains a message that provides more information on why
    // the resource has the given status. Finally, the result also contains
    // a list of standard resources that would belong on the given resource.
    GVKComputeMethods                    map[string]status.GetConditionsFn

    // Map of GVK specific methods which return the set of abnormal conditions for a given resource
    // abnormal conditions reflect any condition indicating an other than nominal state
    GVKAbnormalConditionsMethods map[string]AbnormalConditionsMethod
}

func NewOpenKstatusAgregator(gvkCMs map[string]status.GetConditionsFn, gvkACMs  map[string]AbnormalConditionsMethod) *kstatusAggregator {
    return &kstatusAggregator{
        GVKComputeMethods:                    gvkCMs,
        GVKAbnormalConditionsMethods: gvkACMs,
    }
}

BuildStatus could then be modified such that if a user has configured a GVK specific compute and/or abnormal conditions method, the gvk specific methods are called and the existing Compute and abnormal conditions methods are called otherwise.

For the compute method call, this looks like:

// https://github.com/kubernetes-sigs/kubebuilder-declarative-pattern/blob/6f379365565f641f0ac05b533c8fb6d57509776b/pkg/patterns/addon/pkg/status/kstatus.go#L23
func (k *kstatusAggregator) BuildStatus(ctx context.Context, info *declarative.StatusInfo) error {
    ...
    // From
    // res, err := status.Compute(unstruct)

    // To
    var gvkComputeMethod status.GetConditionsFn = status.Compute
    if computeMethod, ok := k.GVKComputeMethods[gvk.String()];  ok && computeMethod != nil {
        gvkComputeMethod = computeMethod
    }
    res, err := gvkComputeMethod(unstruct)
    ...
}

For the abnormal condition method call, this looks like:

// https://github.com/kubernetes-sigs/kubebuilder-declarative-pattern/blob/6f379365565f641f0ac05b533c8fb6d57509776b/pkg/patterns/addon/pkg/status/kstatus.go#L23
func (k *kstatusAggregator) BuildStatus(ctx context.Context, info *declarative.StatusInfo) error {
    ...
    // From
    // conds := getAbnormalConditions(ctx, unstruct)

    // To
    var getConditionsMethod AbnormalConditionsMethod = getAbnormalConditions
    if conditionsMethod, ok := k.GVKAbnormalConditionsMethods[gvk.String()];  ok && conditionsMethod != nil {
        getConditionsMethod = conditionsMethod
    }
    conds := getConditionsMethod(unstruct)
    ...
}

GVK specific compute and abnormal conditions methods are then specified in the kstatusAggregator initialization.

func (r *Reconciler) Init(mgr ctrl.Manager) error {
    ...
    // initialize GVK x method maps
    computMethods := make(map[string]GetConditionsFn)
    conditionsMethods := make(map[string]AbnormalConditionsMethod)

    // Specify methods for specific GVK
    resourceGVK := schema.GroupVersionKind{...}
    computMethods[resourceGVK.String()] = <user supplied 'Compute' method>
    conditionsMethods[resourceGVK.String()] = <user supplied Abnormal Conditions method>

     // Initialize kstatusAggregator with gvk specific compute and abnormal conditions methods
    statusBuilder := &declarative.StatusBuilder  {
        BuildStatusImpl: status.NewOpenKStatusAggregator(computeMethods, abnormalConditionsMethods),
    }

    ...
    return r.Reconciler.Init(
        ...
        WithStatus(statusBuilder),
    }
}
atoato88 commented 8 months ago

This idea looks good :+1: Feel free to mention reviewers once you create PR(s) about this.

tatehanawalt commented 8 months ago

Awesome thanks @atoato88

I have opened a PR with this issue linked in the summary https://github.com/kubernetes-sigs/kubebuilder-declarative-pattern/pull/372