Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Validating config.yaml in alpine edit, new utility functions, and various cleanups & docs #149

Merged
merged 5 commits into from
May 9, 2023
Merged

Validating config.yaml in alpine edit, new utility functions, and various cleanups & docs #149

merged 5 commits into from
May 9, 2023

Conversation

maxzinkus
Copy link
Collaborator

Closes #148

cmd/edit.go:

  • alpine edit now validates the edited config.yaml file(s)
  • If validation fails, rewrites the original config pre-edits

Validations performed:

  • malformed YAML
  • alias is invalid
  • image is invalid (not one of alpine3.16 or debian11)
  • arch not one of aarch64 or x86_64
  • cpu not a positive integer
  • memory not a positive integer >= 256
  • disk not a valid size
  • mount not a directory on the host
  • port string invalid
  • sshport not a positive integer
  • location not correct (~/.macpine/instance-name matches alias and exists)

qemu/ops.go:

  • Add GetMachineConfig(vmName string) (MachineConfig, error)
  • Add SaveMachineConfig(machineConfig MachineConfig) error

Code rewrites:

  • Use Get/Save Machine Config util funcs in all cmd/ files
  • Remove deprecated use of io/ioutil in favor of os
  • Use instance everywhere instead of vm
  • Improve log message consistency
  • Improve variable name consistency and other code smells

maxzinkus added 4 commits May 8, 2023 20:59
@maxzinkus
Use 'instance' instead of 'vm' everywhere
5465ae8
@maxzinkus
Add {Get, Save}MachineConfig util funcs in qemu/ops.go
c690eaf 
* `GetMachineConfig` replaces current pattern of:
  * get user home dir
  * read config file
  * parse yaml
  * construct `qemu.MachineConfig` object

* `SaveMachineConfig` replaces current pattern of:
  * get user home dir
  * serialize yaml
  * write config file
@maxzinkus
Use Get/Save utils in host/
eb4f403
@maxzinkus
Rewrites in cmd/ and alpine edit validation
1e80b3b 
Rewrites:

* Use Get/Save Machine Config util funcs in cmd/
* Remove deprecated use of `io/ioutil` in favor of `os`
* Improve log message consistency
* Improve variable name consistency and other code smells

`alpine edit`:

* Now validates the edit `config.yaml` file(s)
* If validation fails, rewrites the original config pre-edits
@maxzinkus maxzinkus requested a review from idroz May 9, 2023 02:11
@maxzinkus maxzinkus self-assigned this May 9, 2023
@maxzinkus maxzinkus mentioned this pull request May 9, 2023
Closed
@maxzinkus maxzinkus added documentation Improvements or additions to documentation enhancement New feature or request awaiting review Blocking on input from a core maintainer labels May 9, 2023
@maxzinkus maxzinkus added this to the 1.0.3 milestone May 9, 2023
maxzinkus
maxzinkus commented May 9, 2023
View reviewed changes
docs/docs/troubleshooting.md

### Adjusting time

Time sync issues between the host and a VM are [well known](https://github.com/canonical/multipass/issues/2430). For example, when the
host is suspended, VM clock will also stop ticking.
host is suspended, the VM clock will also stop ticking.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one section talks about VMs in general rather than macpine instances in particular. Use "VM" here only.

@maxzinkus
Copy link
Collaborator Author

Sorry this is a pretty massive review @idroz. Let me know if you have any questions!

@idroz idroz merged commit ed22c72 into main May 9, 2023
@maxzinkus maxzinkus deleted the edit-validate branch May 9, 2023 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@idroz idroz idroz approved these changes

Assignees

@maxzinkus maxzinkus

Labels
awaiting review Blocking on input from a core maintainer documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Milestone
1.0.3
Development

Successfully merging this pull request may close these issues.

[Feature] Validate config.yaml at the end of alpine edit
2 participants
@maxzinkus @idroz