Skip to content
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

Docs: deploying Infinispan with Operator #569

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

pruivo
Copy link
Contributor

@pruivo pruivo commented Oct 12, 2023

  • Write single cluster deployment
  • Write cross-site deployment

@pruivo pruivo marked this pull request as ready for review October 13, 2023 15:07
@ahus1 ahus1 self-requested a review October 13, 2023 15:32
Copy link
Contributor

@ahus1 ahus1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @pruivo - thank you for these two great step-by-step guides. I moved the includes to a separate folder, this way they are not converted to separated pages by Antora.

I did some re-rewording in some places to have shorter sentences and to use the active voice where it was previously the passive voice. Please review my changes to see if I slipped in some mistakes doing that.

Two questions remand open for me:

  • I didn't see the Gossip Router mentioned in here, and that we deploy it only in one datacenter. Deploying only one is AFAIK still the recommendation, so I would expect this to be mentioned in the docs.
  • I found that the work cache is in Keycloak a replicated cache, while it seems to be a distributed cache in the external Infinispan. Could you please comment on this / discuss with this the team? We can also handle this in a separate issue if it requires additional investigation / alignment.

@ahus1
Copy link
Contributor

ahus1 commented Oct 16, 2023

@pruivo - one more thing: Please document on how to update the memory settings (requested + limit on Kuberentes level, JVM parameters for heap). A first iteration would IMHO be: Use the calculation we have to calculate the memory for the 3-node Keycloak deployment, and then use the same setting for Infinispan, assuming that the RAM usage for sessions would be similar.


== Audience

This guide describes the procedures required to deploy {ispn} in a multiple cluster environment (cross-site).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/multiple cluster/multi-cluster?

This guide describes the procedures required to deploy {ispn} in a multiple cluster environment (cross-site).
For simplicity, this guide uses the minimum configuration possible that allows {kc} to be used with an external {ispn}.

This guide assumes two {ocp} clusters named `{site-a}` and `{site-b}`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing full stop.

include::partial$running/infinispan-install-operator.adoc[]
include::partial$running/infinispan-credentials.adoc[]
+
Those commands are required both {ocp} clusters.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of suggestions:

"These commands are required by both {ocp} clusters."
OR
"These commands must be executed on both {ocp} clusters."

+
Those commands are required both {ocp} clusters.
Although we use the same credentials in both {ispn} clusters, they could have been different.
Just adapt your {kc} configuration.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although true, I feel like this is deviating from the minimal required steps and the docs would be more focused if omitted.


. Create a service account.
+
A service account is required to establish the cross-site connection.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"...connection between clusters."

The {ispn} {operator-docs}#creating-caches[Cache CR] allows to deploy the caches in the {ispn} cluster.
Cross-site needs to be enabled per cache as documented by {xsite-docs}[Cross Site Documentation].
The documentation contains more details about the options used by this guide.
In the following example shows the Cache CR for `{site-a}`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The following..."

+
The https://infinispan.org/docs/infinispan-operator/main/operator.html#creating-clusters[Creating Infinispan clusters] documentation provides all the information on how to create and configure your Infinispan cluster.
+
For simplicity, we provide the following example as a starting point.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence doesn't really add anything, I think we can remove it.

@@ -15,12 +15,12 @@ See xref:running/index.adoc[] for additional guides.

* OpenShift or Kubernetes cluster running.
* Existing xref:running/keycloak-deployment.adoc[Basic Keycloak deployment] as it will be extended.
* Existing deployment of Infinispan with the Operator (TODO).
* Existing Infinispan deployment, for example, one of xref:running/infinispan-deployment.adoc[] or xref:running/keycloak-with-external-infinispan.adoc[].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should "infinispan" be "{ispn}"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ispn attribute does not exist in this file.

@@ -0,0 +1,37 @@
. Configure the credential to access the Infinispan cluster.
+
Keycloak needs this credential to be able to access the cluster.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Keycloak needs this credential to be able to authenticate with the {ispn} cluster."?

. Configure the credential to access the Infinispan cluster.
+
Keycloak needs this credential to be able to access the cluster.
By default, the Infinispan Operator generates credentials with a random password and stores it in a secret.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should just document how to define a pre-configured password and then provide a link to the Infinispan Operator docs as further reading at the end of this section. I don't think a Keycloak user cares about Infinispan's defaults etc, just what they need to do to get started and using pre-configured credentials is the easiest way.

@pruivo
Copy link
Contributor Author

pruivo commented Oct 16, 2023

@ahus1 thanks for that! Looks good, just squashed the commits.

I didn't mention the Gossip Router because I'm trying to keep the CRs as minimal as possible and, it is most likely related to how the Openshift is configured. It has been the default to have 1 Gossip Router per site and I haven't heard any reports from customers. In the worst case, we can write some knowledge article or some tunning guide at the end.

The Infinispan memory configuration is documented in our guides, which are linked. If you have some idea of how much memory is required per session, we can try to give some guidance.

The work cache is replicated in Keycloak because it needs to trigger events when it changes. When there is an external Infinispan deployment, the events originate from the external Infinispan. It does not matter (except to prevent data loss) if the cache is distributed or replicated, externally.

Copy link
Contributor

@ahus1 ahus1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks you for the updates. I was concerned with the Gossip Router as I think we did the failure testing with only one Gossip Router and not two. Would it make sense to repeat the failure testing with two Gossip Routers? If yes, please create a follow-up issue.

@ahus1 ahus1 merged commit 931e2dc into keycloak:main Oct 16, 2023
2 checks passed
@pruivo pruivo deleted the t_ispn_operator branch October 16, 2023 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants