Aerogear 7683 provision deprovision keycloak #9
Aerogear 7683 provision deprovision keycloak #9
Conversation
pkg/keycloak/keycloak.go
Outdated
kcCopy.Status.Phase = v1alpha1.PhaseDeprovisioned | ||
|
||
case v1alpha1.PhaseDeprovisioned: | ||
kcCopy.Finalizers = []string{} |
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.
This will remove all finalizers, you should only be removing the one we add in this operator. The finalizeKeycloak
function doe this, but if you want to have it done here and update as normal, you should call sc.RemoveFinalizer(kc, v1alpha1.KeycloakFinalizer)
and get rid of the finalizeKeycloak
function.
To see this working:
This should:
Things worth testing:
N.B. Each test case needs to be a newly created SharedService CRD. |
pkg/keycloak/keycloak.go
Outdated
@@ -47,18 +205,116 @@ func (h *Handler) Handle(ctx context.Context, event sdk.Event) error { | |||
logrus.Info("not doing anything as this resource is already being worked on") |
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.
Can we get rid of this?
pkg/keycloak/keycloak.go
Outdated
if err := h.reconcileRealm(ctx, r, authenticatedClient); err != nil { | ||
return errors.Wrap(err, "failed to reconcile realm "+r.Name) | ||
} | ||
} | ||
|
||
if event.Deleted { |
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.
Is there anything that will go here in the future? Otherwise, can we delete this?
@philbrookes will there be a phase in the future which allows the user to modify the number of slices for a single keycloak? |
pkg/keycloak/keycloak.go
Outdated
} | ||
|
||
// Only update the Keycloak custom resource if there was a change | ||
if !reflect.DeepEqual(kc, kcCopy) { |
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.
made suggested changes, merging |
This is late, but I tested all of the use cases and they are all working as expected |
@JameelB @dimitraz rebased and tested your work after merging @mikenairn PR into master.
I've also rebased the changes Dimitra made to the keycloak-apb on top of the work @witmicko and I did over the last week: aerogearcatalog/keycloak-apb#107
Using these together seems to be working really well!