Skip to content

Commit 20115c7

Browse files
Say a resource was kept in a condition, not a warning every reconcile
When a value a resource is built from goes missing and the scope keeps the resource's current spec, it said so in a warning result. That's right the first time, but it repeats on every reconcile for as long as the value is missing - through a whole cluster teardown, say - and each one is a Warning event on the XR. composing now reports on an XR condition, DependencyValuesAvailable: False with reason KeptCurrentSpec while any resource was kept, naming each and the value it's waiting for, and True otherwise. Every scope reports, so the condition is returned on every reconcile: Crossplane keeps a condition a function set earlier until it's set again, so one returned only while False would never clear. Scopes in one function share it, and one with nothing kept doesn't turn another's False back to True. The message is built from resource names and field paths only, so it holds still while the situation does, and Crossplane leaves an unchanged condition alone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 38b5b91 commit 20115c7

2 files changed

Lines changed: 124 additions & 16 deletions

File tree

‎crossplane/function/dependency.py‎

Lines changed: 50 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,13 @@
6666
_EXTERNAL_NAME = "@externalName"
6767
_EXTERNAL_NAME_ANNOTATION = "crossplane.io/external-name"
6868

69+
CONDITION_TYPE = "DependencyValuesAvailable"
70+
"""The XR condition composing reports whether any resource was kept.
71+
72+
False while a resource was kept at its current spec because a value it's
73+
built from isn't available, naming each one. True otherwise.
74+
"""
75+
6976
_COMPOSED = "composed"
7077
_REQUIRED = "required"
7178

@@ -373,6 +380,7 @@ def __init__(
373380
self.name = name
374381
self._sources: set[_Source] = set()
375382
self._unresolved: list[str] = []
383+
self._kept = False
376384

377385
def ref(self, value: V) -> V:
378386
"""Read a field of a named resource, and depend on that resource.
@@ -488,14 +496,17 @@ def _close(self) -> None:
488496
if self.name in self.rsp.desired.resources or exists:
489497
_record(self.rsp, self.name, self._sources)
490498

499+
_report(self.rsp, self.name, self._unresolved if self._kept else [])
500+
491501
def _keep_current_spec(self) -> None:
492502
"""Keep an existing resource's spec while a reference it needs is gone.
493503
494504
The fields that refer to what's missing came back None and were left
495505
out, and applying that would unset them. A function that didn't
496506
compose the resource at all, because the value it needed wasn't
497507
there, would have it deleted. Either way, keep what the resource
498-
already has for anything this function doesn't set, and say so.
508+
already has for anything this function doesn't set, and say so in
509+
the CONDITION_TYPE condition.
499510
"""
500511
observed = resource.struct_to_dict(
501512
self.req.observed.resources[self.name].resource
@@ -517,12 +528,39 @@ def _keep_current_spec(self) -> None:
517528
body["spec"] = observed["spec"]
518529
resource.update(self.rsp.desired.resources[self.name], body)
519530

520-
response.warning(
521-
self.rsp,
522-
f"{self.name}: kept its current spec, because "
523-
f"{', '.join(self._unresolved)} isn't available",
531+
self._kept = True
532+
533+
534+
def _report(rsp: fnv1.RunFunctionResponse, name: str, missing: list[str]) -> None:
535+
"""Report on CONDITION_TYPE for one scope.
536+
537+
Every scope reports, so the condition is always returned: Crossplane
538+
keeps a condition a function set earlier until the function sets it
539+
again, so one returned only while something was kept would stay False
540+
after it recovered. Scopes in one function share the condition, which
541+
turns False on the first that kept its resource and names each.
542+
543+
The message is built from resource names and field paths only, so it
544+
holds still while the situation does. Crossplane then leaves the
545+
condition alone, where a result would be an event every reconcile.
546+
"""
547+
c = next((c for c in rsp.conditions if c.type == CONDITION_TYPE), None)
548+
if c is None:
549+
c = rsp.conditions.add(
550+
type=CONDITION_TYPE, status=fnv1.STATUS_CONDITION_TRUE, reason="Available"
524551
)
525552

553+
if not missing:
554+
return
555+
556+
line = f"{name} kept its current spec: {', '.join(missing)} isn't available"
557+
if c.status == fnv1.STATUS_CONDITION_TRUE:
558+
c.status = fnv1.STATUS_CONDITION_FALSE
559+
c.reason = "KeptCurrentSpec"
560+
c.message = line
561+
else:
562+
c.message = f"{c.message}; {line}"
563+
526564

527565
def _without_none(v: typing.Any) -> typing.Any:
528566
"""Return v with every None value left out, recursing into dicts and lists."""
@@ -573,15 +611,15 @@ def composing(
573611
The dependencies are recorded when the block exits, so an exception
574612
inside it records nothing.
575613
576-
A value that isn't available yet comes back as None, and the field is
577-
left out: when the block exits, None fields are removed from the resource
614+
A value that isn't available yet comes back as None, and the field is left
615+
out: when the block exits, None fields are removed from the resource
578616
however it was written, because sending one would clear the field. If the
579617
resource doesn't exist yet that's what ordering is for: Crossplane waits
580-
for the dependency before creating it. If it does exist,
581-
it keeps its current spec for the fields that were left out, and the
582-
response carries a warning saying why. That holds even if the block
583-
doesn't compose the resource at all because the value it needed is
584-
missing: an existing resource is kept rather than deleted.
618+
for the dependency before creating it. If it does exist, it keeps its
619+
current spec for the fields that were left out, and the XR's
620+
DependencyValuesAvailable condition turns False, saying why. That holds
621+
even if the block doesn't compose the resource at all because the value it
622+
needed is missing: an existing resource is kept rather than deleted.
585623
586624
Dependencies are declared only for a resource that's composed or already
587625
exists. A block that composes nothing declares nothing.

‎tests/test_dependency.py‎

Lines changed: 74 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,18 @@ def edges(rsp: fnv1.RunFunctionResponse) -> list[dict]:
110110
]
111111

112112

113+
def conditions(rsp: fnv1.RunFunctionResponse) -> list[dict]:
114+
return [
115+
json_format.MessageToDict(c, preserving_proto_field_name=True)
116+
for c in rsp.conditions
117+
]
118+
119+
120+
def kept(name: str) -> str:
121+
"""The condition message for a resource kept while the VPC's id is missing."""
122+
return f"{name} kept its current spec: vpc.status.atProvider.id isn't available"
123+
124+
113125
def body(rsp: fnv1.RunFunctionResponse, name: str) -> dict:
114126
return resource.struct_to_dict(rsp.desired.resources[name].resource)
115127

@@ -331,9 +343,67 @@ def test_existing_resource_keeps_its_spec(self) -> None:
331343
body(rsp, "subnet")["spec"]["forProvider"],
332344
{"region": "us-west-2", "vpcId": "vpc-0123"},
333345
)
334-
self.assertEqual(len(rsp.results), 1)
335-
self.assertEqual(rsp.results[0].severity, fnv1.SEVERITY_WARNING)
336-
self.assertIn("vpc.status.atProvider.id", rsp.results[0].message)
346+
self.assertEqual(rsp.results, [])
347+
self.assertEqual(
348+
conditions(rsp),
349+
[
350+
{
351+
"type": "DependencyValuesAvailable",
352+
"status": "STATUS_CONDITION_FALSE",
353+
"reason": "KeptCurrentSpec",
354+
"message": kept("subnet"),
355+
}
356+
],
357+
)
358+
359+
def test_condition_is_true_when_nothing_is_kept(self) -> None:
360+
# Returned even so: Crossplane keeps a condition a function set
361+
# earlier, so one only returned while False would never clear.
362+
req = fnv1.RunFunctionRequest(meta=ORDERED)
363+
rsp = response.to(req)
364+
with dependency.composing(req, rsp, "subnet") as c:
365+
c.ref(dependency.named("vpc", VPC).status.atProvider.id)
366+
367+
self.assertEqual(
368+
conditions(rsp),
369+
[
370+
{
371+
"type": "DependencyValuesAvailable",
372+
"status": "STATUS_CONDITION_TRUE",
373+
"reason": "Available",
374+
}
375+
],
376+
)
377+
378+
def test_condition_names_every_kept_resource(self) -> None:
379+
existing = {
380+
"spec": {"forProvider": {"region": "us-east-1", "vpcId": "vpc-0123"}}
381+
}
382+
req = fnv1.RunFunctionRequest(
383+
meta=ORDERED, observed=observed(a=existing, b=existing, c=VPC_OBSERVED)
384+
)
385+
rsp = response.to(req)
386+
vpc = dependency.named("vpc", VPC)
387+
388+
# A kept resource, one that's fine, then another kept one: the one
389+
# that's fine mustn't turn the condition back to True.
390+
for name in ("a", "c", "b"):
391+
with dependency.composing(req, rsp, name) as c:
392+
if name == "c":
393+
continue
394+
c.ref(vpc.status.atProvider.id)
395+
396+
self.assertEqual(
397+
conditions(rsp),
398+
[
399+
{
400+
"type": "DependencyValuesAvailable",
401+
"status": "STATUS_CONDITION_FALSE",
402+
"reason": "KeptCurrentSpec",
403+
"message": f"{kept('a')}; {kept('b')}",
404+
}
405+
],
406+
)
337407

338408
def test_composing_nothing_declares_nothing(self) -> None:
339409
req = fnv1.RunFunctionRequest(meta=ORDERED)
@@ -379,7 +449,7 @@ def test_existing_resource_not_composed_is_kept(self) -> None:
379449
self.assertEqual(
380450
edges(rsp), [{"resource": "subnet", "composed_resource": "vpc"}]
381451
)
382-
self.assertEqual(rsp.results[0].severity, fnv1.SEVERITY_WARNING)
452+
self.assertEqual(conditions(rsp)[0]["reason"], "KeptCurrentSpec")
383453

384454
def test_without_capability_holds_back_by_omission(self) -> None:
385455
req = fnv1.RunFunctionRequest(meta=UNORDERED)

0 commit comments

Comments
 (0)