Fixes #348 resizes private volume sizes of sd-app sd-log #405

kushaldas · 2020-01-13T11:33:42Z

sd-app:private is now 10GB
sd-log:private is now 5GB

How to test?

Clone the repo to dom0 and get the config.json and sd-journalist.sec file for your installation
Update config.json to have the vmsizes,

"vmsizes": {
    "sd_app": 10,
    "sd_log": 5
  },

run make sd-app
run make sd-log
run make test
Now, manually change the size for sd-app to 11GB `qvm-volume resize sd-app:private 11GiB
Run make sd-app again, this should fail with a proper failure message.

All of these commands should execute properly.

If you can check the private volume size of the sd-app and sd-log vm in the GUI too.

emkll

Thanks @kushaldas the resize for sd-svs is working as expecting!

However, I experienced some issues with testing sd-log (it appears the test for checking the size is not run, see inline, but is also testing the template VM and not the AppVM, which is strange)

I think we should make the size configurable via the config.json. The diff should be quite small, and it will have significant benefits for the pilot:

Every org (or developer) will have different requirements for storage space (10GiB is ~20 max size submissions, developers use their Qubes workstations for many other things)
Having this hardcoded in .sls is problematic should a user want to make local changes: these changes will be squashed when the .sls is updated.
Per the above, should the .sls file have a size smaller than the current volume, an error will be thrown during provisioning [1]

I think, prior to merge we should:

ensure sd-log (appvm) tests are run as expected
make the value configurable via config.json

[1]

----------
          ID: sd-svs-private-volume-size
    Function: cmd.run
        Name: qvm-volume resize sd-svs:private 9GiB

      Result: False
     Comment: Command "qvm-volume resize sd-svs:private 9GiB
              " run
     Started: 10:36:13.881554
    Duration: 118.459 ms
     Changes:   
              ----------
              pid:
                  19991
              retcode:
                  1
              stderr:
                  For your own safety, shrinking of private is disabled (9663676416 < 10737418240). If you really know what you are doing, resize filesystem manually first, then use `-f` option.
              stdout:
----------

emkll · 2020-01-15T15:22:09Z

tests/test_vms_exist.py

+        # Should be 5GB
+        # >>> 1024 * 1024 * 5 * 1024
+        vol = vm.volumes["private"]
+        self.assertEqual(vol.size, 5368709120)


Here, you are testing the template vm, not the AppVM (sd-log). Strangely though, this test is not failing. Perhaps it is not running?

emkll · 2020-01-15T15:35:20Z

dom0/sd-svs.sls

+sd-svs-private-volume-size:
+  cmd.run:
+    - name: >
+        qvm-volume resize sd-svs:private 10GiB


✔️ It seems like qvm.volume is not implemented in salt, so we must use cmd.run here

emkll · 2020-01-20T18:05:32Z

Due to changes introduced in #407, this will also need a rebase on latest master: sd-svs -> sd-app

Could you also please populate the contents of the config.json.example file with the default values?

emkll

Thanks for the changes @kushaldas . I have taken another pass, and was not able to apply these changes on my local workstation: I had to replace sd-svs by sd-app in validate-config locally to get the config validation to pass. I have added some more comments inline.

emkll · 2020-01-21T14:42:06Z

scripts/validate-config

+        assert "sd_log" in self.config["vmsizes"]
+
+        app = Qubes()
+        if "sd-svs" in app.domains:


This domain should no longer exist due to the vm rename introduced in #407

emkll · 2020-01-21T14:45:08Z

scripts/validate-config

+            ), "sd-svs private volume is already bigger than configuration."
+
+        if "sd-log" in app.domains:
+            vm = app.domains["sd-svs"]


Here, the vol variable represents the private volume of the sd-svs vm. Is this what was intended?

emkll · 2020-01-21T14:55:11Z

Makefile

@@ -61,6 +61,7 @@ clean-salt: assert-dom0 ## Purges SD Salt configuration from dom0

 prep-salt: assert-dom0 ## Configures Salt layout for SD workstation VMs
 	@./scripts/prep-salt
+	@./scripts/validate-config


there's a validate makefile target that could be used here. We should validate the config before running prep-salt here, since prep-salt is not strictly required if the config fails to validate

config.json.example

scripts/validate-config

emkll · 2020-01-21T15:00:32Z

scripts/validate-config

+
+        app = Qubes()
+        if "sd-svs" in app.domains:
+            vm = app.domains["sd-svs"]


same as comment from L:101

ah, I will have to update this.

emkll · 2020-01-21T15:00:40Z

scripts/validate-config

+        """This method checks for existing private volume size and new
+        values in the config.json"""
+        assert "vmsizes" in self.config
+        assert "sd_svs" in self.config["vmsizes"]


same as comment from L:101

emkll · 2020-01-21T15:01:01Z

scripts/validate-config

+            vm = app.domains["sd-svs"]
+            vol = vm.volumes["private"]
+            assert (
+                vol.size <= self.config["vmsizes"]["sd_svs"] * 1024 * 1024 * 1024


same as comment from L:101

emkll · 2020-01-22T15:23:47Z

rebased on latest master

emkll

Thanks @kushaldas for your patience and the changes, they look great! Went through functional testing and everything works as expected. One final request and this should be good to merge:

qvm-volume resize only appears to accept integers, adding a non integer (e.g: 6.5) returns a failure [1]. We should either validate (or round) the value in the config, or document this constraint. Since we cannot add comments in the json file, so it may be best if we addressed this in the validator. What do you think?

[1]

----------
          ID: sd-log-private-volume-size
    Function: cmd.run
        Name: qvm-volume resize sd-log:private 6.5GiB

      Result: False
     Comment: Command "qvm-volume resize sd-log:private 6.5GiB
              " run
     Started: 10:33:17.419232
    Duration: 116.458 ms
     Changes:   
              ----------
              pid:
                  5780
              retcode:
                  1
              stderr:
                  Traceback (most recent call last):
                    File "/bin/qvm-volume", line 5, in <module>
                      sys.exit(main())
                    File "/usr/lib/python3.5/site-packages/qubesadmin/tools/qvm_volume.py", line 354, in main
                      args.func(args)
                    File "/usr/lib/python3.5/site-packages/qubesadmin/tools/qvm_volume.py", line 233, in extend_volumes
                      size = qubesadmin.utils.parse_size(args.size)
                    File "/usr/lib/python3.5/site-packages/qubesadmin/utils.py", line 49, in parse_size
                      return int(size) * multiplier
                  ValueError: invalid literal for int() with base 10: '6.5'
              stdout:

Summary for local
-------------
Succeeded: 38 (changed=6)
Failed:     1
-------------
Total states run:     39
Total run time:    9.235 s

kushaldas · 2020-01-22T16:05:12Z

@emkll thank you for the comment, i will update it tomorrow in my time.

eloquence · 2020-01-22T18:44:58Z

scripts/validate-config

+            vol = vm.volumes["private"]
+            assert (
+                vol.size <= self.config["vmsizes"]["sd_app"] * 1024 * 1024 * 1024
+            ), "sd-svs private volume is already bigger than configuration."


The config.json now must have sizes defined for sd-svs and sd-log private volumes. "vmsizes": { "sd_svs": 10, "sd_log": 5 }, The values must be an integer. The Makefile prep-salt target will also run the validate-config check to make sure that size checks are done in case of: make sd-app make sd-log The validate-config will fail in case the given size is smaller than the current size of the private volumes in the existsing VMs.

kushaldas · 2020-01-23T10:59:31Z

[1]. We should either validate (or round) the value in the config, or document this constraint. Since we cannot add comments in the json file, so it may be best if we addressed this in the validator. What do you think?

I have added a validation check for integer values of the private volume sizes.

emkll

Thanks @kushaldas , changes look good !

kushaldas requested review from conorsch and emkll January 14, 2020 13:31

emkll self-assigned this Jan 14, 2020

emkll suggested changes Jan 15, 2020

View reviewed changes

kushaldas force-pushed the we_want_more_space_give_us_space_moar_space branch from fd4656f to 701e90e Compare January 20, 2020 08:24

kushaldas force-pushed the we_want_more_space_give_us_space_moar_space branch from 701e90e to 4574ab0 Compare January 21, 2020 06:34

emkll suggested changes Jan 21, 2020

View reviewed changes

kushaldas force-pushed the we_want_more_space_give_us_space_moar_space branch from 4574ab0 to 5303833 Compare January 22, 2020 07:50

emkll changed the title ~~Fixes #348 resizes private volume sises of sd-svs sd-log~~ Fixes #348 resizes private volume siszs of sd-svs sd-log Jan 22, 2020

emkll changed the title ~~Fixes #348 resizes private volume siszs of sd-svs sd-log~~ Fixes #348 resizes private volume sizes of sd-svs sd-log Jan 22, 2020

emkll force-pushed the we_want_more_space_give_us_space_moar_space branch from 5303833 to 8e48b14 Compare January 22, 2020 15:23

emkll reviewed Jan 22, 2020

View reviewed changes

eloquence reviewed Jan 22, 2020

View reviewed changes

eloquence changed the title ~~Fixes #348 resizes private volume sizes of sd-svs sd-log~~ Fixes #348 resizes private volume sizes of sd-app sd-log Jan 22, 2020

kushaldas force-pushed the we_want_more_space_give_us_space_moar_space branch from 8e48b14 to 3e39647 Compare January 23, 2020 10:58

emkll approved these changes Jan 23, 2020

View reviewed changes

emkll merged commit 6a35ee0 into master Jan 23, 2020

emkll deleted the we_want_more_space_give_us_space_moar_space branch January 23, 2020 13:37

eloquence mentioned this pull request Jul 28, 2021

sd-app/sd-log resize logic can conflict with custom changes #716

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #348 resizes private volume sizes of sd-app sd-log #405

Fixes #348 resizes private volume sizes of sd-app sd-log #405

kushaldas commented Jan 13, 2020 •

edited by eloquence

Loading

emkll left a comment

emkll Jan 15, 2020

emkll Jan 15, 2020

emkll commented Jan 20, 2020 •

edited

Loading

emkll left a comment

emkll Jan 21, 2020

emkll Jan 21, 2020

emkll Jan 21, 2020

emkll Jan 21, 2020

kushaldas Jan 21, 2020

emkll Jan 21, 2020

emkll Jan 21, 2020

emkll commented Jan 22, 2020

emkll left a comment

kushaldas commented Jan 22, 2020

eloquence Jan 22, 2020

kushaldas commented Jan 23, 2020

emkll left a comment

Fixes #348 resizes private volume sizes of sd-app sd-log #405

Fixes #348 resizes private volume sizes of sd-app sd-log #405

Conversation

kushaldas commented Jan 13, 2020 • edited by eloquence Loading

How to test?

emkll left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emkll commented Jan 20, 2020 • edited Loading

emkll left a 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

emkll commented Jan 22, 2020

emkll left a comment

Choose a reason for hiding this comment

kushaldas commented Jan 22, 2020

Choose a reason for hiding this comment

kushaldas commented Jan 23, 2020

emkll left a comment

Choose a reason for hiding this comment

kushaldas commented Jan 13, 2020 •

edited by eloquence

Loading

emkll commented Jan 20, 2020 •

edited

Loading