`patch.Helper` creates race conditions in downstream controllers

[EronWright]

When the helper patches the status block, it uses multiple writes to write out .status.conditions and then .status.artifact.

pkg/runtime/patch/patch.go

Lines 225 to 234 in 84030fc

	// Patch the conditions first.
	//
	// Given that we pass in metadata.resourceVersion to perform a 3-way-merge conflict resolution,
	// patching conditions first avoids an extra loop if spec or status patch succeeds first
	// given that causes the resourceVersion to mutate.
	h.patchStatusConditions(ctx, obj, options.ForceOverwriteConditions, options.OwnedConditions, statusOpts),
	// Then proceed to patch the rest of the object.
	h.patch(ctx, obj, clientOpts...),
	h.patchStatus(ctx, obj, statusOpts),

This approach may confuse a downstream controller that is watching for repository updates. Imagine that the controller is watching for the Ready condition to be true and for .status.observedGeneration to match .metadata.generation, before consuming the contents of .status.artifact. In the case of a new commit for the same generation, information contained in artifact might be incorrect, e.g. it might pertain to a previous commit.

For example, here's the result of watching a GitRepository using kubectl get --watch, as a new commit is pushed to the repository (going from main@sha1:00ca386d43834c41d9626b6d93137d396fd771ed to main@sha1:9eb98f3e74333ae8abadf0d73356a1bca9d3b9de). Observe that the object becomes ready before the artifact is updated, making for ambiguity about the artifact details.

apiVersion: source.toolkit.fluxcd.io/v1
kind: GitRepository
metadata:
  annotations:
  creationTimestamp: "2025-05-27T20:42:28Z"
  finalizers:
  - finalizers.fluxcd.io
  generation: 2
  name: pierskarsenbarg-simple-random
  namespace: default
  resourceVersion: "2292286"
  uid: 6dca94f9-deeb-4713-9b2f-55530b76ee12
spec:
  interval: 30s
  ref:
    branch: main
  timeout: 60s
  url: https://github.com/pierskarsenbarg/simple-random.git
status:
  artifact:
    digest: sha256:617e0171fd6c8d0b01c04ca9ef788b6447fab174c48411fb9a1cb857a9b4aae5
    lastUpdateTime: "2025-05-27T20:42:30Z"
    path: gitrepository/default/pierskarsenbarg-simple-random/00ca386d43834c41d9626b6d93137d396fd771ed.tar.gz
    revision: main@sha1:00ca386d43834c41d9626b6d93137d396fd771ed
    size: 585
    url: http://source-controller.flux-system.svc.cluster.local./gitrepository/default/pierskarsenbarg-simple-random/00ca386d43834c41d9626b6d93137d396fd771ed.tar.gz
  conditions:
  - lastTransitionTime: "2025-05-27T21:27:13Z"
    message: 'building artifact: new upstream revision ''main@sha1:9eb98f3e74333ae8abadf0d73356a1bca9d3b9de'''
    observedGeneration: 2
    reason: Progressing
    status: "True"
    type: Reconciling
  - lastTransitionTime: "2025-05-27T21:27:13Z"
    message: 'building artifact: new upstream revision ''main@sha1:9eb98f3e74333ae8abadf0d73356a1bca9d3b9de'''
    observedGeneration: 2
    reason: Progressing
    status: Unknown
    type: Ready
  - lastTransitionTime: "2025-05-27T20:42:30Z"
    message: stored artifact for revision 'main@sha1:00ca386d43834c41d9626b6d93137d396fd771ed'
    observedGeneration: 2
    reason: Succeeded
    status: "True"
    type: ArtifactInStorage
  - lastTransitionTime: "2025-05-27T21:27:13Z"
    message: new upstream revision 'main@sha1:9eb98f3e74333ae8abadf0d73356a1bca9d3b9de'
    observedGeneration: 2
    reason: NewRevision
    status: "True"
    type: ArtifactOutdated
  observedGeneration: 2

---
apiVersion: source.toolkit.fluxcd.io/v1
kind: GitRepository
metadata:
  creationTimestamp: "2025-05-27T20:42:28Z"
  finalizers:
  - finalizers.fluxcd.io
  generation: 2
  name: pierskarsenbarg-simple-random
  namespace: default
  resourceVersion: "2292287"
  uid: 6dca94f9-deeb-4713-9b2f-55530b76ee12
spec:
  interval: 30s
  ref:
    branch: main
  timeout: 60s
  url: https://github.com/pierskarsenbarg/simple-random.git
status:
  artifact:
    digest: sha256:617e0171fd6c8d0b01c04ca9ef788b6447fab174c48411fb9a1cb857a9b4aae5
    lastUpdateTime: "2025-05-27T20:42:30Z"
    path: gitrepository/default/pierskarsenbarg-simple-random/00ca386d43834c41d9626b6d93137d396fd771ed.tar.gz
    revision: main@sha1:00ca386d43834c41d9626b6d93137d396fd771ed
    size: 585
    url: http://source-controller.flux-system.svc.cluster.local./gitrepository/default/pierskarsenbarg-simple-random/00ca386d43834c41d9626b6d93137d396fd771ed.tar.gz
  conditions:
  - lastTransitionTime: "2025-05-27T21:27:13Z"
    message: stored artifact for revision 'main@sha1:9eb98f3e74333ae8abadf0d73356a1bca9d3b9de'
    observedGeneration: 2
    reason: Succeeded
    status: "True"
    type: Ready
  - lastTransitionTime: "2025-05-27T21:27:13Z"
    message: stored artifact for revision 'main@sha1:9eb98f3e74333ae8abadf0d73356a1bca9d3b9de'
    observedGeneration: 2
    reason: Succeeded
    status: "True"
    type: ArtifactInStorage
  observedGeneration: 2

---
apiVersion: source.toolkit.fluxcd.io/v1
kind: GitRepository
metadata:
  creationTimestamp: "2025-05-27T20:42:28Z"
  finalizers:
  - finalizers.fluxcd.io
  generation: 2
  name: pierskarsenbarg-simple-random
  namespace: default
  resourceVersion: "2292288"
  uid: 6dca94f9-deeb-4713-9b2f-55530b76ee12
spec:
  interval: 30s
  ref:
    branch: main
  timeout: 60s
  url: https://github.com/pierskarsenbarg/simple-random.git
status:
  artifact:
    digest: sha256:c84370fd81474ea743d030a86b74b847c714af74d87b038de5fa41d21336e0e6
    lastUpdateTime: "2025-05-27T21:27:13Z"
    path: gitrepository/default/pierskarsenbarg-simple-random/9eb98f3e74333ae8abadf0d73356a1bca9d3b9de.tar.gz
    revision: main@sha1:9eb98f3e74333ae8abadf0d73356a1bca9d3b9de
    size: 585
    url: http://source-controller.flux-system.svc.cluster.local./gitrepository/default/pierskarsenbarg-simple-random/9eb98f3e74333ae8abadf0d73356a1bca9d3b9de.tar.gz
  conditions:
  - lastTransitionTime: "2025-05-27T21:27:13Z"
    message: stored artifact for revision 'main@sha1:9eb98f3e74333ae8abadf0d73356a1bca9d3b9de'
    observedGeneration: 2
    reason: Succeeded
    status: "True"
    type: Ready
  - lastTransitionTime: "2025-05-27T21:27:13Z"
    message: stored artifact for revision 'main@sha1:9eb98f3e74333ae8abadf0d73356a1bca9d3b9de'
    observedGeneration: 2
    reason: Succeeded
    status: "True"
    type: ArtifactInStorage
  observedGeneration: 2

Suggestions for a fix:

1. Update the whole status block atomically.

Workarounds:

1. Perform a substring test for the message field of the ArtifactInStorage condition, does it contain the value of .status.artifact.revision? If not, the object is not actually ready.
2. Watch for changes on .status.artifact and act on them immediately; do not consider the status conditions or the observed generation.

[stefanprodan]

@EronWright this is how the Pulumi controller should watch for source changes:

https://github.com/fluxcd/kustomize-controller/blob/main/internal/controller/source_predicate.go

[EronWright]

Hi @stefanprodan and thanks for the suggestion. The problem isn't so much with the watch logic but with the reconciliation logic. As explained in "Kubernetes API Conventions", controllers are expected to be level-based rather than edge-based. That is, able to make decisions based only on current state. Because of how the patch.Helper writes the status non-atomically, a controller might see and act on the intermediate state where .status.artifact has bad information.

In other words, this revision (2292287) of the resource is incoherent, because the actual condition of the artifact is not reflected in the conditions block. In this case, the artifact is not truly ready and may even be 404.

apiVersion: source.toolkit.fluxcd.io/v1
kind: GitRepository
metadata:
  creationTimestamp: "2025-05-27T20:42:28Z"
  finalizers:
  - finalizers.fluxcd.io
  generation: 2
  name: pierskarsenbarg-simple-random
  namespace: default
  resourceVersion: "2292287"
  uid: 6dca94f9-deeb-4713-9b2f-55530b76ee12
spec:
  interval: 30s
  ref:
    branch: main
  timeout: 60s
  url: https://github.com/pierskarsenbarg/simple-random.git
status:
  artifact:
    digest: sha256:617e0171fd6c8d0b01c04ca9ef788b6447fab174c48411fb9a1cb857a9b4aae5
    lastUpdateTime: "2025-05-27T20:42:30Z"
    path: gitrepository/default/pierskarsenbarg-simple-random/00ca386d43834c41d9626b6d93137d396fd771ed.tar.gz
    revision: main@sha1:00ca386d43834c41d9626b6d93137d396fd771ed
    size: 585
    url: http://source-controller.flux-system.svc.cluster.local./gitrepository/default/pierskarsenbarg-simple-random/00ca386d43834c41d9626b6d93137d396fd771ed.tar.gz
  conditions:
  - lastTransitionTime: "2025-05-27T21:27:13Z"
    message: stored artifact for revision 'main@sha1:9eb98f3e74333ae8abadf0d73356a1bca9d3b9de'
    observedGeneration: 2
    reason: Succeeded
    status: "True"
    type: Ready
  - lastTransitionTime: "2025-05-27T21:27:13Z"
    message: stored artifact for revision 'main@sha1:9eb98f3e74333ae8abadf0d73356a1bca9d3b9de'
    observedGeneration: 2
    reason: Succeeded
    status: "True"
    type: ArtifactInStorage
  observedGeneration: 2