Skip to content

Move logging target config to templateVMs, remove redis from large template.#632

Merged
conorsch merged 4 commits intomainfrom
631-fix-logging-config
Nov 4, 2020
Merged

Move logging target config to templateVMs, remove redis from large template.#632
conorsch merged 4 commits intomainfrom
631-fix-logging-config

Conversation

@zenmonkeykstop
Copy link
Contributor

@zenmonkeykstop zenmonkeykstop commented Nov 3, 2020

Status

Ready for review

Description of Changes

Fixes #631 .

  • Moves setup of /etc/sd-rsyslog.conf to templateVMs, ensuring logging target is set for derived AppVMs.
  • Limits sd-log redis setup to sd-small-buster-template

Testing

Development

Run sudo dnf remove qubes-template-securedrop-workstation-buster in dom0. Then perform a make dev using this branch and verify that:

  • the installation completes successfully
  • logging directories for all SDW VMs are created under ~/QubesIncomingLogs in sd-log
  • Running the command logger IMAPOTATO on sd-app, sd-proxy, sd-whonix results in corresponding messages in their logfiles in sd-log.
  • redis is not installed in sd-large-buster-template.

Prod (fresh)

  • Remove any existing workstation install on the target machine
  • Build the sdw-config RPM off this branch and transfer it to dom0, also transfer the repo to ~/securedrop-workstation.
  • install the RPM: sudo dnf install sdw-config.rpm
  • switch to QA repos using sudo bash securedrop-workstation/utils/qa-switch.sh
  • install with sdw-admin --apply and verify that:
    • the installation completes successfully
    • logging directories for all SDW VMs are created under ~/QubesIncomingLogs in sd-log
    • Running the command logger IMAPOTATO on sd-app, sd-proxy, sd-whonix results in corresponding messages in their logfiles in sd-log.
    • redis is not installed in sd-large-buster-template.

Checklist

If you have made changes to the provisioning logic

  • All tests (make test) pass in dom0 of a Qubes install not tested yet

@conorsch
Copy link
Contributor

conorsch commented Nov 3, 2020

Looks great, reviewing now. Might tack on some tests to verify the state if time allows. Also will append an rc3 version bump and tag, so we can build rc3 for a final round of QA.

@conorsch
Copy link
Contributor

conorsch commented Nov 3, 2020

Twice now I've encountered an error during make dev as the logging VMs are being configured.

failure output
 ----------
            ID: sd-log-remove-rsyslog-qubes-plugin
      Function: cmd.run
          Name: /rw/config/rc.local
        Result: False
       Comment: Command "/rw/config/rc.local" run
       Started: 13:14:35.687489
      Duration: 34.629 ms
       Changes:   
                ----------
                pid:
                    1051
                retcode:
                    5
                stderr:
                    Failed to start redis.service: Unit redis.service not found.
                    Failed to start securedrop-log.service: Unit securedrop-log.service not found.
                stdout:
  ----------
            ID: remove-sd-rsyslog-config-for-logserver
      Function: file.absent
          Name: /etc/rsyslog.d/sdlog.conf
        Result: True
       Comment: File /etc/rsyslog.d/sdlog.conf is not present
       Started: 13:14:35.722320
      Duration: 0.731 ms
       Changes:   
  
  Summary for sd-log
  ------------
  Succeeded: 2 (changed=2)
  Failed:    1
  ------------
  Total states run:     3
  Total run time:  74.525 ms
Traceback (most recent call last):
  File "/usr/bin/sdw-admin", line 79, in provision_all
    subprocess.check_call([os.path.join(SCRIPTS_PATH, "scripts/provision-all")])
  File "/usr/lib64/python3.5/subprocess.py", line 271, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['/usr/share/securedrop-workstation-dom0-config/scripts/provision-all']' returned non-zero exit status 20

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/bin/sdw-admin", line 188, in <module>
    main()
  File "/usr/bin/sdw-admin", line 165, in main
    provision_all()
  File "/usr/bin/sdw-admin", line 81, in provision_all
    raise SDWAdminException("Error during provision-all")
__main__.SDWAdminException: Error during provision-all
Makefile:19: recipe for target 'dev' failed
make: *** [dev] Error 1

Currently running make dev on main branch to confirm this is a regression, since I've only been using the prod scenario recently.

@conorsch
Copy link
Contributor

conorsch commented Nov 3, 2020

Yes, looks specific to this PR. On main, the failure does not occur:

results of 'make dev' on main
sd-log:
  ----------
            ID: sd-log-remove-rsyslog-qubes-plugin
      Function: file.blockreplace
          Name: /rw/config/rc.local
        Result: True
       Comment: Changes were made
       Started: 13:26:51.693976
      Duration: 41.047 ms
       Changes:   
                ----------
                diff:
                    --- 
                    +++ 
                    @@ -8,3 +8,10 @@
                     #  rm -rf /etc/cups
                     #  ln -s /rw/config/cups /etc/cups
                     #  systemctl --no-block restart cups
                    +### BEGIN securedrop-workstation ###
                    +# Removes sdlog.conf file for rsyslog
                    +rm -f /etc/rsyslog.d/sdlog.conf
                    +systemctl restart rsyslog
                    +systemctl start redis
                    +systemctl start securedrop-log
                    +### END securedrop-workstation ###
  ----------
            ID: sd-log-remove-rsyslog-qubes-plugin
      Function: cmd.run
          Name: /rw/config/rc.local
        Result: True
       Comment: Command "/rw/config/rc.local" run
       Started: 13:26:51.738388
      Duration: 140.2 ms
       Changes:   
                ----------
                pid:
                    994
                retcode:
                    0
                stderr:
                stdout:
  ----------
            ID: remove-sd-rsyslog-config-for-logserver
      Function: file.absent
          Name: /etc/rsyslog.d/sdlog.conf
        Result: True
       Comment: File /etc/rsyslog.d/sdlog.conf is not present
       Started: 13:26:51.878932
      Duration: 0.978 ms
       Changes:   
  
  Summary for sd-log
  ------------
  Succeeded: 3 (changed=2)
  Failed:    0
  ------------

Investigating now.

@zenmonkeykstop
Copy link
Contributor Author

zenmonkeykstop commented Nov 3, 2020

@conorsch that's weird - are the redis and securedrop-log debs getting installed in sd-small-buster-template?

Conor Schaefer added 2 commits November 3, 2020 16:55
Confirms that the redis packages are NOT installed in the "sd-viewer"
VM. Really we want to ensure it's absent from all VMs based on
the large version of the consolidated templates, but effectively this is
doing that.
Since the /etc/sd-rsyslog.conf file is not yet in place on sd-log,
trying to start the "securedrop-log" service will fail. Although the
rc.local script doesn't exit on error, it will return the final error
code of a command run within it. Since the securedrop-log task is last,
it'll exit non-zero on fresh installs.
@conorsch
Copy link
Contributor

conorsch commented Nov 4, 2020

Summarizing discussion among the team, looks like the problem is reproducible if-and-only-if the qubes-template-securedrop-workstation-buster RPM is uninstalled prior to make dev. The failure is occurring during the rc.local script on sd-log, and the securedrop-log service bounce fails because its required files aren't there yet—on a fresh install, the files won't be there until sd-log is rebooted to use the TemplateVM. So, adding "exit 0" to the rc.local customizations resolves: the final task in the rc.local script determines the exit code.

I've pushed a commit doing so, and would appreciate verification from someone else that it works as expected.

All tests (make test) pass in dom0 of a Qubes install

Separately, there are a few failing tests on my machine, that don't appear to relate to this PR, but likely should be resolved as part of the 0.5.0 release. We may want to follow up with a separate PR to address those, though.

@zenmonkeykstop
Copy link
Contributor Author

make dev is now passing for me with a completely clean slate (securedrop-workstation-buster RPM removed prior). Thanks @conorsch

Seeing the same failures in make test, added output to #634

@emkll
Copy link
Contributor

emkll commented Nov 4, 2020

Also observed the same errors, but resolved them through rebooting VMs (see #634 (comment)) however, one final test is failing, due to the absence of sd-whonix logs in sd-log

@conorsch
Copy link
Contributor

conorsch commented Nov 4, 2020

one final test is failing, due to the absence of sd-whonix logs in sd-log

Log collection from sd-whonix to sd-log is working well for me, I'll rebuild and try again. If you manually run logger foo on sd-whonix, do you see those events captured?

@conorsch
Copy link
Contributor

conorsch commented Nov 4, 2020

Based on positive results above, I'm inclined to bump to rc3 & tag on this PR, then approve and merge. Thoughts, @zenmonkeykstop & @emkll?

@zenmonkeykstop
Copy link
Contributor Author

SGTM - the test plan for fresh installs has been updated to make sure that any existing securedrop-workstation-buster templateVM is removed prior to installing, so that should help shake out any more other race-condition errors.

Includes the logging fixes in
#632
@conorsch
Copy link
Contributor

conorsch commented Nov 4, 2020

Bumped version to rc3, tagged, and pushed. Approving for merge.

@conorsch conorsch merged commit ecd8ca2 into main Nov 4, 2020
@zenmonkeykstop zenmonkeykstop deleted the 631-fix-logging-config branch November 4, 2020 19:22
eaon pushed a commit to freedomofpress/securedrop-updater that referenced this pull request Sep 20, 2022
cfm pushed a commit that referenced this pull request Apr 1, 2024
Move logging target config to templateVMs, remove redis from large template.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[0.5.0] Logging client config not set up for appvms built from large, small templates.

3 participants