Added a few checks on deployment for Windows VMs #51
Conversation
MOBergeron
force-pushed
the
windows-vm
branch
from
d7be9f4 to
c5d6584
Compare
|
|
||
| if "}" == line and name: | ||
| tracks.add( | ||
| Track( |
There was a problem hiding this comment.
messemble que ce code là est weird. Tu crées une Track et un des paramètres au constructeur est has_virtual_machine et pour savoir ça tu crées une autre Track? Ça l'air redondant
There was a problem hiding this comment.
C'est que pour la constance j'ai voulu répliqué la fonction does track needs build container. Mais ce n'était pas la bonne solution effectivement. Je devrais prendre un str ou Track, puis de là faire le traitement. Ça éviterait de créer un objet nowhere.
ctf/utils.py
Outdated
| ) and bool(load_yaml_file(build_yaml_file_path)) | ||
|
|
||
|
|
||
| def does_track_has_virtual_machine(track: Track) -> bool: |
There was a problem hiding this comment.
soit does_track_have_virtual_machine() ou track_has_virtual_machine(). Je préfère 2e option perso
| ansible_incus_host: build-container | ||
| {% endif %}{% if data.is_windows %} | ||
| # This section is needed if you need Windows virtual machines. It's a group of hosts regrouped under the name "windows" which MUST remain the same. | ||
| # The group "windows" is removed from the "cleanup.yaml" and "common.yaml", which is why you should not change it. |
There was a problem hiding this comment.
pas sûr de comprendre cette ligne. le groupe windows est retiré de cleanup.yaml et common.yaml, mais ta PR ne modifie pas ces fichiers?
There was a problem hiding this comment.
Je les ai enlevé sur mon instance local de ctf. Mais j'ai oublié de l'enlever des fichiers templates, ouch. Good catch.
ctf/templates/new/common/main.tf.j2
Outdated
| profiles = ["default", incus_profile.this.name] | ||
| type = {% if data.is_windows %}"virtual-machine"{% else %}"container"{% endif %} | ||
|
|
||
| image = {% if data.is_windows %}"CHANGE_ME" # Change to the Windows image location{% else %}"images:ubuntu/24.04"{% endif %} |
There was a problem hiding this comment.
mettre instructions sur comment trouver cette image location?
There was a problem hiding this comment.
Ok. Je pourrais. C'est une image self created dans Incus... Stéphane parlait d'avoir des pourparlers avec Microsoft pour avoir des images Windows dans Incus mais ce n'est pas fait encore. Mais oui. C'est compliqué par contre de dire exactement comment faire une image locale. Faudrait que je pointe vers le README et expliquer comment faire dans le README.
| # Waiting for virtual machine to be up and running | ||
| # Starting with a minute | ||
| if start_timer > time.time() - (seconds := 60.0): | ||
| LOG.info( |
There was a problem hiding this comment.
ça serait mieux d'utiliser https://rich.readthedocs.io/en/latest/progress.html à la place
There was a problem hiding this comment.
Pas certain de comprendre comment utiliser progress, surtout dans ce contexte. Penses-tu pouvoir t'en occuper?
There was a problem hiding this comment.
je sais pas comment utiliser ça. Quel nom d'image windows je peux utiliser?
There was a problem hiding this comment.
Tu dois créer ta propre image avec incus-windows par antifob, voir le README.
ctf/deploy.py
Outdated
|
|
||
| match s.returncode: | ||
| case 127: | ||
| # What else should be put here? |
There was a problem hiding this comment.
jcomprend pas, pourquoi ce case et debug log?
There was a problem hiding this comment.
Une VM pourrait être Windows ou Linux. Donc il faut une commande commune dans chaque OS. Et si le binaire n'est pas trouvé, j'essaie avec un autre binaire. Au pire on pourrait commencer par cmd puis aller vers sh après si sh existe sur tous les Unix.
There was a problem hiding this comment.
J'ai changé un peu le commentaire.
MOBergeron
force-pushed
the
windows-vm
branch
4 times, most recently
from
7ea5f0c to
074e004
Compare
MOBergeron
force-pushed
the
windows-vm
branch
from
074e004 to
65f6ef5
Compare
2 participants
No description provided.