Use createOrUpdate pattern for pod environment configmap
#191
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
LimKianAn
force-pushed
the
use-create-or-update-pattern
branch
from
aed02b4 to
fef8968
Compare
LimKianAn
changed the title
createOrUpdate patterncreateOrUpdate pattern for pod environment configmap
eberlep
requested changes
Apr 6, 2021
| if err := r.ensureZalandoDependencies(ctx, instance); err != nil { | ||
| r.recorder.Eventf(instance, "Warning", "Error", "failed to install operator: %v", err) | ||
| return ctrl.Result{}, fmt.Errorf("error while ensuring Zalando dependencies: %w", err) | ||
| if err := r.CreateOrUpdateOperator(ctx, instance); err != nil { |
There was a problem hiding this comment.
I think I don't like the fact that we now always update the operator each time anything changes and that it will not be skipped if the operator is already installed.
Before this change, we once called this code when we initially created the namespace, and once on startup of the postgreslet. Now it will be called from the postgres_controller, meaning every time anything changes.
I would like this to stay as it was: Once on creation, once on startup and each time the backup-config changes.
Porposal
- CreateOrUpdate stays the way it is (no check if it is already installed), CreateOrUpdatePodEnvironmentConfigMap will be exported
- The postgres_controller checks if the operator is already installed and skips CreateOrUpdate if it is already installed (just like before)
- The postgres_controller will no longer update the pod environment configmap
- On startup, CreateOrUpdateOperator will be called (as before)
- A new BackupConfigController will watch the backup-configs and update the PodEnvironmentConfigMaps if neccessary.
There was a problem hiding this comment.
LimKianAn
force-pushed
the
use-create-or-update-pattern
branch
from
f1f755e to
7e6183f
Compare
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.
So that we can later migrate to idiomatic
controllerutil.CreateOrUpdateor evencontrollerutil.CreateOrPatch. Moreover, the logic related to the extended opeartor (zalando operator + pod environment configmap + sidecars configmap) is all shifted toOperatorManager, not mixed inPostgresControlleranymore.