Never Ready When Creating Env Config

reedjosh commented 6 months ago

What happened?

I created a few things with a composition pipeline and ended with auto-ready.

One of the things I created was an environment configuration.

Because the env config never really gets a status, fn auto ready never seems to do anything.

  conditions:
  - lastTransitionTime: "2024-05-06T13:17:39Z"
    reason: ReconcileSuccess
    status: "True"
    type: Synced
  - lastTransitionTime: "2024-05-06T13:19:52Z"
    message: 'Unready resources: cluster-josh-x1-env'
    reason: Creating
    status: "False"
    type: Ready

Any chance we could either tell fn-auto-ready of a list of resources to ignore, and or make fn-auto ready schema aware of objects. Particularly env-configs since it is a known k8s resource?

Thanks!

How can we reproduce it?

Create stuff without a status.

What environment did it happen in?

Function version: v0.2.1

tomaszkiewicz commented 3 months ago

+1, I have similar case when creating ProviderConfig - they don't provide any conditions and seems function-auto-ready ignores them.

tomaszkiewicz commented 3 months ago

@reedjosh I'm created a quick fix for that, tested with ProvideConfigs, but should work for any resource without conditions specified in status.

You can test that by replacing your function-auto-ready definition with my build of the function:

apiVersion: pkg.crossplane.io/v1beta1
kind: Function
metadata:
  name: function-auto-ready
spec:
  package: xpkg.upbound.io/luktom/function-auto-ready:v0.2.2

Let me know if this is what you needed :)

tomaszkiewicz commented 3 months ago

@negz can you take a look at my fix code, please? I'm quite new to Crossplane development so I'm not sure if there's a better way to solve the issue.

Also, I've tried to enforce setting the Available status for the resource like that:

// just after dr.Ready = resource.ReadyTrue
dr.Resource.SetConditions(xpv1.Available())

But it seems not working property, or I misunderstand something...

tomaszkiewicz commented 3 months ago

Above there's a link to next attempt, this time I added a function input which takes a list of GVK to force readiness, sample usage is in PR description, you can test with package xpkg.upbound.io/luktom/function-auto-ready:v0.2.4

negz commented 3 months ago

This function is mostly a convenience. Most other functions (e.g. function-go-templating, function-patch-and-transform) will let you specifically configure whether a resource is or isn't ready.

With that in mind, I lean toward keeping this function simple and not adding support for resources with no status. For those resources, I'd just configure your "primary" function (e.g. the function that actually composes the resource) to let Crossplane know the resource should be considered ready as soon as its created.

twobiers commented 3 days ago

Finding myself in the same situation (https://github.com/crossplane/crossplane/discussions/6086) and utilizing gotemplating.fn.crossplane.io/ready from function-go-templating does not fix it, because I think those resources don't have a readiness associated, therefore it's also not carried through the pipeline?

I would agree with keeping this function easy, but I'm wondering if that actually is expected behavior for users. If a composed resource does not have a ready status, would the whole composite resource be treated as unready? Personally I would vote for a implementation where the function would succeed when all available readiness checks passed and not implicitly assuming it where none is available.

Crossplane considers a composite resource (XR) to be ready when all of its desired composed resources are ready. The description from the README leaves room for interpretation. I think at least its worth documenting the intended behavior.

I would volunteer to implement this if you agree.

briferz commented 1 day ago

Hello,

I'm facing exactly the same issue with a ProviderConfig. I tried to add a step after auto-ready containing said ProviderConfig in the hopes that this function wouldn't account for it, without success. I'm using the go-templating function but haven't been able to figure out how to manually set the XR's status to Ready (even though there's a dummy example in its doc).

If the key goal is to keep the function simple, what about adding a special autoready.fn.crossplane.io/ready annotation to flag those particular resources as specified by such annotation? This way there isn't even need to update any contract.

phisco commented 1 day ago

A few things:

The Composite Resource (XR) is considered ready when all its Composed Resources are reported as ready from the pipeline or, since https://github.com/crossplane/crossplane/pull/6021, when the XR itself is explicitly reported as ready.- Resource readiness reported from the pipeline is not the Ready condition, they are two different things, this function is automatically setting Composed Resources readiness based on the Ready condition, but other functions could have different opinions.
There is no way to tell apart a resource with no status at all from one that doesn't have a status yet; therefore, we can't automatically detect such resources. We'd have to hardcode known types to ignore, which is obviously not scalable.

The nice thing about functions is that they are composable, so if someone wants to build a function, e.g. function-always-ready, which would always set as ready specific resources matching some selector, either by GVK, labels or name, that would definitely be a great addition to the ecosystem 🙏

twobiers commented 1 day ago

There is no way to tell apart a resource with no status at all from one that doesn't have a status yet; therefore, we can't automatically detect such resources. We'd have to hardcode known types to ignore, which is obviously not scalable.

Ok, I see the problem now. I think your idea of a function-always-ready would work well, but I still feel like this only masks the problem, especially since the same is true for the Synced condition. Basically, when any composite resource not having a Synced and Ready status the XR will never be in that state. And besides ProviderConfig, Environment there are also other valid candidates like Secret and ConfigMap.

I think the problem lays deeper that XRs support those conditions, while the resources the status depends on must not. This leads to unexpected behavior for the user as here in the issue and an additional function would only mask the problem. Maybe a fair assumption the crossplane controllers could make is that an XR considers a composite resource as ready when

it does not have any condition at all OR
if its ready/synced condition = True I still don't think this is a perfect solution though. Maybe there's another option.

crossplane-contrib / function-auto-ready