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

Add reconciliation timestamp to the Module CRD #430

Open
petar-cvit opened this issue Jul 16, 2024 · 9 comments · May be fixed by #572
Open

Add reconciliation timestamp to the Module CRD #430

petar-cvit opened this issue Jul 16, 2024 · 9 comments · May be fixed by #572
Assignees
Labels
controller Update on controller go Pull requests that update Go code good first issue Good for newcomers kubernetes k8s related issues

Comments

@petar-cvit
Copy link
Collaborator

Is your feature request related to a problem? Please describe.
There is currently reconciliation status on the Module CRD, but another useful information to add to the status would be to add the time when the reconciliation finished. Also, the time should be then propagated to the history entry.

Describe the solution you'd like
As a solution, we should add a new field to the reconciliation status and map it when setting the status of the module after reconciliation.

@petar-cvit petar-cvit added good first issue Good for newcomers go Pull requests that update Go code controller Update on controller kubernetes k8s related issues labels Jul 16, 2024
@petar-cvit petar-cvit changed the title Store reconciliation timestamp to the Module CRD Add reconciliation timestamp to the Module CRD Jul 16, 2024
@SpiffyEight77
Copy link
Contributor

Hey @petar-cvit, I'm interested in this issue. Could you please assign it to me? 🙇🏻‍♂️

@petar-cvit
Copy link
Collaborator Author

@SpiffyEight77 do you have any updates on the issue?

@SpiffyEight77 SpiffyEight77 removed their assignment Aug 8, 2024
@SpiffyEight77
Copy link
Contributor

@petar-cvit
I apologize for my delayed response. I've been extremely busy with work and will be until October, so I don't think I'll have enough time to complete this task. I've removed myself from the assignment.

@Abiji-2020
Copy link
Contributor

@petar-cvit I could work on this issue
Things needed to be done :

  • add a new field in the ReconciliationStatus struct for the HistoryEntry
  • Map the values of the status in the setStatus for the updated ReconciliationStatus

@petar-cvit
Copy link
Collaborator Author

@Abiji-2020 yup, that sounds good. Just make sure that after you update the struct in go, you generate the manifests using make generate and make manifests and apply created manifests to your cluster

@petar-cvit
Copy link
Collaborator Author

Hey @Abiji-2020, do you have any updates on the issue? Let me know if you have any questions

@Abiji-2020
Copy link
Contributor

I have a doubt that currently before updating the code I tried to run the Makefile to generate but it is giving me nil pointer error and with no further details. So I am struck in that process

@petar-cvit
Copy link
Collaborator Author

Hey @Abiji-2020, thanks for bringing this up.

There is a mismatch between go and controller-gen versions. I bumped the controller-gen version in #540. Sync your fork and pull the latest main branch.

To install a new version and generate manifests, run the following commands from the cyclops-ctrl folder.

Remove previous controller-gen:

rm -rf bin

Install the new version:

make controller-gen

and then you can generate code (make generate) and manifests (make manifests).

@Abiji-2020
Copy link
Contributor

oh okay, I was a beginner so I was wondering whether I am doing anything wrong here

  • will update you after the correction

@Abiji-2020 Abiji-2020 linked a pull request Sep 14, 2024 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
controller Update on controller go Pull requests that update Go code good first issue Good for newcomers kubernetes k8s related issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants