fix(clouddriver): Only Disable Instances in Discovery When Discovery …

[evansb]

Fixes: spinnaker/spinnaker#6121

[robzienert]

Seems fine to me, but would like more familiar eyes on it.

[ajordens]

Most stages aren't going to have interestingHealthProviderNames explicitly defined right?

With this change I feel we're going to miss out on Discovery in almost all cases right?

We do have the DetermineHealthProvidersTask but it's provider-specific and doesn't look at underlying account attributes.

[ajordens]

I don't believe this will work in places that we actually expect Discovery?

[evansb]

Thanks for the feedback @ajordens , I reverted back to ["Discovery", "LoadBalancer"] if interestingHealthProviderNames is not defined. I am still not sure if this is the right approach, but I cannot find a better way to detect if Discovery is enabled for a specific account.

Another way to fix this that I found is to make this no-op instead of fatal but I feel this is intended to detect misconfiguration https://github.com/spinnaker/clouddriver/blob/752bebc664da84b04918fbb15643e07c9cb0ff1a/clouddriver-aws/src/main/groovy/com/netflix/spinnaker/clouddriver/aws/deploy/ops/discovery/AbstractEnableDisableInstanceDiscoveryAtomicOperation.groovy#L52

[ajordens]

Still thinking about this but don't have a great answer.

In the past, we have bounced around ideas of allowing an application to declare it's health signals more than just "only platform" or not.

We do have an DetermineHealthProvidersTask class that can adjust interestingHealthProviderNames based on the provider.

It might be possible to support something that looks at what is supported by the account (rather than just the provider).