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

Never Ready When Creating Env Config #29

Open
reedjosh opened this issue May 6, 2024 · 9 comments · May be fixed by #33
Open

Never Ready When Creating Env Config #29

reedjosh opened this issue May 6, 2024 · 9 comments · May be fixed by #33
Labels
bug Something isn't working

Comments

@reedjosh
Copy link

reedjosh commented May 6, 2024

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

@reedjosh reedjosh added the bug Something isn't working label May 6, 2024
@tomaszkiewicz
Copy link

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

@tomaszkiewicz
Copy link

@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
Copy link

@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
Copy link

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
Copy link
Member

negz commented Jul 30, 2024

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
Copy link

twobiers commented Nov 11, 2024

Finding myself in the same situation (crossplane/crossplane#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
Copy link

briferz commented Nov 13, 2024

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
Copy link

phisco commented Nov 13, 2024

A few things:

  • The Composite Resource (XR) is considered ready when all its Composed Resources are reported as ready from the pipeline or, since feat(xfn): Consider composite ready state in function response crossplane/crossplane#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
Copy link

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
1. it does not have any condition at all
OR
2. if its ready/synced condition = True
I still don't think this is a perfect solution though. Maybe there's another option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
6 participants