yeswiki icon indicating copy to clipboard operation
yeswiki copied to clipboard

Feat/archive

Open J9rem opened this issue 2 years ago • 18 comments

Cette PR permet d'introduire une fonctionnalité d'archivage asynchrone du wiki. Spécifications pour cette PR : https://pad.lescommuns.org/YmnJJPjJRMGj7EhAlD1eZg#

Pour tester:

  • faire /update une fois sur la branche feat/archive
  • penser à faire un composer install juste au cas où
  • se rendre sur la page GererMisesAJour :
    • il y a alors un bouton "Gérer les sauvegardes"
    • une sauvegarde est lancée avant chaque mise à jour

Matrice de couverture de la spécification:

Numéro Nom Implémenté Testé Commentaire
[R01] console + command spécifique [x] [x]
[R02] command avec params [x] [x]
[R02.1] params pour choix sql, fichiers ou tout [x] [x]
[R02.2] par défaut tout [x] [x] test par lecture du code
[R02.3] nom des fichiers [x] N.A. voir constantes dans ArchiveService
[R02.4] params extrafiles [x] [x] test manuel
[R02.4.1] params extrafiles chemins relatifs [x] [x] test phpunit
[R02.4.2] params extrafiles OK pour dossiers [x] [x] test manuel
[R02.4.3] params extrafiles OK pour sous-dossiers [x] [x] test manuel
[R02.4.4] params extrafiles compatible glob [x] [x] test manuel
[R02.4.5] params extrafiles save dans wakka.config.php du zip [x] [x] test phpunit
[R02.4.6] params extrafiles configurable via frontend [x] [x] tests manuels
[R02.4.8] params extrafiles save dans wakka.config.php du zip sous forme de tableau [x] [x] test phpunit
[R02.4.9] params extrafiles ne doit tenir compte que des chemins inclus dans le dossier de tête [x] [x] test phpunit
[R02.5] params excludedfiles [x] [x] test hors phpunit
[R02.5.1] params excludedfiles chemins relatifs [x] [x] test phpunit
[R02.5.2] params excludedfiles OK pour dossiers [x] [x] test phpunit
[R02.5.3] params excludedfiles compatible glob [x] [x] test hors phpunit
[R02.5.4] params excludedfiles save dans wakka.config.php du zip [x] [x] test phpunit
[R02.5.5] params excludedfiles configurable via frontend [x] [x] tests manuels
[R02.5.6] params excludedfiles save dans wakka.config.php du zip sous forme de tableau [x] [x] test phpunit
[R03] permettre l'anonymisation de wakka.config.php [x] [x] test phpunit
[R03.1] anonymisation de wakka.config.php réglable par un paramètre [x] [x] test phpunit
[R03.2] anonymisation de wakka.config.php valeurs par défaut [x] [x] test phpunit
[R03.3] anonymisation de wakka.config.php save dans wakka.config.php [x] [x]
[R03.4] anonymisation wakka.config.php avec tableaux aussi [x] [x] test phpunit
[R03.5] anonymisation wakka.config.php avec tableaux : format [x] [x] test phpunit
[R04] un controlleur pour appeler la CLI [x] [x] test phpunit
[R04.1] un controlleur dans includes [x] [x] test par lecture du code; c'est un service
[R04.2] le controlleur fournit les paramètres d'anonymisation [x] [x] test phpunit
[R04.3] les paramètres d'anonymisation ne peuvent être réglés par le frontend [x] N.A. vérifiaction en lisant le code
[R05] la CLI vérifie le dossier n'est pas sur le web [x] [x] tests manuel en suppriment .htaccess
[R05.1] chemin de sauvegarde dans wakka.config.php [x] [x] test manuel
[R05.2] chemin de sauvegarde configurable via {{editconfig}} [x] [x] test manuel
[R06] mode hibernate pendant la sauvegarde [x] [x] proposition de créer un mode archiving
[R07] archivage sur PID différent [x] [x] test phpunit + test manuels
[R07.1] utilisation possible de symfony/process [x] [x]
[R07.2] pouvoir arrêter la CLI [x] [x] test manuel
[R07.3] fournit un état instantané de la CLI [x] [x] test manuel
[R07.4] communication avec la CLI via un fichier [x] [x] test manuel
[R08] zip avec arborescence similaire au yeswiki [x] [x] test phpunit
[R08.1] sauvegarde sql dans zip [x] [x] test phpunit
[R08.4] sauvegarde sql dans zip private/backups [x] [x] test phpunit
[R08.5] private/backups avec .htaccess et README [x] [x] test phpunit
[R08.6] pas d'assurance de la sécurité des données [x] N.A.
[R09] téléchargement du fichier sans lien direct [x] [x] test manuel
[R10] pas de sauvegarde pendant une autre sauvegarde [x] [x] test phpunit
[R11] vérification de l'espace disque avant de lancer une sauvegarde [x] [x] test manuel
[R11.1] règle pour la marge d'espace disque [x] [x] choix taille files + custom + 300 Mo
[R12] limiter le nombre de sauvegardes à 10 [x] [x] test manuel
[R12.1] toujours supprimer la plus ancienne [x] [x] test manuel ! conserve toujours au moins une archive qui a deux jours
[R12.2] demander une confirmation du frontend avant suppression [x] [x] test manuel
[R21] frontend via {{adminbackups}} [x] [x] test manuel
[R21.1] pas d'impact dans {{update}} [x] N.A.
[R22] {{adminbackups}} gestion des sauvegardes précédentes [x] [x] test manuel
[R22.1] {{adminbackups}} actions : liste/télécharger/supprimer [x] [x] test manuel
[R22.2] {{adminbackups}} actions : laisser la possibilité pour "restaurer" [x] N.A. le code frontend contient déja des bouts de codes en commentaires
[R23] {{update}} lancer une sauvegarde automatique avant update [x] [x] tests manuels
[R23.1] {{update}} sauvegarde auto contournable [x] [x] tests manuels
[R23.2] {{update}} sauvegarde auto contourné = confirmation requise [x] [x] tests manuels
[R23.3] {{adminbackups}} permet une sauvegarde manuelle [x] [x] tests manuels
[R23.4] {{adminbackups}} permet la configuration de la sauvegarde [x] [x] tests manuels
[R23.5] {{update}} sauvegarde auto contournable uniquement si param dans wakka.config [x] [x] tests manuels
[R24] {{adminbackups}} VueJS possible [x] [x] usage de VueJs avec syntaxe compatible vuejs 2 et 3
[R25] {{adminbackups}} appel routes API [x] [x]
[R25.1] {{adminbackups}} appel routes API en ajax [x] [x]
[R25.2] {{adminbackups}} suivi CLI en dynamique [x] [x] test manuel
[R25.3] {{adminbackups}} permettre arrêter la CLI [x] [x] test manuel
[R25.4] {{adminbackups}} rafraichissement mini 5 sec [x] [x] test manuel
[R26] route API pour lancer la sauvegarde [x] [x] test manuel
[R26.1] route API pour lancer la sauvegarde sauf si pas assez d'espace [x] [x] test manuel
[R26.2] route API pour lancer la sauvegarde suppression auto de la plus ancienne si > 10 [x] [x] test manuel
[R26.3] route API pour lancer la sauvegarde limite 2 par jours maxi N.A. N.A. non réalisable car c'est la même route api

N.A. : non applicable

J9rem avatar Jul 12 '22 16:07 J9rem

Je viens d'ajouter un refacor de la classe Configuration avec un service associé afin de mutualiser les différents appels des classes concernées (j'ai pas réussi à simplement mettre à jour PackageCore dans le cadre de refactor donc j'ai laissé l'ancienne syntaxe)

J9rem avatar Jul 13 '22 11:07 J9rem

@mrflos je te propose de tester sur l'été cette PR pour l'archivage. Une fois les remarques prises en compte et bugs corrigés, je propose que nous fusionnons cette PR sur doryphore-dev.

Je ferai une PR séparée en septembre pour ce qui concerne la sauvegarde automatique avant {{update}}

Bons tests (j'ai testé sur une configuration. Si ça marche sur ta config., je l'espère, c'est vraiment puissant je trouve)

J9rem avatar Jul 26 '22 22:07 J9rem

J'aurais envie de retirer [R05] | la CLI vérifie le dossier n'est pas sur le web car c'est plus compliqué que ce que je pensais. Je me dis que l'administrateur doit faire attention et que nous pouvons travailler la documentation pour l'accompagner (d'ailleurs, je ferais des commits pour la nouvelle doc en septembre)

J9rem avatar Jul 26 '22 22:07 J9rem

@mrflos j'estime que la présente PR est suffisamment mature pour la soumettre à relecture finale. en effet, je couvre selon mes tests l'ensemble de notre spécification.

Merci pour tes relectures

J9rem avatar Sep 22 '22 09:09 J9rem

YES! top!

mrflos avatar Sep 22 '22 09:09 mrflos

Ma première tentative de test par l'interface web n'a hélas pas marché.

  • je suis passé par la branche feat/archive (sur le dernier commit 22b561364d744cb9ccbf5ecefdfa725bf7be8155 ) et fait un make install pour les dépendances , puis le /update
  • j'ai tenté sans rien configurer > message d'erreur (Lancement de la sauvegarde impossible Car le dossier de sauvegarde n'est pas accessible en écriture. - Vérifier la validité du paramètre 'archive[privatePath]', dans la page 'GererConfig' (rubrique 'Sécutité') - si ce paramètre est vide, le remplir avec un chemin non accessible sur le internet - Vérifier que le dossier est bien accessible pour 'php' (si 'archive[privatePath]' est vide, c'est le dossier '/tmp' qui est utilisé))
  • je rempli la conf en supposant que c'est un chemin relatif (private/backups) mais erreur (=== Checking free space === There is enough free space. => Preparing list of excluded files)
  • le bouton d'arret ne m'a pas permis d’arrêter
  • je mets un chemin complet : ça démarre mais reste bloqué sur le fait qu'une sauvegarde précédente n’était pas terminée avec le message (Lancement de la sauvegarde impossible Car une sauvegarde semble être déjà en cours. Si ça n'est pas le cas, se rendre dans la page 'GererConfig' pour vider la valeur du paramètre wiki_status dans la partie Sécurité)
  • En remettant le bon statut, et relançant le backup, je reviens sur (=== Checking free space === There is enough free space. => Preparing list of excluded files)

Alors peut être vaut il mieux que je commence par tester la cli ?

En tout cas, quelques remarques:

  • a mon avis il faut forcer un chemin par défaut pour le dossier du backup, genre <dossier du wiki>/private/backups présent dans le git, avec un .htaccess pour le protéger sur le conf apache, et un readme pour indiquer la directive nginx pour bloquer l’accès en web
  • j'ai retrouvé des fichiers temporaires (sql vides et json) dans le dossier utilisé pour les backups, je pense qu'il vaudrait mieux n'y trouver que le zip final, toutes les opérations avant devraient être dans un dossier temporaire (soit /tmp soit le dossier cache local, je croies qu'on l'utilise pour les maj déja car /tmp peut être bloqué dans certaines config exotiques)
  • qu'il y ait erreur ou succes, il faut que le statut archiving saute, mais c'est sans doute prévu de fonctionner comme cela, juste je suis sur un bug qui bloque en cours de route.

A ta disposition pour suivre tes instructions de test et/ou debug

Merci!

mrflos avatar Sep 24 '22 17:09 mrflos

@mrflos bon déjà, si tu es arrivé à un endroit où tu me dis que la bouton arrêter ne fonctionne pas, c'est plutôt très bon signe, ça veut dire que beaucoup de choses fonctionnent ou que les boutons sont plutôt intuitifs.

mauvais fonctionnement du bouton "arrêter"

J'ai identifié qu'il est parfois difficile de le faire fonctionner car l'ordre d'arrêt est envoyé en asynchrone dans un fichier texte qui doit être lu par le process d'archivage. Ce dernier ne reçoit pas l'information alors que le fichier est bien à jour, vérification faite par ftp. Je suppose qu'il y ait parfois un cache de système de fichier qui peut proposer des résultats différents celui les PID des process php. Je vais creuser pour voir s'il y a une méthode plus robuste pour interroger les fichiers sans cache mais ça dépend vraiment des configurations. Sur la moitié d'entre elles, tout fonctionne.

a mon avis il faut forcer un chemin par défaut pour le dossier du backup, genre /private/backups présent dans le git, avec un .htaccess pour le protéger sur le conf apache, et un readme pour indiquer la directive nginx pour bloquer l’accès en web

Je suis d'accord avec toi, ce sera plus stable pour le comportement par défaut. Bonne idée. Je dois ajouter un test avec curl pour être sûr que le dossier n'est pas accessible depuis l'extérieur avant de faire une sauvegarde

j'ai retrouvé des fichiers temporaires (sql vides et json) dans le dossier utilisé pour les backups

Ca n'est normalement pas dû au comportement de cette PR. Il y a bien un fichier info.json nécessaire pour assurer la discussion avec les process lancés en arrière-plan. Il ne peut pas être possible d'utiliser le dossier <wiki-dir>/cache car il est accessible depuis internet. Il ne peut pas être possible de considérer que le dossier /tmp car sur certains serveur, son usage n'est pas autorisé et nous retomberions dans le problème que tu as eu. Toutefois, j'insiste, j'ai normalement configuré pour que le fichier .sql ne soit pas généré dans le système de fichiers, donc il ne devrait pas y avoir de tel fichier dans le dossier (où s'il y ait, ça pourrait être dû à autre chose)

qu'il y ait erreur ou succes, il faut que le statut archiving saute, mais c'est sans doute prévu de fonctionner comme cela, juste je suis sur un bug qui bloque en cours de route.

La statut archiving est une sécurité pour éviter que 2 archives soient lancées en même temps. Si la sauvegarde plante en plein milieu, ce qui normalement ne devrait pas arriver sauf exception, effectivement, le statut archiving reste. J'ai volontairement choisis de le laisser pour que le webmaster réfléchisse avant de le retirer et soit sûr qu'il n'y ait pas déjà une autre sauvegarde en cours. En temps normal, le statut archiving est retiré à la fin, qu'il y ait réussite, erreur, ou demande d'arrêt de la part de l'usager. Le cas où celui-ci reste ne survient que dans les erreurs php plus complexes que nous allons identifier, je l'espère, pendant nos phases de tests.

Alors peut être vaut il mieux que je commence par tester la cli ?

Euh, tu pourrais si tu veux, le fonctionnement est similaire. php includes/commands/console core:archive --help Toutefois, je vais faire un commit aujourd'hui ou demain pour améliorer la prise en compte des erreurs relevées et stabiliser le comportement par défaut. Peut-être que ce sera plus facile à tester ensuite.

J9rem avatar Sep 26 '22 06:09 J9rem

@mrflos j'ai fait de nouveaux commits pour:

  • par défaut : utiliser le dossier local private/backups
  • vérifier qu'il n'est pas accessible depuis internet
  • améliorer les messages d'erreur s'il n'y a pas assez de place (je crois que c'est le souci que tu as eu)

J9rem avatar Sep 27 '22 09:09 J9rem

j'ai modifié un peu les tests pour que cela marche sur mon environnement mais je coince encore sur "Preparing list of excluded files". Par contre nickel la création des dossiers

mrflos avatar Sep 27 '22 18:09 mrflos

bien vu pour les fichiers temporaires pour les tests. Juste pour le test des headers, il faudrait aussi retirer les codes en 30x car sur certains serveurs, on peut avoir un cache qui retourne un code 304 (pas besoin recharger) et il ne faudrait pas que ce soit valide. je vais donc modifier légèrement le commit pour être sûr que les codes 30x ne sont pas acceptés.

J9rem avatar Sep 27 '22 18:09 J9rem

"Preparing list of excluded files".

@mrflos est-ce que ça fonctionne avec les paramètres avancés en ne choisissant que la base de données ? ceci devrait mieux fonctionner (car il n'y a pas les fichiers à scanner).

Ensuite, deuxième test

  • excludes *,.*
  • includes wakka.config.php
  • sauvegarde complète

en espérant que ça passe

ensuite, ajouter progressivement le nombre de fichiers à scanner pour identifier quel dossier pose souci

J9rem avatar Sep 27 '22 18:09 J9rem

mais je suis d'accord, il peut y avoir un souci de boucle infinie au moment de la génération de la liste des fichiers à exclure, ... je vais essayer d'identifier le souci

J9rem avatar Sep 27 '22 18:09 J9rem

en tout cas en cli, ca marche nickel, j'ai essayé les 3 options avec succès

mrflos avatar Sep 27 '22 19:09 mrflos

en web : avec juste la DB : === Checking free space === There is enough free space. (puis bloqué) avec la totale et le fichier wakka.config.php et l'exclude ,. > (=== Checking free space === There is enough free space. => Preparing list of excluded files)

mrflos avatar Sep 27 '22 19:09 mrflos

Merci pour ces retours. La recherche de ce qui ne va pas me semble plus complexe que prévu. Je vais te fournir des instructions pour mettre dans certaines conditions et je pourrai alors commencer à trouver le souci.

  1. en allant dans la page GererConfig, rubrique sécurité, mettre false pour l'option archive[call_archive_async]
  2. fais un test de sauvegarde depuis l'interface web
    • si ça ne fonctionne pas, je veux bien que tu m'indiques le message d'erreur, s'il y en a un
  3. en allant dans la page GererConfig, rubrique sécurité, mettre true ou vide pour l'option archive[call_archive_async]
  4. revenir dans la page GererSauvegardes
  5. ouvrir l'inspecteur de ton navigateur, rubrique réseau pour activer le suivi des requêtes
  6. lancer une sauvegarde et me fournir les résultats des requêtes du type http://www.example.com/?api/archives/uidstatus/<uid>
    • précise moi le navigateur utilisé et le type de serveur
    • tu peux m'envoyer les résultats en message privé via Framateam pour éviter de surcharger ici

L'idée que je traque, c'est de voir comment se comporte le retour texte et si une erreur pourrait être dues à l'usage de l'asynchrone sur un système de fichier pouvant avoir un cache

J9rem avatar Sep 28 '22 06:09 J9rem

Je soupçonne aussi que le comportement potentiellement récursif de cette méthode puisse poser problème dans certains cas : https://github.com/YesWiki/yeswiki/blob/d571122627d99fcb0727c957b0fb60798c0d4168/includes/services/ArchiveService.php#L803-L821

Question à creuser aussi (mais normalement, tu n'aurais pas pu faire les tests via CLI)

J9rem avatar Sep 28 '22 06:09 J9rem

est-ce que tu peux faire un composer test sur ta machine pour voir tout se passe bien déjà

J9rem avatar Sep 28 '22 06:09 J9rem

@mrflos avec les derniers commits, j'ai:

  • stabilisé le comportement des liens vers l'api pour les url courtes (pas toujours facile de les faire fonctionner sans le ?)
  • rajouter un export sql de secours dans DbService qui n'est appelé que si mysqldump n'est pas accessible (soit parce que non trouvé, soit parce que proc_open est interdit)
  • activer par défaut à truele paramètre preupdate_backup_activated et je l'ai ajouté dans EditConfig car mes derniers tests m'ont permis de voir que maintenant, il est possible de trouver une solution de fonctionnement dans de nombreuses config serveur planet hoster, infini.fr, windows, yunohost sur raspberry pi à termes, il faudra même retirer la possibilité de changer ce paramètre

Je pense que nous pouvons maintenant plus sereinement insérer la fonction sur doryhore-dev

J9rem avatar Sep 30 '22 10:09 J9rem

bon, il y a des soucis lors de l'exécution de phpunit.yml pour lancer les tests automatiques. J'ai ajouté ce qu'il fallait pour donner accès au dossier de sauvegarde dans docker (/private/archives) et à wakka.config.php, ... maintenant j'ai un souci pour réussir à exporter la base de données, ....

Est-ce que vous savez si mysqldump est facilement accessible dans Docker sans l'exécuter en sudo ? ...

J9rem avatar Dec 13 '22 21:12 J9rem

@J9rem pareil, ya les lint et commits de seballot dans les fichier s à reviewer, pas pratique.
Et je voies une erreur sur les tests, du coup cette feature est elle prete? Besoin d'un coup de main @J9rem ?

mrflos avatar Dec 21 '22 18:12 mrflos

idem @mrflos je n'ai pas compris pourquoi la base de cette PR avait bougée. J'ai remis vers doryphore-dev

J9rem avatar Dec 21 '22 19:12 J9rem

effectivement, @mrflos j'ai besoin d'aide et je vois que le commentaire que je pensais avoir posté n'a pas été distribué.

En effet, il y a un souci pour lancer les tests automatiques. Je ne sais pas pourquoi mysqldump n'est pas accessible pour l'utilisateur runner qui est l'utilisateur du script docker lancé par phpunit.yml. Ceci ne permet pas de réaliser les tests car l'archive ne peut pas réussir. L'autre erreur pourrait être que l'utilisateur runner qui lance la commande composer test ne fait pas partie du groupe www-data et alors il n'a pas tous les droits nécessaires pour écrire dans le dossier lors de l'archivage.

bref:

  • choix 1 : on met en veilleuse les tests pour pouvoir faire la fusion de la PR (j'y suis défavorable)
  • choix 2: quelqu'un m'aide à résoudre ces soucis de droits dans docker pour phpunit.yml (merci d'avance)

Sinon la PR est prête pour les tests (a priori il y a forcément des adaptations d'UX à faire pour les cas qui ne fonctionnent pas du premier coup sans un réglage avancé)

J9rem avatar Dec 21 '22 19:12 J9rem

ok, je ne suis pas spécialiste docker mais je vais demander de l'aide à @BDouze et @mose pour qu'on s'en dépatouille, ca serait en effet bien que les tests marchent plutot que de contourner.

mrflos avatar Dec 21 '22 20:12 mrflos

@J9rem , on a regardé avec @BDouze , et finalement c'était une erreur à la con et un / manquant dans un chemin pour le sql...
Par contre on a remarqué que les fichiers pouvaient etre persistant d'un test ou d'un build à l'autre, et on pense que ca serait la réponsabilité des tests de faire le ménage avant de se lancer, si jamais tu veux regarder.

mrflos avatar Dec 22 '22 15:12 mrflos

OK, je vais regarder d'ici début janvier (pause de fin d'année en approche). Je me permets juste une rebase des derniers commits pour ne garder que l'essentiel. merci @BDouze et @mrflos pour le debug

J9rem avatar Dec 22 '22 18:12 J9rem

pour le moment, j'ai fait un commit qui nettoie le contenu éventuel de phpunit

J9rem avatar Dec 22 '22 19:12 J9rem

Et normalement, il y a bien nettoyage des fichiers pendant les tests : https://github.com/YesWiki/yeswiki/blob/cceb5f108bf13b9a10c5ae2eda5129c16049fe05/tests/includes/services/ArchiveServiceTest.php#L221

J9rem avatar Dec 22 '22 19:12 J9rem

@J9rem je fusionne dans dev, en vue de faire des tests et rajouter des fonctions cli, on pourra directement continuer dans la branche doryphore-dev.

Merci pour cette grosse PR! Et bonne fêtes.

mrflos avatar Dec 23 '22 06:12 mrflos