[CN-1152]: add localConfig to persistence #171
Conversation
kutluhanmetin
requested review from
Serdaro,
rebekah-lawrence and
oliverhowell
as code owners
✅ Deploy Preview for pedantic-goldberg-f76ec1 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Co-authored-by: rebekah-lawrence <[email protected]>
Co-authored-by: rebekah-lawrence <[email protected]>
Co-authored-by: rebekah-lawrence <[email protected]>
|
@rebekah-lawrence I updated the document, may I request a re-review? |
Co-authored-by: rebekah-lawrence <[email protected]>
Co-authored-by: rebekah-lawrence <[email protected]>
Co-authored-by: rebekah-lawrence <[email protected]>
Co-authored-by: rebekah-lawrence <[email protected]>
| The agent configuration is optional. If you give `restore` under `persistence` and do not pass the agent configuration, Hazelcast Platform Operator | ||
| uses the latest agent version that is compatible with its version. | ||
|
|
||
| To restore from local backups that were not created by the HotBackup resource, configure `localConfig`. |
There was a problem hiding this comment.
We should have titles or at least listing approach to emphasize the two different restore scenario when we have local backup. I don't know if it makes sense for you but I suggest something like this.
- Local Backups taken with the Operator.
- Local Backups taken with the Helm Chart
While thinking about their difference, I bring another idea. How about creating new page just for the newly added restore support? Since backup and restore features are expected as one of the main features which will be used always by the all customers who are using operator. However, newly supported restore is not actually a feature which will be used continuously. Instead, it's kinda migration solution for older operator versions and helm chart users. Also in the future we may decide to support fully custom backups. Are we going to put it also in this page? I don't think so, it'll be a migration solution as well so I suggest to separate new restore option from the existing one. Sorry for late feedback, I just got into this idea.
There was a problem hiding this comment.
Such a nice a feedback Cagri, let's talk with the team and decide together. I am not sure %100 but it might be a good idea to make it a part of migration documents.
There was a problem hiding this comment.
A better idea would be to put the documentation about localConfig in here, and reference it from migration documentations.
There was a problem hiding this comment.
Can we provide examples of both methods, and some guidance on when to use Operator and when to use Helm?
All information on restore ought to be in a single place; preferably with headings that clearly indicate what the section covers. For example, in this section it could be Use Restore for Migration; in the migration section it could be Migrate using a Backup.
The required configuration is best with the content it will be used with.
There was a problem hiding this comment.
I committed more changes to improve the docs: 32bb2dd. I didn't mention about the helm and migration words explicitly, because in the feature we will have specific guides for those scenarios like hazelcast/hz-docs#1052. We will refer to this document from them.
| include::ROOT:example$/hazelcast-persistence-restore-local.yaml[] | ||
| ---- | ||
|
|
||
| <1> The name of the cluster for both backup and restore must be the same. If the cluster name is different, the restore fails. |
There was a problem hiding this comment.
Since there is a field called ClusterName in HZ CR. This sentence may confuse some people. I would go with Hazelcase CR Name. WDYT?
There was a problem hiding this comment.
Makes sense: 7d5b290
Co-authored-by: rebekah-lawrence <[email protected]>
Co-authored-by: rebekah-lawrence <[email protected]>
| ---- | ||
| include::ROOT:example$/pod-local-pvc-content.yaml[] | ||
| ---- | ||
| <1> Replace `/data/persistence` with the path to your PV, which is mounted inside the container. |
There was a problem hiding this comment.
| <1> Replace `/data/persistence` with the path to your PV, which is mounted inside the container. | |
| <1> Replace `/data/persistence` with the path to your PV, which is mounted inside the container. |
No description provided.