Allow Zync Database to be configured as an external database on the operator #489
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
207bcd3 to
9488701
Compare
|
Looking good so far |
9488701 to
4f96390
Compare
|
Added unit tests and documentation. Ready for a review @eguzki. Pending doing some local testing with internal DBs configured before merging the PR. |
4f96390 to
3c9d2e5
Compare
|
I've tried in a local ocp cluster the following:
And the expected elements seem to be created / not created accordingly to each scenario. QE testing will help us with testing it with real external DBs endpoints available and content. To me I think this is ready @eguzki |
178cdf4 to
19eb741
Compare
eguzki
approved these changes
Oct 9, 2020
19eb741 to
19a571c
Compare
|
Code Climate has analyzed commit 19a571c and detected 4 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR intends to allow Zync Database to be configured externally.
Currently it does so by adding a field
.spec.highAvailability.externalZyncDatabaseEnabled, whose value (trueorfalse) only takes effect whenspec.highAvailability.enabledis set totrue. This implies that with this implementation the only way to set an external zync database is by enabling the other ones to be external too. We can discuss whether we want that to happen or be more granular, taking into account the implications.The reason why behavior of the existing
spec.highAvailability.Enabledhas not been changed to include external zync database is because doing that would break current users deployments (thus why another different new optional parameter is needed)When the zync database is configured externally:
zync-database-postgresqlImageStream is not created anymorezync-databaseDeploymentConfig is not created anymorezync-databaseService is not created anymoreThis is an operator-only functionality.
The possibility of splitting the internal zync database specific components into its own component has been evaluated but to simplify the number of changes needed and diminish the impact on templates side that approach has not been followed.
Let's iterate from here.