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.

Sign up for GitHub

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

Jump to bottom

Add LastSyncedTime field on CRDs #448

Merged
merged 1 commit into from
Mar 4, 2021
Merged

Add LastSyncedTime field on CRDs #448

merged 1 commit into from
Mar 4, 2021

Conversation

thisisnotashwin
Copy link
Contributor

@thisisnotashwin thisisnotashwin commented Mar 4, 2021 •
edited
Loading

Changes proposed in this PR:

  • Add a field "LastSyncedTime" to the Status of the Custom Resources that described the timestamp for when the CRD was last successfully synced with Consul
  • Add a printer column on the CRDs to depict the last time the resource was synced successfully.

How I've tested this PR:

  • Added unit tests to validate desired behavior.
  • Validated on consul-helm manually!!

How I expect reviewers to test this PR:

  • Code review
    – Install consul-helm with the following consul-k8s image: ashwinvenkatesh/consul-k8s@sha256:8f2d59278f4b8deca8eaeb643a3e6143684e4c37a3e03c051781a1c42a8d70d4

Install a few Custom Resources and watch the Last Synced time update when kubectl get resource is performed. Perform a kubectl edit on the resource and this should lead the to last synced time getting updated.

Checklist:

  • Tests added
  • CHANGELOG entry added (HashiCorp engineers only, community PRs should not add a changelog entry)

@thisisnotashwin thisisnotashwin requested review from lkysow, a team and kschoche and removed request for a team March 4, 2021 00:09
kschoche
kschoche approved these changes Mar 4, 2021
View reviewed changes
Copy link
Contributor

@kschoche kschoche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧨
I also love the cleanup here, great work!

@thisisnotashwin thisisnotashwin mentioned this pull request Mar 4, 2021
Merged
2 tasks
lkysow
lkysow approved these changes Mar 4, 2021
View reviewed changes
Copy link
Member

@lkysow lkysow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

@thisisnotashwin thisisnotashwin merged commit 125f7e3 into master Mar 4, 2021
@thisisnotashwin thisisnotashwin deleted the add-last-synced-time branch March 4, 2021 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lkysow lkysow lkysow approved these changes

@kschoche kschoche kschoche approved these changes

Assignees
No one assigned
Labels
theme/crds type/enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@thisisnotashwin @lkysow @kschoche