Merge pull request #232 from seddonym/review-11-seddonym

Review Ansible chapter

Author: hjwp

chapter_10_production_readiness.asciidoc

@@ -555,6 +555,9 @@ It's not quite working yet!  Let's take a look manually: <<django-400-error>>.
 .An ugly 400 error
 image::images/twp2_1002.png["An unfriendly page showing 400 Bad Request"]
 
+// DAVID: noticed in passing that the screenshots in this chapter are from the hosted version
+// which isn't covered until next chapter.
+
 We've set our two environment variables but doing so seems to have broken things.
 But once again, by running our FTs frequently,
 we're able to identify the problem early,

chapter_11_ansible.asciidoc

@@ -69,6 +69,8 @@ just pick any old registrar and buy a cheap one--it
 should only cost you $5 or so, and you can even find free ones.
 I promise seeing your site on a "real" website will be a thrill.
 
+// DAVID: just wondering if it's worth giving them the option to cheat and
+// specify a domain name in a hosts file?
 
 
 === Manually Provisioning a Server to Host Our Site
@@ -187,6 +189,7 @@ wild place!
 ((("Platform-As-A-Service (PaaS)")))
 *******************************************************************************
 
+// DAVID: looks like plusbryan.com has changed hands.
 ////
 
 TODO: good advice but not quite sure it's phrased quite right for the new version of the chapter.
@@ -261,7 +264,7 @@ rather than specifying a procedural series of steps to be followed one by one.
 
 ==== Installing Ansible
 
-Take a look at the  https://docs.ansible.com/ansible/latest/installation_guide/intro_installation.html [documentation] for instructions on how to install Ansible/
+Take a look at the  https://docs.ansible.com/ansible/latest/installation_guide/intro_installation.html [documentation] for instructions on how to install Ansible.
 // RITA: Please anchor all URLs to descriptive text.
 
 The simplest thing to do is to install Ansible into the virtualenv
@@ -293,9 +296,11 @@ The "hate" part is that the actual syntax is surprisingly fiddly to get right:
 the difference between lists and key/value maps is subtle and I can never quite remember it honestly.]
 for.
 
+// DAVID: I didn't know if this was an example or a file to create.
+// I think it's the latter, it would be good to make that clear.
 
 [role="sourcecode"]
-.infra/ansible-provision.yaml (ch11l001)
+    .infra/ansible-provision.yaml (ch11l001)
 ====
 [source,yaml]
 ----
@@ -342,7 +347,7 @@ I often skip to the
 https://docs.ansible.com/ansible/latest/collections/ansible/builtin/apt_module.html#examples[Examples section].
 
 // RITA: Please add a sentence or two to introduce the following code block.
-
+// DAVID: +1, this wrong footed me.
 [subs="specialcharacters,quotes"]
 ----
 $ *ansible-playbook --user=elspeth -i staging.ottg.co.uk, infra/ansible-provision.yaml -vv*
@@ -388,6 +393,25 @@ PLAY RECAP ********************************************************************
 staging.ottg.co.uk         : ok=3    changed=2    unreachable=0    failed=0
 skipped=0    rescued=0    ignored=0
 ----
+// DAVID: rather than having to edit the username and domains each time,
+// what about getting the reader to set them as environment variables at the beginning of the chapter?
+
+// DAVID: I got ansible-playbook: error: unrecognized arguments: --vv
+// ansible==9.4.0
+// ansible-core==2.16.6
+// Changed it to -v
+
+// DAVID: Then I got:
+// FAILED! => {"msg": "Missing sudo password"}
+// I don't have much context about what's going on here, e.g. there are several machines
+// this could be interacting with - my local, a container in my local,
+// the remote, a container in remote. Or maybe I'm contacting the wrong remote computer.
+// So not that clear how to troubleshoot.
+// (I did set up a sudo user on the server as per your github guide.)
+// Maybe giving a bit more explanation on what this command is doing would help.
+// UPDATE: got this working with `--ask-become-pass` - we discussed this in person.
+// But, as discussed, it is annoying to have to keep typing your password in when we could
+// authenticate with the ssh key. Could we change this?
 
 I don't know about you, but whenever I make a terminal spew out a stream
 of output, I like to make little _brrp brrp brrp_ noises, a bit like the
@@ -425,6 +449,11 @@ elspeth@server:$ *docker logs testcontainer*
 hello world
 ----
 
+// DAVID: Got
+// permission denied while trying to connect to the Docker daemon socket at unix:///var/run/docker.sock
+// Sudo fixed it.
+// Update: I think better to add my user to the docker group before this point.
+
 TIP: Look out for that `elspeth@server`
     in the command-line listings in this chapter.
     It indicates commands that must be run on the server,
@@ -570,6 +599,13 @@ false, "AttachStdout": true, "Cmd": ["gunicorn", "--bind", ":8888",
 [...]
 ----
 
+// DAVID: I got error during the 'Export container image locally' step.
+// FAILED! => {"changed": false, "msg": "Error connecting: Error while fetching server API version:
+// ('Connection aborted.', FileNotFoundError(2, 'No such file or directory'))"}
+// Update: I fixed it by doing ``sudo usermod -aG docker ${USER}`` on the remote machine.
+// Maybe worth getting them to shell in and run `docker run hello-world` to
+// check their user has permission.
+// DAVID: by the way it's getting annoying having to type my sudo password in all the time.
 
 For completeness, let's also add a step to explicitly build the image locally.
 This means we don't have a dependency on having run `docker build` locally.
@@ -621,6 +657,10 @@ PORTS     NAMES
 3a2e600fbe77   busybox   "echo hello world"   2 days ago   Exited (0) 10
 minutes ago             testcontainer
 
+// DAVID: I can also see superlists listed (though exited).
+// In any event, feels like needs a bit more explanation of
+// your thought process?
+
 elspeth@server:$ *docker logs testcontainer*
 [2024-02-26 22:19:15 +0000] [1] [INFO] Starting gunicorn 21.2.0
 [2024-02-26 22:19:15 +0000] [1] [INFO] Listening at: http://0.0.0.0:8888 (1)
@@ -665,6 +705,9 @@ It's a common tool in Ansible scripts, and the syntax is very similar to Django'
 
 Here's what our template for the env file will look like:
 
+// DAVID: These days wouldn't it be called env.jinja as
+// per https://jinja.palletsprojects.com/en/3.1.x/templates/#template-file-extension?
+
 [role="sourcecode"]
 .infra/env.j2 (ch11l004)
 ====
@@ -799,9 +842,14 @@ staging.ottg.co.uk         : ok=8    changed=6    unreachable=0    failed=0
 skipped=0    rescued=0    ignored=0
 ----
 
-Looks good!  What do our tests think?
+// DAVID: suggest you get us to we shell in and see the env file there.
 
+Looks good!  What do our tests think?
 
+// DAVID: this command assumes we'll be in a different working directory
+// to the previous command.
+// Also... I originally just pasted this as-is, which contacted YOUR server. Another
+// reason to get them to set environment variables at the start of the chapter.
 ==== More debugging
 
 We run our tests as usual and run into a new problem:
@@ -814,6 +862,9 @@ selenium.common.exceptions.WebDriverException: Message: Reached error page:
 about:neterror?e=connectionFailure&u=http%3A//staging.ottg.co.uk/[...]
 ----
 
+// DAVID: I got selenium.common.exceptions.NoSuchElementException
+// On the server, I get 'Host not found'.
+
 That `neterror` makes me think it's another networking problem.
 Let's try `curl` locally:
 
@@ -826,6 +877,8 @@ curl: (7) Failed to connect to staging.ottg.co.uk port 80 after 25 ms: Couldn't
 connect to server
 ----
 
+// DAVID: I was able to connect, and got Host not found.
+
 Now let's ssh in and try `curl` from the server itself:
 
 [subs="specialcharacters,quotes"]
@@ -892,6 +945,8 @@ selenium.common.exceptions.NoSuchElementException: Message: Unable to locate
 element: [id="id_list_table"]; [...]
 ----
 
+// DAVID: tests pass for me now! And I can create lists on the server...
+// Strange, I'm pretty sure I didn't run migrations.
 
 === Mounting the database on the server and running migrations
 
@@ -905,6 +960,7 @@ $ *ssh elspeth@server docker logs superlists*
 django.db.utils.OperationalError: no such table: lists_list
 ----
 
+// DAVID: confused by this next para.
 
 [subs="specialcharacters,quotes"]
 ----
@@ -1001,6 +1057,7 @@ and reloads it automatically if it crashes.
 (used to need systemd, now you can just set restart_policy.
 ////
 
+// DAVID: Maybe you should say this debugging stuff at the beginning, not the end!
 
 .More Debugging Tips and Commands
 *******************************************************************************
@@ -1114,7 +1171,10 @@ You now have a live website!  Tell all your friends!
 Tell your mum, if no one else is interested!
 And, in the next chapter, it's back to coding again.((("", startref="Fstage11")))
 
-
+// DAVID: maybe more of a conclusion here? It's quite a heavy chapter,
+// a bit of an anticlimax to stop here. I want some inspiring note to end on.
+// In particular, how does this tie into TDD?
+// DAVID: Also - now it's on staging, should we release to prod too?
 
 === Further Reading