cyclops icon indicating copy to clipboard operation
cyclops copied to clipboard

To add reconciliation time stamps

Open Abiji-2020 opened this issue 1 year ago • 8 comments

📑 Description

Updated the setStatus function in the ModuleReconciler to include the FinishedAt timestamp in the reconciliation status. The FinishedAt field now uses the current time, formatted as a string in RFC3339 format. Updated the custom JSON marshaler and unmarshaler for ReconciliationStatus to handle this timestamp field correctly. Fixes #430

  • Added FinishedAt field of type string to ReconciliationStatus.
  • Updated setStatus function to set FinishedAt to the current time in RFC3339 format.
  • Implemented custom JSON marshaler and unmarshaler for converting FinishedAt between time.Time and string.

✅ Checks

  • [x] I have tested my code:
  • [x] I have performed a self-review of my code:
    • Ensured setStatus function updates the FinishedAt timestamp correctly.
    • Verified that the custom JSON marshaler and unmarshaler handle the FinishedAt field as expected.

ℹ Additional context

  • Related Code Changes:
    • Updated the ReconciliationStatus struct to include a FinishedAt field.
    • Modified the setStatus function in the ModuleReconciler to set the FinishedAt field to the current time.
    • Implemented JSON marshaler and unmarshaler for proper handling of the FinishedAt timestamp.

Abiji-2020 avatar Sep 14 '24 03:09 Abiji-2020

@Abiji-2020 check the CI. Its failing on unused import

petar-cvit avatar Sep 15 '24 15:09 petar-cvit

Updates

There is a time package which was planned to use initally and then i have'nt used it so now i had removed it.

Abiji-2020 avatar Sep 16 '24 04:09 Abiji-2020

How can i check for this checking as again tests in CI is failing?

Abiji-2020 avatar Sep 16 '24 08:09 Abiji-2020

The code was updated and the formatting was cleared

Abiji-2020 avatar Sep 17 '24 14:09 Abiji-2020

Hey @Abiji-2020, I tried running Cyclops from your branch one more time and found that Modules kept reconciling endlessly. The issue is that on each reconciliation, finishedAt is updated with a different timestamp, which then triggers a new reconciliation.

We need to change our reconciliation policy in order to include this PR and not trigger unnecessary reconciliations

petar-cvit avatar Sep 23 '24 13:09 petar-cvit

Hey @Abiji-2020, I tried running Cyclops from your branch one more time and found that Modules kept reconciling endlessly. The issue is that on each reconciliation, finishedAt is updated with a different timestamp, which then triggers a new reconciliation.

We need to change our reconciliation policy in order to include this PR and not trigger unnecessary reconciliations

so What can we do now. Like modifications.?

Abiji-2020 avatar Sep 23 '24 13:09 Abiji-2020

@Abiji-2020 yeah, I will make a PR that will introduce event filter so reconciliation is not triggered on every module update. Will update here when that PR is merged and merge yours as well :)

petar-cvit avatar Sep 23 '24 20:09 petar-cvit

@petar-cvit any updates on this?

Abiji-2020 avatar Nov 25 '24 16:11 Abiji-2020