-
Notifications
You must be signed in to change notification settings - Fork 199
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
feat: Documentation for starting standalone schema manager #5101
Conversation
👋 🤖 🤔 Hello, @mesellings! Did you make your changes in all the right places? These files were changed only in versioned_docs/version-8.6/. You might want to duplicate these changes in docs/.
You may have done this intentionally, but we wanted to point it out in case you didn't. You can read more about the versioning within our docs in our documentation guidelines. |
versioned_docs/version-8.6/self-managed/concepts/elasticsearch-privileges.md
Outdated
Show resolved
Hide resolved
versioned_docs/version-8.6/self-managed/concepts/elasticsearch-without-cluster-privileges.md
Outdated
Show resolved
Hide resolved
versioned_docs/version-8.6/self-managed/concepts/elasticsearch-without-cluster-privileges.md
Outdated
Show resolved
Hide resolved
versioned_docs/version-8.6/self-managed/concepts/elasticsearch-without-cluster-privileges.md
Outdated
Show resolved
Hide resolved
versioned_docs/version-8.6/self-managed/concepts/elasticsearch-without-cluster-privileges.md
Outdated
Show resolved
Hide resolved
4f97750
to
8f35dc0
Compare
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
username: camunda-app | ||
password: camunda123 | ||
archiver: | ||
ilmEnabled: false |
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.
Please note that I removed these 3 params that were present in our test-config so that standard values are used:
elsRolloverDateFormat: "yyyy-MM-dd-HH-mm"
rolloverInterval: 1m
waitPeriodBeforeArchiving: 1m
username: camunda-app | ||
password: camunda123 | ||
archiver: | ||
ilmEnabled: false |
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.
Please note that I removed these 3 params that were present in our test-config so that standard values are used:
elsRolloverDateFormat: "yyyy-MM-dd-HH-mm"
rolloverInterval: 1m
waitPeriodBeforeArchiving: 1m
@grlimacan Here are the Helm chart values to achieve the same result (disable Schema Manager in the Helm deployment). There are 2 cases, depending on how the user manages the customers:
Case 1: Auto-generated app config by Helm chart.Using Spring Boot convention using env vars to override config. Here, we rely on the Helm auto-generated config and only provide the config needed to disable Schema Manager in the apps.
Case 2: - Manually-managed app config by the user.Only if the user manages the apps config directly and don't rely on the Helm chart auto-generated config.
|
Great stuff! Thanks!! :-) |
d8983be
to
4c8b223
Compare
Hi @camunda/tech-writers ! This is an urgent PR for a change that only applies to 8.6 (to be more exact the to-be-released 8.6.10 onwards). This needs to be merged by tomorrow. Can you please make sure it gets reviewed today in the evening so that I can merge it tomorrow (European) morning? |
@grlimacan I'll review this for you, and I can then merge and release for you in the morning as well seeing as I'm in the same timezone 👍 I will make changes directly to the branch considering the tight timescale if that's okay? |
Thanks @mesellings! Yes please do make the changes straight away :-) I am just going to apply some minor changes quickly now, so just make sure to re-sync the branch. Please do not merge straight away since a technical review from Engineering is still pending |
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.
I've left a couple of tiny comments in the Helm section.
versioned_docs/version-8.6/self-managed/concepts/elasticsearch-without-cluster-privileges.md
Outdated
Show resolved
Hide resolved
versioned_docs/version-8.6/self-managed/concepts/elasticsearch-without-cluster-privileges.md
Outdated
Show resolved
Hide resolved
versioned_docs/version-8.6/self-managed/concepts/elasticsearch-without-cluster-privileges.md
Outdated
Show resolved
Hide resolved
versioned_docs/version-8.6/self-managed/concepts/elasticsearch-without-cluster-privileges.md
Outdated
Show resolved
Hide resolved
versioned_docs/version-8.6/self-managed/concepts/elasticsearch-without-cluster-privileges.md
Outdated
Show resolved
Hide resolved
versioned_docs/version-8.6/self-managed/concepts/elasticsearch-without-cluster-privileges.md
Show resolved
Hide resolved
@mesellings I'm done with my changes for today at least, please make your changes directly on the PR now. Thanks :-) |
@grlimacan I've completed my review for you in the comments, I haven't made them directly. None of them are blocking if you wanted us to apply them in a retrospective PR instead so we don't hold up the release tomorrow 👍 |
Thanks @mesellings ! Could it be that you forgot to submit the comments? I don't see anything in the PR? |
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.
Great work @grlimacan - approving so we don't block tomorrow, but I've added some readability improvements as suggestions 👍
versioned_docs/version-8.6/self-managed/concepts/elasticsearch-privileges.md
Outdated
Show resolved
Hide resolved
versioned_docs/version-8.6/self-managed/concepts/elasticsearch-without-cluster-privileges.md
Outdated
Show resolved
Hide resolved
versioned_docs/version-8.6/self-managed/concepts/elasticsearch-without-cluster-privileges.md
Outdated
Show resolved
Hide resolved
versioned_docs/version-8.6/self-managed/concepts/elasticsearch-without-cluster-privileges.md
Outdated
Show resolved
Hide resolved
versioned_docs/version-8.6/self-managed/concepts/elasticsearch-without-cluster-privileges.md
Outdated
Show resolved
Hide resolved
versioned_docs/version-8.6/self-managed/concepts/elasticsearch-without-cluster-privileges.md
Outdated
Show resolved
Hide resolved
versioned_docs/version-8.6/self-managed/concepts/elasticsearch-without-cluster-privileges.md
Outdated
Show resolved
Hide resolved
versioned_docs/version-8.6/self-managed/concepts/elasticsearch-without-cluster-privileges.md
Outdated
Show resolved
Hide resolved
versioned_docs/version-8.6/self-managed/concepts/elasticsearch-without-cluster-privileges.md
Outdated
Show resolved
Hide resolved
versioned_docs/version-8.6/self-managed/concepts/elasticsearch-without-cluster-privileges.md
Outdated
Show resolved
Hide resolved
FYI @camunda/tech-writers I'm liaising/reviewing this with @grlimacan and plan is for us to get this merged and released by lunchtime (Europe) tomorrow - it might benefit from a @conceptualshark review at some point as an SME, but not urgently 👍 |
authentication: | ||
username: camunda-admin | ||
password: camunda123 | ||
camunda: |
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 configuration requires tasklist and operate to know the elasticsearchs url in the form of this example:
camunda.operate.elasticsearch.url: "http://camunda-elasticsearch:9200"
and
camunda.tasklist.elasticsearch.url: "http://camunda-elasticsearch:9200"
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.
I left this undefined on purpose, because I am only documenting the necessary changes additionally to the settings the customer already has. When left unset, in our example, all tools will use the default http://localhost:9200
, however if customers have a running setup where they use different URLs, if I add this field here, this would possibly confuse them
Co-authored-by: Mark Sellings <[email protected]>
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.
Approved for release - great job 🚀 (I might add some non-blocking edits in a follow-up PR).
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.
🚀 Some comments, but nothing major :) Please address them (accept or reject them) via comments before merging
title: "Elasticsearch without cluster privileges" | ||
--- | ||
|
||
In case the Camunda single application cannot access Elasticsearch with cluster privileges, it is possible to run the schema manager as a stand-alone application separate from the main application. The schema manager solely creates all necessary Elasticsearch Indices. In this case, elevated privileges are only required for the schema creation, the single application does not need to have cluster privileges to work any more. |
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.
🔧 I would omit the sentence The schema manager solely creates all necessary Elasticsearch Indices
. I think it just leads to confusion (since it actually does more than this anyway), and gives unnecessary details. We can leave it at that the application creates the necessary schema for C8 to run afterwards without cluster privileges.
🔧 Similarly, we use elevated privileges
and cluster privileges
interchangeably, and I don;'t know if that's true? I would pick one, likely cluster level privileges
, and just stick to it.
versioned_docs/version-8.6/self-managed/concepts/elasticsearch-without-cluster-privileges.md
Show resolved
Hide resolved
|
||
## Setup | ||
|
||
For this setup to work, the database schema must be initialized first (step 1). This requires cluster level privileges for the database. This only needs to be executed once. |
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 only needs to be executed once.
Wouldn't this be, this needs to be executed once on the first installation, and then every time they perform an update? Or are we not discussing updates?
Because they will eventually update, and they will need to run it again at that point.
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.
Following this discussion it is better to leave as is to avoid confusion. We will update this text before 8.6.11
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.
Do we have a way to ensure it will be done before the next update? Note that the next patch is coming out this Tuesday 😄
I mean, I don't expect we have schema changes until then, so it's fine. But you get the idea ;)
### 1. Initialize the Schema Manager | ||
|
||
The schema manager is started as a separate standalone Java application and is responsible for creating and managing the database schema and applying database settings, such as retention policies for example. | ||
This step requires cluster level privileges for the database and only needs to be executed once per installation. |
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.
❓ Do we need to specify the required permissions? Or is there an ES equivalent, i.e. saying superuser
or something (is that a thing for ES?).
|
||
#### 1.1 Configure Schema Manager | ||
|
||
Create an additional custom configuration for the schema manager with the following values: |
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.
🔧 The configuration below implies they want ILM. Maybe we can add a note that the configuration can be tweaked based on existing configuration (link to the respective components configuration), but that config here would be a minimal sample configuration.
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.
I've added a note on the config file about ILM. About the existing configuration, that is what is implied with additional custom configuration
Oops, it was merged already 😄 Oh well, we can always iterate |
The preview environment relating to the commit d2b7730 has successfully been deployed. You can access it at https://preview.docs.camunda.cloud/pr-5101/index.html |
Description
Adding documentation for stand alone schema manager
Parent epic: camunda/camunda#27883
When should this change go live?
bug
orsupport
label)available & undocumented
label)hold
label)low prio
label)PR Checklist
/docs
directory (version 8.8)./versioned_docs/version-8.7/
directory (version 8.7)./versioned_docs
directory.@camunda/tech-writers
unless working with an embedded writer.