Bug 1538321 - Continue to load specs even when a spec fails to load

[rthallisey]

Describe what this PR does and why we need it:
When a spec fails to load, it will cause all the specs from that registry adaptor to be ignored. If the broker is configure to look at only one adaptor, it causes the broker to fail.

Let's log the spec failure and continue processing valid specs.

Changes proposed in this pull request

• Be more fault tolerant to failed specs

Which issue this PR fixes (This will close that issue when PR gets merged)
fixes #681

[shawn-hurley]

I personally don't like this approach.

This needs to be changed on the interface, if we are never going to return something went wrong I think we should force that across all adapters IMO. I think this is a conversation that we should have about what we expect from the registry perspective if an adapter throws an error here. Do we want to respect it? do we want the adapter to handle its own errors and just pass us a clean list of specs?

[rthallisey]

The problem with moving out to the interface is with the local_openshift_adapter unfortunately. It has a few places where we need to respect the errors for, like creating the openshift client, checking the ip of the docker-registry, and loading images from the docker-registry.

I think this is a conversation that we should have about what we expect from the registry perspective if an adapter throws an error here. Do we want to respect it? do we want the adapter to handle its own errors and just pass us a clean list of specs?

I think it's worth having. In the near term though (3.9), this PR will ignore any failed specs which will prevent anyone from hitting the issue. I think it's worth coming back to do more thorough improvement on this. We can do things like log how many bad specs were found in a repo and define a firmer policy on handling registry errors.

[shawn-hurley]

👍 that sounds reasonable to me

[jmontleon]

I like it, at least for the near term. It seems better than having a bad APB update potentially crashing brokers.

[jmrodri]

It's too late for my comments on this but instead of eating the errors we should've fixed the registry.go code to not lose all of the specs
https://github.com/openshift/ansible-service-broker/blob/master/pkg/registries/registry.go#L136-L142

	// Debug output filtered out names.
	specs, err := r.adapter.FetchSpecs(validNames)
	if err != nil {
		log.Errorf("unable to fetch specs for registry %v - %v",
			r.config.Name, err)
		return []*apb.Spec{}, 0, err
}