-
Notifications
You must be signed in to change notification settings - Fork 772
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
To add reconciliation time stamps #572
base: main
Are you sure you want to change the base?
Conversation
… in the string formt and used them to store the history
@Abiji-2020 check the CI. Its failing on unused import |
UpdatesThere is a time package which was planned to use initally and then i have'nt used it so now i had removed it. |
How can i check for this checking as again tests in CI is failing? |
…e_controller file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Abiji-2020 thanks for the PR and the description of your changes! Left two comments, but could you also format the code? I see a lot of changes coming from different indentation
The code was updated and the formatting was cleared |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Abiji-2020 thanks, looks good! 🧡
Will merge after #582
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, 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 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 :) |
📑 Description
Updated the
setStatus
function in theModuleReconciler
to include theFinishedAt
timestamp in the reconciliation status. TheFinishedAt
field now uses the current time, formatted as a string in RFC3339 format. Updated the custom JSON marshaler and unmarshaler forReconciliationStatus
to handle this timestamp field correctly. Fixes #430FinishedAt
field of typestring
toReconciliationStatus
.setStatus
function to setFinishedAt
to the current time in RFC3339 format.FinishedAt
betweentime.Time
andstring
.✅ Checks
setStatus
function updates theFinishedAt
timestamp correctly.FinishedAt
field as expected.ℹ Additional context
ReconciliationStatus
struct to include aFinishedAt
field.setStatus
function in theModuleReconciler
to set theFinishedAt
field to the current time.FinishedAt
timestamp.