Merge pull request #31 from rogpeppe/033-better-docs

all: add some doc comments

TODO.md

@@ -19,52 +19,38 @@ charm has been upgraded?
 We could also check local state for backward
 compatibility.
 
-Misc
-----
+Compressed binary support
+----------------------
 
-Use gopkg.in/errgo.v1 package.
+We could support a gzipped executable and uncompress
+at install/upgrade-charm time.
+Given that we are not always guaranteed to call upgrade-charm,
+we should probably compare runhook.gz and runhook binary
+mtimes on every hook execution and uncompress when
+runhook.gz is updated.
 
-Avoid mutation of charms in place
----------------------------
-
-Just use normal Go package. Perhaps the package name must be "runhook"
-(along the same lines as "main").
-
-	gocharm [-series $series1,$series2...] [-f] [-build architecture] [-vendor] [-godeps] [-name $charmname] $packagepath 
-
-Make new charm directory, $d
-
-cp -r $packagepath to $d/src/$packagepath
-
-except for:
-	assets -> $d/assets
-	README.md -> $d/README.md
-	metadata.yaml -> $d/metadata.yaml (with relations added)
-
-runhook.go gets generated and put into $d/runhook.go, importing
-from $packagepath.
-
-runhook gets compiled and put into $d/bin/runhook
+Testing
+------
 
-We put the resulting charm in $JUJU_REPOSITORY by default.
-If there's anything in the target directory that's not in:
+Support for testing service-based charms.
+Change RegisterCommand so that instead of taking
+a func(args []string), it takes a func(args []string) (Worker, error),
+where: 
 
-	src/...
-	bin/...
-	assets/...
-	README*
-	metadata.yaml
-	config.yaml
+	type Worker interface {
+		Kill()
+		Wait() error
+	}
 
-then we abort; otherwise we wipe out everything from the
-target directory and replace it with stuff copied from the charm
-package as specified above.
+This will enable testing code to stop the service that's
+been started (and also potentially allow graceful shutdown
+when the service gets a non-lethal signal)
 
 Support for cross-series / architecture compilation.
 -----------------------------
 
 The default destination charm series should be taken from the current series.
-We should support a -arch flag to compile for other architectures, and
+We should support a -build <architecture> flag to compile for other architectures, and
 have some way for the hooks to know what binary to run
 depending on the host architecture.
 
@@ -80,8 +66,17 @@ Possible for command line flags for the future:
 
 With -deploy, we can just make a repository in /tmp before deploying it to juju.
 
-Naming
+Logging
 ------
 
-Perhaps rename charmbits/*charm to charmbits/*relation
-e.g. httprelation.Provider.
+Integration of service output with charm output might be nice.
+At least some better visisibility of service output would be good.
+
+HTTP service
+----------
+
+We should not pass all the arguments in the upstart
+file and restart every time anything changes. We could
+implement a better scheme that passes
+the arguments through the socket and restarts the server
+only when necessary.

charmbits/httpservice/service.go

@@ -54,6 +54,10 @@ type localState struct {
 // for some type T that can be marshaled as JSON.
 // When the service is started, this function will be called
 // with the arguments provided to the Start method.
+//
+// Note that the handler function will not be called with
+// any hook context available, as it is run by the OS-provided
+// service runner (e.g. upstart).
 func (svc *Service) Register(r *hook.Registry, serviceName, relationName string, handler interface{}) {
 	h, err := newHandler(handler)
 	if err != nil {

charmbits/service/service.go

@@ -52,6 +52,11 @@ type localState struct {
 // with the context for the running service and any arguments
 // that were passed to the Service.Start method.
 // When the start function returns, the service will exit.
+//
+// Note that when the start function is called, the hook context
+// will not be available, as at that point the hook will be
+// running in the context of the OS-provided service runner
+// (e.g. upstart).
 func (svc *Service) Register(r *hook.Registry, serviceName string, start func(ctxt *Context, args []string)) {
 	if start == nil {
 		panic("nil start function passed to Service.Register")

example-charms/concat/runhook.go

@@ -1,7 +1,33 @@
-// The concat package implements a
-// charm that takes all the string values from units on upstream
-// relations, concatenates them and makes them available to downstream
-// relations.
+// The concat package implements a charm that takes all the string
+// values from units on upstream relations, concatenates them and makes
+// them available to downstream relations.
+//
+// This provides a silly but simple example that
+// allows the exploration of relation behaviour in Juju.
+//
+// For example, here's an example of this being used from the juju-utils repository:
+//     JUJU_REPOSITORY=$GOPATH/src/launchpad.net/juju-utils/cmd/gocharm/example-charms
+//     export JUJU_REPOSITORY
+//     gocharm
+//     juju deploy local:concat concattop
+//     juju deploy local:concat concat1
+//     juju deploy local:concat concat2
+//     juju deploy local:concat concatjoin
+//     juju add-relation concattop:downstream concat1:upstream
+//     juju add-relation concattop:downstream concat2:upstream
+//     juju add-relation concat1:downstream concatjoin:upstream
+//     juju add-relation concat2:downstream concatjoin:upstream
+//     juju set concattop 'val=top'
+//     juju set concat1 'val=concat1'
+//     juju set concat2 'val=concat2'
+//     juju set concatjoin 'val=concatjoin'
+//
+// The final value of the downstream relation provided by
+// by the concatjoin service in this case will be:
+//
+//     {concatjoin {concat2 {top}} {concat1 {top}}}
+//
+// Feedback loops can be arranged for further amusement.
 package concat
 
 import (

example-charms/do-nothing/runhook.go

@@ -1,29 +1,10 @@
-// Package mycharm implements the simplest possible Go charm.
+// Package do-nothing implements the simplest possible Go charm.
 // It does nothing at all when deployed.
-package mycharm
+package runhook
 
 import (
 	"github.com/juju/gocharm/hook"
 )
 
-type nothing struct {
-	ctxt *hook.Context
-}
-
 func RegisterHooks(r *hook.Registry) {
-	var n nothing
-	r.RegisterContext(n.setContext, nil)
-	r.RegisterHook("install", n.hook)
-	r.RegisterHook("start", n.hook)
-	r.RegisterHook("config-changed", n.hook)
-}
-
-func (n *nothing) setContext(ctxt *hook.Context) error {
-	n.ctxt = ctxt
-	return nil
-}
-
-func (n *nothing) hook() error {
-	n.ctxt.Logf("hook %s is doing nothing at all", n.ctxt.HookName)
-	return nil
 }

example-charms/helloworld-configurable/runhook.go

@@ -1,3 +1,10 @@
+// The helloworld-configurable package implements an example
+// charm similar to http://godoc.org/github.com/juju/gocharm/example-charms/helloworld
+// but which allows the message to be configured, demonstrating how
+// changing configuration options can affect a running service.
+//
+// Once deployed, the message can be changed with:
+// 	juju set helloworld-configurable message='my new message'
 package runhook
 
 import (

example-charms/helloworld/runhook.go

@@ -1,4 +1,33 @@
-package runhook
+// The helloworld package implements an example charm that
+// exposes an HTTP service that returns "hello, world"
+// from every endpoint.
+//
+// To deploy it, first build it:
+//
+//    gocharm github.com/juju/gocharm/example-charms/helloworld
+//
+// Then deploy it to a juju environment and expose the service:
+//
+//	juju deploy local:trusty/helloworld
+//	juju expose helloworld
+//
+// Then, when the helloworld unit has started, find the public
+// address of it and check that it works:
+//
+//	curl http://<address>/
+//
+// The port that it serves on can be configured with:
+//
+//	juju set helloworld http-port=12345
+//
+// It can also be configured to serve on https by
+// setting the https-certificate configuration option
+// to a PEM-format certificate and private key.
+//
+// See http://godoc.org/github.com/juju/gocharm/example-charms/helloworld-configurable
+// for a slightly more advanced version of this charm
+// that allows the message to be configured.
+package helloworld
 
 import (
 	"fmt"

example-charms/mongodbclient/runhook.go

@@ -1,3 +1,8 @@
+// The mongodbclient package implements an example charm that
+// acts as the client of the mongodb charm. Note that
+// a real charm that used this would pass the mongo
+// URL to a service that would make the actual connection
+// to mongo.
 package mongodbclient
 
 import (
@@ -23,6 +28,6 @@ func (c *charm) setContext(ctxt *hook.Context) error {
 }
 
 func (c *charm) changed() error {
-	c.ctxt.Logf("mongo addresses are now %q", c.mongodb.Addresses())
+	c.ctxt.Logf("mongo URL is now %q", c.mongodb.URL())
 	return nil
 }

hook/hook.go

@@ -1,8 +1,37 @@
 // The hook package provides a Go interface to the
 // Juju charm hook commands. It is designed to be used
-// alongside the gocharm command (github.com/juju/gocharm/cmd/gocharm)
+// alongside the gocharm command.
+// See http://godoc.org/github.com/juju/gocharm/cmd/gocharm .
 //
-// TODO explain more about relations, relation ids and relation units.
+// When a gocharm-compiled Juju hook runs, the first thing that happens
+// is that the RegisterHooks function is called. This is called both when
+// the hook actually runs and when the charm is built, so it is important
+// that code that runs in this context does nothing except register
+// anything that needs to be registered with the provided Registry.
+//
+// Note that it is important that the code runs deterministically - it
+// should not register different hooks or relations depending on the
+// current external environment.
+//
+// Note also that when passing a Registry to some external code, it
+// should be cloned (see the Registry.Clone method) with some locally
+// unique identifier. This identifier has a similar purpose to a field
+// name in a struct - it provides the gocharm logic with a name
+// that it can use to store data associated with registry. At runtime,
+// all local state is stored in the directory /usr/lib/juju-localstate/<env-UUID>.
+// You will see the names provided to Registry.Clone reflected in the
+// names of the files created there.
+//
+// After all hooks, relations and config options have been registered,
+// any functions registered with Registry.SetContext will be called.
+// This provides code with the Context, which is a charm's handle onto
+// the external Juju world.
+//
+// Then any registered hooks will be called in the order that they were
+// registered (except wildcard hooks, which run after any others).
+// This is the time that all your hook logic should do what it needs to,
+// such as maintaining relation settings, reacting to configuration changes,
+// etc.
 package hook
 
 import (
@@ -23,6 +52,7 @@ type RelationId string
 // UnitId is the type of the id of a unit.
 type UnitId string
 
+// Tag returns the juju "tag" name of the unit.
 func (id UnitId) Tag() names.UnitTag {
 	return names.NewUnitTag(string(id))
 }
@@ -141,6 +171,10 @@ func (ctxt *Context) CommandName() string {
 	return "cmd-" + ctxt.registryName
 }
 
+// IsRelationHook reports whether the current hook is executing
+// as a result of a relation change. If it returns true, then
+// ctxt.RelationName, ctxt.RelationId and possibly ctxt.RemoteUnit
+// will be set.
 func (ctxt *Context) IsRelationHook() bool {
 	return ctxt.RelationName != ""
 }
@@ -151,11 +185,15 @@ func (ctxt *Context) UnitTag() string {
 	return names.NewUnitTag(string(ctxt.Unit)).String()
 }
 
+// OpenPort opens the given port using the given protocol ("tcp" or "udp").
+// It if the port is already open, this is a no-op.
 func (ctxt *Context) OpenPort(proto string, port int) error {
 	_, err := ctxt.Runner.Run("open-port", fmt.Sprintf("%d/%s", port, proto))
 	return errgo.Mask(err)
 }
 
+// ClosePort closes the given port associated with the given protocol.
+// If the port is already closed, this is a no-op.
 func (ctxt *Context) ClosePort(proto string, port int) error {
 	_, err := ctxt.Runner.Run("close-port", fmt.Sprintf("%d/%s", port, proto))
 	return errgo.Mask(err)

hook/hooktest/hooktest.go

@@ -1,3 +1,4 @@
+// Package hooktest contains utilities for testing gocharm hooks.
 package hooktest
 
 import (