-
Notifications
You must be signed in to change notification settings - Fork 21
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
Reconcile MultiClusterService #476
Reconcile MultiClusterService #476
Conversation
c3cfc6b
to
0739289
Compare
472d821
to
f2fb0d4
Compare
f2fb0d4
to
bebe529
Compare
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.
LGTM. Regarding the integration tests, it feels like there is room to implement more checks (e.g. the actual statuses of the requested object or dependant objects) but I'd left that idea to think to the author and other reviewers
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.
looks good!
bebe529
to
61de7ea
Compare
Further tests to check status can be implemented as part of #460. |
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.
LGTM
307357a
to
38e489c
Compare
38e489c
to
72c2932
Compare
Description
Sanity Testing
The following demo shows some sanity testing:
multiclusterservice-720p.mov