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

Missing hint on a resource being suspended in the event log #5174

Open
norman-zon opened this issue Feb 5, 2025 · 10 comments
Open

Missing hint on a resource being suspended in the event log #5174

norman-zon opened this issue Feb 5, 2025 · 10 comments

Comments

@norman-zon
Copy link

When a resource is suspended, the event log of the CR gives no hint. This can be confusing.

The event log of an example kustomization:

Events:
  Type    Reason                   Age   From                  Message
  ----    ------                   ----  ----                  -------
  Normal  ReconciliationSucceeded  20m   kustomize-controller  Reconciliation finished in 321.88µs, next run in 30m0s

The corresponding log of the controller:

{"level":"info","ts":"2025-02-05T15:46:56.075Z","msg":"Reconciliation is suspended for this object","controller":"kustomization","controllerGroup":"kustomize.toolkit.fluxcd.io","controllerKind":"Kustomization",
"Kustomization":{"name":"keycloak","namespace":"keycloak"},"namespace":"keycloak","name":"keycloak","reconcileID":"06e55339-b119-406a-a43a-51d216ccce8f"}
{"level":"info","ts":"2025-02-05T15:46:56.075Z","msg":"Reconciliation finished in 321.88µs, next run in 30m0s","controller":"kustomization","controllerGroup":"kustomize.toolkit.fluxcd.io","controllerKind":"Kus
tomization","Kustomization":{"name":"keycloak","namespace":"keycloak"},"namespace":"keycloak","name":"keycloak","reconcileID":"06e55339-b119-406a-a43a-51d216ccce8f","revision":"main@sha1:bf32da749d42a42c363dbdf
ffd609efe1318875a"}

The controller does inform about the kustomization being suspended, but the event log does not.

I would expect the info about the resource being suspended to also be shown in the event log.

I am aware that the info is present in the status of the resource, but when just looking at the event log and seeing Reconciliation finished feels misleading to me.

@matheuscscp
Copy link
Member

Personally I think there's not much value in this, if we add this event and you suspend a Kustomization on one day, in the next day the event won't be there anymore, Kubernetes deletes events after one hour (if you haven't changed this value in the Kubernetes API Server with the CLI flag --event-ttl).

@norman-zon
Copy link
Author

I would rather see the event emitted as additional info to the reconciliation, if the Kustomization is suspended. So when a reconciliation is triggered on a suspended resource I can directly see from the events, that the reconciliation has no effect because of the suspension.

Like so:

Events:
  Type    Reason                                  Age   From                  Message
  ----    ------                                  ----  ----                  -------
  Normal  ReconciliationSucceeded**(suspended)**  20m   kustomize-controller  Reconciliation finished in 321.88µs, **but had no effect due to suspended state** , next run in 30m0s

@matheuscscp
Copy link
Member

matheuscscp commented Feb 6, 2025

but had no effect due to suspended state , next run in 30m0s

There's no such thing, if a Kustomization is suspended the controller does not requeue it after an interval. The reconciliation is only ever reconciled if the spec.suspend changes back to false. Hence why such an event would be emitted only once and then fall into oblivion after one hour.

@scheying
Copy link

scheying commented Feb 6, 2025

@matheuscscp looking at the code it seems that the "Reconciliation finished..." event message is emitted after every requeue interval even if the resource is suspended. This can be quite confusing since it makes it look like an active reconciliation took place when, in reality, it was a no-op.

I think it would make sense to either:

  1. Make it explicit in the event message that it was a no-op reconciliation (as @norman-zon suggested).
  2. Skip the message entirely for suspended resources.

What do you think?

@matheuscscp
Copy link
Member

matheuscscp commented Feb 6, 2025

This is what you are looking for:

https://github.com/fluxcd/kustomize-controller/blob/e4546048c80c35e1574500f9637883ad96f557bc/internal/controller/kustomization_controller.go#L219-L223

Most specifically:

return ctrl.Result{}, nil

Which is the same as:

return ctrl.Result{Requeue: false}, nil

This causes controller-runtime to not requeue that object. So, unless .metadata.generation changes (which only happens when something in the .spec changes e.g. .spec.suspend changing back to false), there will be no more reconciliations for this object. So there's no interval, no loop, no nothing. There won't be any more events after that happens, it'd be just one, that would go away after one hour.

I hope this clears the confusion.

@scheying
Copy link

scheying commented Feb 7, 2025

Thank you very much, @matheuscscp, for the explanation! This clarifies what caused the reconciliation. Nevertheless, it seems that changes to a resource that the Kustomization depends on do trigger “no-op” reconciliations. Given that, I still believe it would be helpful to explicitly mark their event messages as no-ops to avoid misunderstandings. (Actually, we’ve encountered this case in a real-world scenario and only later realized that the resource was suspended, as the message suggested that a reconciliation had occurred.)

@matheuscscp
Copy link
Member

@norman-zon @scheying Are you guys willing to contribute this feature? We would need to change this across all Flux controllers.

Just changing this string to mention that the object is suspended when that's the case would suffice:

https://github.com/fluxcd/kustomize-controller/blob/07a74c85769d76ec6785d8d73be9dded3224dd9d/internal/controller/kustomization_controller.go#L194

@scheying
Copy link

scheying commented Feb 10, 2025

@matheuscscp sure! We'll come up with a PR/PRs.

@matheuscscp
Copy link
Member

@scheying Thanks! We are planning to release source-controller this Thursday, and we will only merge any PRs if we have PRs for all controllers, this is because the Flux 2.5 release is close, we're planning it for end of next week. Otherwise we will ship all PRs in Flux 2.6 next quarter.

@scheying
Copy link

Thanks, @matheuscscp! We tried to prepare the PRs in time, but daily business got in the way. Given that, we'll prepare them with the 2.6 release in mind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants