Arvados

Looks like the "unknown service type" handling is already correct.

        if service.ReadOnly == false {
            writableLocalRoots[service.Uuid] = url
            if service.SvcType != "disk" {
                this.replicasPerService = 0
            }
        }

So, maybe we just need

• use proxy timeouts if any non-disk (instead of "if any proxy")
• tests

We still seem to set Using_proxy to true and false in various places, but we never read it, so presumably we can just delete all references to it?

• $ git grep Using_proxy
  sdk/go/keepclient/discover.go:  this.Using_proxy = false
  sdk/go/keepclient/discover.go:                  this.Using_proxy = true
  sdk/go/keepclient/keepclient.go:        Using_proxy        bool
  sdk/go/keepclient/keepclient.go:                Using_proxy:   false,
  sdk/go/keepclient/keepclient_test.go:   kc.Using_proxy = true
  sdk/go/keepclient/keepclient_test.go:   kc.Using_proxy = true
  services/datamanager/datamanager_test.go:               Using_proxy:   true,
  services/keepproxy/keepproxy_test.go:   kc.Using_proxy = true
  services/keepstore/keepstore.go:                Using_proxy:   true,
  services/keepstore/pull_worker_integration_test.go:             Using_proxy:   true,
  

These assertions are not effective -- the nil value of a string is "", not nil, so the assertion will pass even if the map doesn't have the desired entry. Should be Not(Equals), "" ...?

•     c.Assert(kc.LocalRoots()[blobKeepService["uuid"].(string)], NotNil)
  

Pushed 5f7c3a3 to get some test coverage (admittedly not much) on keep_services#accessible.

Related: datamanager errors out if it sees a service_type other than "disk". The easiest way to fix this for typical cases also gives us an interim solution for #7853:

• Make the "disk" string configurable with a command line argument (-service-type=X, default "disk")
• If other service types are encountered, log that they're being ignored, but then continue.
• Error out only if there are no services with the configured type.

We still seem to set Using_proxy to true and false in various places, but we never read it, so presumably we can just delete all references to it?

Removed all these references. All the tests seem to be passing (still running tests). However, do we need to set ArvadosClient.External in any of these places?

These assertions are not effective -- the nil value of a string is "", not nil

Thanks for noticing. Updated these.

Related: datamanager errors out if it sees a service_type other than "disk".

Added an extra service-type arg with default value "disk" in keep.go and used it instead.

No need to write the default in the help message, the flag package does it for us:

•   -service-type string
          Supported keepservice type. Default is disk. (default "disk")
  

• "Supported" is a bit mysterious too; suggest "Operate only on keep_services with the specified service_type, ignoring all others."

The words "indexable" and "supported" seem to be used interchangeably in the names "indexableKeepServers" and "supportedServiceType". Perhaps they could just be called "selectedKeepServers" and "selectedServiceType", or even just "keepServers" and "serviceType"?

Likewise, the word "unsupported" in "Ignore unsupported service type: %v" seems weird (I'd expect "unsupported" to mean "datamanager can't handle this service_type"). How about:

• "Skipping keep_service %q because its service_type %q does not match -service-type=%q"

This comment is superfluous -- it's already obvious from the code.

• // Ignore any services that are of type other than the "supportedServiceType".
  // If no services of the "supportedServiceType" are found, raise an error.
  

Noting we don't have one of the tests requested by the issue:

Ensure that the client only sends one write when the service reports that it wrote enough replicas

(This shouldn't hold up the current branch, but still seems like a good idea. Perhaps we already have a test like this attached to the "max replicas expected per PUT" code?)

• Made all those text updates for -service-type

• Test "Ensure that the client only sends one write when the service reports that it wrote enough replicas". TestMakeKeepClientWithNonDiskTypeService asserts that kc.replicasPerService is set correctly, though there is room for more tests.

LGTM @ 311993c

There aren't any merge conflicts here, so doing "git rebase master" on this branch before merging it will eliminate some "merge master" noise in the git history.