Add snapshots support #76

rolljee · 2020-10-30T08:47:26Z

What does this PR do?

This PR adds the snapshots module to kourou.

Close #70

How should this be manually tested?

git clone
npm install
./bin/run es:snapshots:<command> <args> --<flags>

Aschen · 2020-10-30T11:33:55Z

features/docker/elasticsearch/elasticsearch.yml

+network.host: 0.0.0.0
+
+# You need to create this folder in order to run snapshots
+path.repo: ["/tmp/snapshots"]


Can't we pass this variable to ES with the environment instead of having to build a whole new Docker image?

I tried, does not work

I think this is due to the fact that the value is an array. Could you try to use YAML objects for environment variables? Like this:

services: kuzzle: # ... elasticsearch: image: yourImage environments: path.repo: - /tmp/snapshots # ...

or using regular YAML array of string:

services: kuzzle: # ... elasticsearch: image: yourImage environments: - 'path.repo=["/tmp/snapshots"]' # ...

Double quote in the second snippet are required for YAML parsing (#76 (comment))

I'll try that, get back to you if it works or not 👍

@alexandrebouthinon both of the options you proposed weren't successful. Only option I see is to keep it that way :x

Ok I thought it was a parsing and type inference problem. That's good for me then 🙂

…pshots

Aschen · 2020-11-02T13:44:41Z

src/commands/es/snapshots/restore.ts

+  async getIndices(esClient: Client) {
+    const beforeResponse = await esClient.cat.indices({ format: 'json' })
+
+    const indices: string[] = beforeResponse.body


Type is automatically inferred here

Suggested change

const indices: string[] = beforeResponse.body

const indices = beforeResponse.body

Aschen · 2020-11-02T13:45:44Z

src/commands/es/snapshots/restore.ts

+
+    const response = await esClient.snapshot.restore(esRequest)
+
+    const indicesAfterRestore = await this.getIndices(esClient)


You should call the method only one time and store the result

You are right, i did that because i wasn't sure indices where put in open state after a restore leading to closed indice. but I checked in the doc

The restore operation must be performed on a functioning cluster. However, an existing index can be only restored if it’s closed and has the same number of shards as the index in the snapshot. The restore operation automatically opens restored indices if they were closed and creates new indices if they didn’t exist in the cluster.

So we can safely remove this call indeed.

Aschen · 2020-11-02T13:46:15Z

src/commands/es/snapshots/restore.ts

+      await esClient.indices.open({ index })
+    }
+
+    this.logOk(`Success ${JSON.stringify(response.body)}`)


I'm not sure about this. Can't we have a better message or at least an indented JSON?

Aschen · 2020-11-02T13:46:36Z

tsconfig.json

    "strict": true,
    "target": "es2017",
-    "esModuleInterop": true
+    "esModuleInterop": true,
+    "resolveJsonModule": true


I think we don't need this line anymore

Aschen · 2020-11-02T13:47:36Z

src/commands/es/snapshots/dump.ts

+
+import { Kommand } from '../../../common'
+
+export default class ESDump extends Kommand {


Nitpicking

Suggested change

export default class ESDump extends Kommand {

export default class EsSnapshotsDump extends Kommand {

Aschen · 2020-11-02T13:48:07Z

src/commands/es/snapshots/create-repository.ts

+
+import { Kommand } from '../../../common'
+
+export default class EsCreateSnapshot extends Kommand {


Nitpicking

Suggested change

export default class EsCreateSnapshot extends Kommand {

export default class EsSnapshotsCreateRepository extends Kommand {

Aschen · 2020-11-02T13:48:44Z

src/commands/es/snapshots/list.ts

+
+import { Kommand } from '../../../common'
+
+export default class ESlistSnapshot extends Kommand {


Nitpicking

Suggested change

export default class ESlistSnapshot extends Kommand {

export default class EsSnapshotsList extends Kommand {

Aschen · 2020-11-02T20:24:10Z

src/commands/es/snapshots/restore.ts

+    }
+
+    const response = await esClient.snapshot.restore(esRequest)
+


You need to open the indices again and you should do that in a finally block to ensure they don't stay closed

alexandrebouthinon · 2020-11-02T20:37:01Z

features/docker/elasticsearch/elasticsearch.yml

+network.host: 0.0.0.0
+
+# You need to create this folder in order to run snapshots
+path.repo: ["/tmp/snapshots"]


I think this is due to the fact that the value is an array. Could you try to use YAML objects for environment variables? Like this:

services: kuzzle: # ... elasticsearch: image: yourImage environments: path.repo: - /tmp/snapshots # ...

or using regular YAML array of string:

services: kuzzle: # ... elasticsearch: image: yourImage environments: - 'path.repo=["/tmp/snapshots"]' # ...

alexandrebouthinon · 2020-11-02T20:37:46Z

features/docker/elasticsearch/elasticsearch.yml

+network.host: 0.0.0.0
+
+# You need to create this folder in order to run snapshots
+path.repo: ["/tmp/snapshots"]


Double quote in the second snippet are required for YAML parsing (#76 (comment))

alexandrebouthinon · 2020-11-02T20:40:14Z

src/commands/es/snapshots/dump.ts

+
+import { Kommand } from '../../../common'
+
+export default class EsSnapshotsDump extends Kommand {


I don't think we should use the term "dump" in this module since they are not dumps but snapshots which is quite different

Suggested change

export default class EsSnapshotsDump extends Kommand {

export default class EsSnapshots extends Kommand {

So the command should be called create to follow ES terminology: https://www.elastic.co/guide/en/elasticsearch/reference/current/snapshot-restore.html

I think this is a little confusing to just call it EsSnapshots

Since in this "snapshots" module we can create 2 types of thing

Repository (the holder of snapshots)

Snapshot (data)

I prefer option EsSnapshotsCreate since we have an EsSnapshotsCreateRepository

@alexandrebouthinon wdyt ?

And also, shall we rename the Kommand -> es:snapshots:create ?

ping @Aschen @alexandrebouthinon

Yes I agree with you for the naming, we might as well stick to the Elasticsearch documentation 👍

Aschen

Last nitpicking I'm sorry 😅
The commands should be under the name snapshot and not snapshots since that's the name of a "module"

# [0.17.0](https://github.com/kuzzleio/kourou/releases/tag/0.17.0) (2020-11-10) #### New features - [ [#76](#76) ] Add snapshots support ([rolljee](https://github.com/rolljee)) #### Enhancement - [ [#78](#78) ] Fix app:scaffold and improve api action hook help ([Aschen](https://github.com/Aschen)) ---

github-actions · 2024-12-17T09:08:55Z

🎉 This PR is included in version 1.0.0-beta.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

github-actions · 2024-12-17T09:54:09Z

🎉 This PR is included in version 1.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Add snapshots support

2ac37b2

rolljee self-assigned this Oct 30, 2020

rolljee added the changelog:new-features label Oct 30, 2020

rolljee added 4 commits October 30, 2020 10:11

Use v3.4 of doco to enable build.target support

e4467f8

mkdir of the dir where will be stored snapshots

63134dd

Add after_failure hook to log the issue

a3de03f

target the proper file while logging doco

c1ea346

Aschen suggested changes Oct 30, 2020

View reviewed changes

rolljee added 10 commits October 30, 2020 13:49

chown to elasticsearch user for the snapshots folder

e98b98e

cannot change ownership, change mod instead

88e09f9

Add "resolveJsonModule": true

e6609b0

add rootDir property to tsconfig.json

bbbb8b7

change rootDit to include package.json

e8acaa2

Use correct rootDir

cc870e1

Merge branch 'develop' of github.com:kuzzleio/kourou into feat/es-sna…

e9c2e4d

…pshots

Fix tsconfig.json

e122cb0

target path for execute method

d428ccf

fix issue with package.json version

1d2631f

rolljee marked this pull request as ready for review November 2, 2020 10:52

rolljee linked an issue Nov 2, 2020 that may be closed by this pull request

Add es:snapshot module #70

Closed

9 tasks

rolljee marked this pull request as draft November 2, 2020 10:55

Typo + docs

8f5d5cf

rolljee marked this pull request as ready for review November 2, 2020 13:28

rolljee requested a review from Aschen November 2, 2020 13:28

Aschen approved these changes Nov 2, 2020

View reviewed changes

Aschen suggested changes Nov 2, 2020

View reviewed changes

rolljee added 2 commits November 2, 2020 17:23

requested changes from @Aschen

b5d99e8

Nitpicking

ac9f0cf

Aschen suggested changes Nov 2, 2020

View reviewed changes

alexandrebouthinon requested changes Nov 2, 2020

View reviewed changes

requested changes

2a0e43d

rolljee requested review from Aschen and alexandrebouthinon November 3, 2020 10:11

alexandrebouthinon approved these changes Nov 3, 2020

View reviewed changes

Aschen approved these changes Nov 3, 2020

View reviewed changes

Rename from snapshots -> snapshot

7cdac50

rolljee merged commit 1beed5c into develop Nov 4, 2020

rolljee deleted the feat/es-snapshots branch November 4, 2020 10:11

Aschen mentioned this pull request Nov 10, 2020

Release 0.17.0 #80

Merged

github-actions bot added the released on @beta label Dec 17, 2024

github-actions bot added the released label Dec 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add snapshots support #76

Add snapshots support #76

rolljee commented Oct 30, 2020 •

edited

Loading

Aschen Oct 30, 2020

rolljee Oct 30, 2020

Aschen Oct 30, 2020

alexandrebouthinon Nov 2, 2020 •

edited

Loading

alexandrebouthinon Nov 2, 2020 •

edited

Loading

rolljee Nov 3, 2020

rolljee Nov 3, 2020

alexandrebouthinon Nov 3, 2020

Aschen Nov 2, 2020

Aschen Nov 2, 2020

rolljee Nov 2, 2020

Aschen Nov 2, 2020

Aschen Nov 2, 2020

Aschen Nov 2, 2020

Aschen Nov 2, 2020

Aschen Nov 2, 2020

Aschen Nov 2, 2020

alexandrebouthinon Nov 2, 2020 •

edited

Loading

alexandrebouthinon Nov 2, 2020 •

edited

Loading

alexandrebouthinon Nov 2, 2020

Aschen Nov 2, 2020

rolljee Nov 3, 2020

rolljee Nov 3, 2020

alexandrebouthinon Nov 3, 2020

Aschen left a comment

github-actions bot commented Dec 17, 2024

github-actions bot commented Dec 17, 2024

	const indices: string[] = beforeResponse.body
	const indices = beforeResponse.body


		const response = await esClient.snapshot.restore(esRequest)

		const indicesAfterRestore = await this.getIndices(esClient)


		import { Kommand } from '../../../common'

		export default class ESDump extends Kommand {

	export default class ESDump extends Kommand {
	export default class EsSnapshotsDump extends Kommand {


		import { Kommand } from '../../../common'

		export default class EsCreateSnapshot extends Kommand {

	export default class EsCreateSnapshot extends Kommand {
	export default class EsSnapshotsCreateRepository extends Kommand {


		import { Kommand } from '../../../common'

		export default class ESlistSnapshot extends Kommand {

	export default class ESlistSnapshot extends Kommand {
	export default class EsSnapshotsList extends Kommand {

	export default class EsSnapshotsDump extends Kommand {
	export default class EsSnapshots extends Kommand {

Add snapshots support #76

Add snapshots support #76

Conversation

rolljee commented Oct 30, 2020 • edited Loading

What does this PR do?

How should this be manually tested?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexandrebouthinon Nov 2, 2020 • edited Loading

Choose a reason for hiding this comment

alexandrebouthinon Nov 2, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexandrebouthinon Nov 2, 2020 • edited Loading

Choose a reason for hiding this comment

alexandrebouthinon Nov 2, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aschen left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 17, 2024

github-actions bot commented Dec 17, 2024

rolljee commented Oct 30, 2020 •

edited

Loading

alexandrebouthinon Nov 2, 2020 •

edited

Loading

alexandrebouthinon Nov 2, 2020 •

edited

Loading

alexandrebouthinon Nov 2, 2020 •

edited

Loading

alexandrebouthinon Nov 2, 2020 •

edited

Loading