source-controller icon indicating copy to clipboard operation
source-controller copied to clipboard

Allow deleting suspended objects

Open darkowlzz opened this issue 1 year ago • 3 comments

  • Reorders the object suspended check in all the reconcilers to allow deletion of objects when they are suspended. Objects used to get stuck on delete because the finalizers were not getting removed due to the suspended state.

  • Add setters and getters for spec.suspend and status.artifact in internal/object package. This is needed for writing generic tests for any source kind.

  • Adds a generic test for all the reconcilers to check if a suspended source object can be delete.

Some observations related to the metrics:

When a suspended object is created, finalizer is set and status.observedGeneration remains in -1.

apiVersion: source.toolkit.fluxcd.io/v1beta2
kind: GitRepository
metadata:
  ...
  finalizers:
  - finalizers.fluxcd.io
  generation: 1
  name: podinfo3
  namespace: default
  ...
status:
  observedGeneration: -1

With prometheus query gotk_reconcile_condition{kind=~"GitRepository",name="podinfo3"}==1, following are the observed metrics:

  • status=Unknown when suspended on creation

gotk_reconcile_condition{container="manager", endpoint="http-prom", exported_namespace="default", instance="10.244.0.21:8080", job="monitoring/flux-system", kind="GitRepository", name="podinfo3", namespace="flux-system", pod="source-controller-8bb478cb9-knmg4", status="Unknown", type="Ready"}

  • status=Ready on resuming

gotk_reconcile_condition{container="manager", endpoint="http-prom", exported_namespace="default", instance="10.244.0.21:8080", job="monitoring/flux-system", kind="GitRepository", name="podinfo3", namespace="flux-system", pod="source-controller-8bb478cb9-knmg4", status="True", type="Ready"}

  • status=Ready when the object is suspended again, same as above.

  • status=Deleted when the suspended object is deleted

gotk_reconcile_condition{container="manager", endpoint="http-prom", exported_namespace="default", instance="10.244.0.21:8080", job="monitoring/flux-system", kind="GitRepository", name="podinfo3", namespace="flux-system", pod="source-controller-8bb478cb9-knmg4", status="Deleted", type="Ready"}


Fixes #934

darkowlzz avatar Oct 18 '22 00:10 darkowlzz

Looks like main brach is broken as well:

--- FAIL: TestGitRepositoryReconciler_reconcileSource_authStrategy (29.15s)
    --- FAIL: TestGitRepositoryReconciler_reconcileSource_authStrategy/SSH_with_password_protected_private_key_secret_makes_ArtifactOutdated=True (25.77s)
        --- FAIL: TestGitRepositoryReconciler_reconcileSource_authStrategy/SSH_with_password_protected_private_key_secret_makes_ArtifactOutdated=True/libgit2 (19.21s)
            gitrepository_controller_test.go:543: 
                expected
                	[]v1.Condition{v1.Condition{Type:"FetchFailed", Status:"True", ObservedGeneration:0, LastTransitionTime:time.Date(2022, time.October, 11, 15, 7, 2, 0, time.UTC), Reason:"GitOperationFailed", Message:"failed to checkout and determine revision: recovered from git2go panic: runtime error: invalid memory address or nil pointer dereference"}}
                to match
                	[]v1.Condition{v1.Condition{Type:"ArtifactOutdated", Status:"True", ObservedGeneration:0, LastTransitionTime:time.Date(1, time.January, 1, 0, 0, 0, 0, time.UTC), Reason:"NewRevision", Message:"new upstream revision 'master/9a6150b9f28091197cb5c0e6f52a50f2fa07b6bd'"}, v1.Condition{Type:"Reconciling", Status:"True", ObservedGeneration:0, LastTransitionTime:time.Date(1, time.January, 1, 0, 0, 0, 0, time.UTC), Reason:"NewRevision", Message:"new upstream revision 'master/9a6150b9f28091197cb5c0e6f52a50f2fa07b6bd'"}}

stefanprodan avatar Oct 18 '22 13:10 stefanprodan

Looks like main brach is broken as well

failed to checkout and determine revision: recovered from git2go panic: runtime error: invalid memory address or nil pointer dereference

Seems to be because of git2go panic, which is resulting in a different status condition. Also, only panicking on macOS 11.

darkowlzz avatar Oct 18 '22 13:10 darkowlzz

Ok we can ignore the panic then, it's not something we can fix in this PR.

stefanprodan avatar Oct 18 '22 13:10 stefanprodan