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

refactor/envoyconfigrevision-controller #64

Merged
merged 7 commits into from
Dec 16, 2020
Merged

refactor/envoyconfigrevision-controller #64

merged 7 commits into from
Dec 16, 2020

Conversation

roivaz
Copy link
Member

@roivaz roivaz commented Dec 14, 2020 •
edited
Loading

This PR refactors the EnvoyConfigRevision controller:

  • All optional fields in the status of the EnvoyConfigRevision resource are now pointers, as per the k8s api recommendations.
  • The reconciler logic has been refactored to make it much more clear by following operator best practices as per described in the openshift blog. There are now clear ronciliation phases: intialize, finalize, the reconciler itself and the status reconciliation. Tests have been added to improve the coverage of each one of these phases. Specially important are the tests added to the status reconciliation, as now the behavior is fully covered by the tests, which was not before.
  • Code for each reconciling phase now lives in a separate package
  • The semantics of the ResourcesOutOfSync condition have been reverted to ResourcesInSyncCondition. It's just a semantic change with no other implication.

@roivaz roivaz force-pushed the refactor/envoyconfigrevision-controller branch 5 times, most recently from a5a693f to c922fc9 Compare December 14, 2020 13:31
@roivaz roivaz changed the title [WIP] refactor/envoyconfigrevision-controller refactor/envoyconfigrevision-controller Dec 14, 2020
@roivaz roivaz self-assigned this Dec 14, 2020
@roivaz
Refactor envoyconfigrevision to follow controller best practises
b5d5bb0
@roivaz
Improve status reconciliation for EnvoyConfigRevision controller
f642209 
Reconciliation of the status was performed based on a certain sequantiality of the states an EnvoyConfigRevision passed since creation until publication. This approach does not allow to know the exact status of the revision, as it is based on events occurring in a certain order and not in observations of the real status.

This commit changes that so the status clearly reflects if the resources in the revision are correctly synced with the xDS server cache or not. It does so using the only possible way, inspecting the contents of the xDS server cache.
@roivaz
Add missing comment for exported function
52b37f0
@roivaz
Reverse the semantic of ResourcesOutOfSyncCondition to ResourcesInSyn…
8e3ac52 
…cCondition
@roivaz
Fix coverprofile file name in Makefile
9a84c7c
@roivaz
Add tests for EnvoyConfigRevision status
ec146b1
@roivaz
Move all envoyconfigrevision reconcile logic to the reconcilers package
d89942b
@roivaz roivaz force-pushed the refactor/envoyconfigrevision-controller branch from c922fc9 to d89942b Compare December 14, 2020 13:50
@roivaz roivaz merged commit 6408ef1 into master Dec 16, 2020
@roivaz roivaz deleted the refactor/envoyconfigrevision-controller branch December 16, 2020 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slopezz slopezz slopezz approved these changes

@raelga raelga raelga approved these changes

Assignees

@roivaz roivaz

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@roivaz @raelga @slopezz