Skip to content

Removes the watched secret store by directly using the statefulsets - #21161

Closed
shawkins wants to merge 1 commit into
keycloak:mainfrom
shawkins:iss21125
Closed

Removes the watched secret store by directly using the statefulsets#21161
shawkins wants to merge 1 commit into
keycloak:mainfrom
shawkins:iss21125

Conversation

@shawkins

Copy link
Copy Markdown
Contributor

What's going on here:

  • removes the usage of a direct call to rollingUpdate, instead a rollingUpdate will be triggered by a secret hash on the statefulset template.
  • this first point also means that the upgrade of the operator will trigger a rolling restart
  • removes the need for the watched secret store, under the assumption there's nothing else using that resource. Instead we'll add an annotation to the statefulset to directly identify the secrets being watched.
  • This moves things in the direction of dependent resources / controllers that only modify their resource. Simplistic ways of handling this would keep the programatic workflow of adding labels to the secrets after the statefulset has been modified and/or require a fresh read over all of the statefulsets when determining if the label can be removed. Here instead there's two logical controllers to take responsiblity for watching secrets then notifying the main keycloak controller of changes, and the other takes reponsibility for adding labels to secrets so that they can be watched. This keeps a stricter separation of reponsibilities and allows for the usage of the caches. The logic is a little more straight-forward as co-located on the WatchedSecretsStatefulSetController, which makes it the succesor to WatchedSecretsStore.
  • The major drawback is that there are now two statefulset informers running - one for dependent logic of the KeycloakController, and one as the primary source for the new controller. I believe that we'll be able to remove this duplication once things move further toward dependent resources (we just need to ensure that whatever statefulset cache we're reasoning over is consistent with what needs to be watched).
  • This largely mimics the old handling, but you could try to do more event filtering - differentiate statefulsets that have no watched secrets [*], omit secret events if the data is not changing, etc. That seems best to tackle later.
  • The changes assume the new label prefix of operator.keycloak.org.

closes #21125

[*] I'm wondering if at some point the initial admin secret will need to be watched as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove watched secrets store from the operator

1 participant