Arvados

15954-boot-test-cluster @ 8a719dbcdfd5da64172855ace2395ce682941214 -- developer-run-tests: #1756

(fuse tests fail because #16151)

On a system with all the dependencies needed by run-tests.sh, this brings up a test cluster on port 12345 using the code in CWD:

~/arvados$ go run ./cmd/arvados-server boot -config ./doc/examples/config/zzzzz.yml -type test -own-temporary-database -controller-address :12345 -listen-host 0.0.0.0

• "https://0.0.0.0:12345/" (controller endpoint) appears on stdout when the cluster is ready to use
• everything else (i.e., logging) is sent to stderr
• ^C or SIGTERM shuts down all child processes before exiting

There is a new test suite in source:lib/controller/integration_test.go that boots 3 clusters, with one test that saves a collection on cluster A and retrieves it by PDH from cluster B.

15954-boot-test-cluster @ a15c20803fb7a1e400a028c00d1c2dd924765a3e -- developer-run-tests: #1757

(merged master to get #16151 fix)

Some comments & questions:

• File lib/boot/cmd.go
  • Line 25: “…should call cancel.” comment, is referring to the super.cancel() or fail() func?
  • Line 81: Shouldn’t be logged using super.logger()?
  • Line 85: I think the else clause could be avoided.
• File lib/boot/supervisor.go
  • Line 75: Shouldn’t be logged using super.logger()?
  • Lines 99, 109: Can we use a more strict permission scheme for dirs/files creation?
  • Line 493: Why does autofillConfig() need the logger to be passed as an argument if it’s already on the Supervisor struct? It also seems that it’s not being used.
  • Lines 521-525: Can this be replaced with a nextPort() call?
• File lib/boot/cert.go - Line 49: Can we use a more strict permission scheme?
• Other Qs (probably out of scope of this particular story):
  • Do you think adding a -only-install-deps flag would be useful to do some cache population?
  • What happens to an owned temporary database after quitting? Can we have a not-so-temporary database too?

• File lib/boot/cmd.go
  • Line 25: “…should call cancel.” comment, is referring to the super.cancel() or fail() func?

Oops, changed to "fail".

• Line 81: Shouldn’t be logged using super.logger()?

This goes to stdout so a script can easily find the controller URL when it's ready. (Added a comment.)

• Line 85: I think the else clause could be avoided.

Yes, fixed to use handle-errors-first style.

• File lib/boot/supervisor.go
  • Line 75: Shouldn’t be logged using super.logger()?

Yes, fixed.

• Lines 99, 109: Can we use a more strict permission scheme for dirs/files creation?

Yes, fixed. (Typically umask is 022, and this is all in a temp dir with 0700, but turning off group/other-write seems sensible anyway... and config.yml sure doesn't need to be executable.)

• Line 493: Why does autofillConfig() need the logger to be passed as an argument if it’s already on the Supervisor struct? It also seems that it’s not being used.

Indeed, removed.

• Lines 521-525: Can this be replaced with a nextPort() call?

Yes, done.

• File lib/boot/cert.go - Line 49: Can we use a more strict permission scheme?

Done

• Other Qs (probably out of scope of this particular story):
  • Do you think adding a -only-install-deps flag would be useful to do some cache population?

Yes, either that or "after starting, shutdown and exit 0" which would give more assurance that setup/deps actually worked.

• What happens to an owned temporary database after quitting? Can we have a not-so-temporary database too?

Yes, for a more convenient dev/trial experience we could put a persistent data dir in /var/lib/arvados and run a dedicated postgresql server on demand -- but for production I imagine we'll still recommend providing connection info for a regular postgresql installation so we don't need to handle tuning, backups, migrating data after upgrading postgresql, etc.

15954-boot-test-cluster @ a9988d4cde254df59d1790ef1e3768d14e2a812e -- developer-run-tests: #1769

LGTM, please merge. Thanks!