Skip to content

patch.Helper creates race conditions in downstream controllers #934

New issue
New issue
Open
Open
patch.Helper creates race conditions in downstream controllers#934

Description

@EronWright
EronWright
opened on May 27, 2025

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.

Activity

  1. stefanprodan commented on May 28, 2025

    @stefanprodan
    stefanprodan
    on May 28, 2025
    Member

    @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

  2. EronWright commented on May 28, 2025

    @EronWright
    EronWright
    on May 28, 2025
    Author

    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
  3. IvanHunters
    added 16 commits that reference this issue on Jul 2, 2026

    docs(operator): link workaround to fluxcd/pkg#934 (existing upstream …

    eeca7f1

    refactor(operator): drive source-watcher via bounded requeue instead …

    6924d74

    fix(operator): force source-watcher drift via status patch, not reque…

    7b49d91

    fix(operator): schedule follow-up reconcile when Ready is Unknown wit…

    97b054b

    docs(operator): link workaround to fluxcd/pkg#934 (existing upstream …

    0eb88ec

    refactor(operator): drive source-watcher via bounded requeue instead …

    53f7db9

    fix(operator): force source-watcher drift via status patch, not reque…

    e187f44

    fix(operator): schedule follow-up reconcile when Ready is Unknown wit…

    b10b985

    docs(operator): link workaround to fluxcd/pkg#934 (existing upstream …

    5231901

    refactor(operator): drive source-watcher via bounded requeue instead …

    edfbd8d

    fix(operator): force source-watcher drift via status patch, not reque…

    b043620

    fix(operator): schedule follow-up reconcile when Ready is Unknown wit…

    6e9be49

    docs(operator): link workaround to fluxcd/pkg#934 (existing upstream …

    1225840

    refactor(operator): drive source-watcher via bounded requeue instead …

    ff31479

    fix(operator): force source-watcher drift via status patch, not reque…

    7c3af85

    fix(operator): schedule follow-up reconcile when Ready is Unknown wit…

    f64081b
  4. myasnikovdaniil
    added a commit that references this issue on Jul 6, 2026

    fix(operator): force source-watcher drift on stuck ArtifactGenerator …

    6b6fad0
  5. IvanHunters
    added a commit that references this issue on Oct 5, 2026

    fix(operator): rebuild each changed ArtifactGenerator generation once (…

    b0e07ca
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions