feat(cloudfoundry): bind services before building droplet (#5152)

clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/HttpCloudFoundryClient.java

@@ -224,7 +224,6 @@ public HttpCloudFoundryClient(
         new ServiceInstances(
             createService(ServiceInstanceService.class),
             createService(ConfigService.class),
-            organizations,
             spaces);
     this.routes =
         new Routes(

clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/ServiceInstances.java

@@ -23,6 +23,7 @@
 import static com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v2.LastOperation.Type.*;
 import static com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v2.ServiceInstance.Type.MANAGED_SERVICE_INSTANCE;
 import static com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v2.ServiceInstance.Type.USER_PROVIDED_SERVICE_INSTANCE;
+import static java.util.Collections.emptyList;
 import static java.util.Collections.singletonList;
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toSet;
@@ -50,34 +51,21 @@
 public class ServiceInstances {
   private final ServiceInstanceService api;
   private final ConfigService configApi;
-  private final Organizations orgs;
   private final Spaces spaces;
 
-  public Void createServiceBindingsByName(
-      CloudFoundryServerGroup cloudFoundryServerGroup,
-      @Nullable List<String> serviceInstanceNames) {
-    if (serviceInstanceNames != null && !serviceInstanceNames.isEmpty()) {
-      List<String> serviceInstanceQuery =
-          getServiceQueryParams(serviceInstanceNames, cloudFoundryServerGroup.getSpace());
-      List<Resource<? extends AbstractServiceInstance>> serviceInstances = new ArrayList<>();
-      serviceInstances.addAll(
-          collectPageResources("service instances", pg -> api.all(pg, serviceInstanceQuery)));
-      serviceInstances.addAll(
-          collectPageResources(
-              "service instances", pg -> api.allUserProvided(pg, serviceInstanceQuery)));
-
-      if (serviceInstances.size() != serviceInstanceNames.size()) {
-        throw new CloudFoundryApiException(
-            "Number of service instances does not match the number of service names");
-      }
-
-      for (Resource<? extends AbstractServiceInstance> serviceInstance : serviceInstances) {
-        api.createServiceBinding(
-            new CreateServiceBinding(
-                serviceInstance.getMetadata().getGuid(), cloudFoundryServerGroup.getId()));
+  public void createServiceBinding(CreateServiceBinding createServiceBinding) {
+    try {
+      safelyCall(() -> api.createServiceBinding(createServiceBinding)).get();
+    } catch (CloudFoundryApiException e) {
+      if (e.getErrorCode() == null) throw e;
+
+      switch (e.getErrorCode()) {
+        case SERVICE_INSTANCE_ALREADY_BOUND:
+          return;
+        default:
+          throw e;
       }
     }
-    return null;
   }
 
   private Resource<Service> findServiceByServiceName(String serviceName) {
@@ -128,6 +116,19 @@ public List<CloudFoundryService> findAllServicesByRegion(String region) {
         .orElse(Collections.emptyList());
   }
 
+  public List<Resource<? extends AbstractServiceInstance>> findAllServicesBySpaceAndNames(
+      CloudFoundrySpace space, List<String> serviceInstanceNames) {
+    if (serviceInstanceNames == null || serviceInstanceNames.isEmpty()) return emptyList();
+    List<String> serviceInstanceQuery = getServiceQueryParams(serviceInstanceNames, space);
+    List<Resource<? extends AbstractServiceInstance>> serviceInstances = new ArrayList<>();
+    serviceInstances.addAll(
+        collectPageResources("service instances", pg -> api.all(pg, serviceInstanceQuery)));
+    serviceInstances.addAll(
+        collectPageResources(
+            "service instances", pg -> api.allUserProvided(pg, serviceInstanceQuery)));
+    return serviceInstances;
+  }
+
   // Visible for testing
   CloudFoundryServiceInstance getOsbServiceInstanceByRegion(
       String region, String serviceInstanceName) {

clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/api/ServiceInstanceService.java

@@ -31,7 +31,7 @@ Page<UserProvidedServiceInstance> allUserProvided(
       @Query("page") Integer page, @Query("q") List<String> queryParam);
 
   @POST("/v2/service_bindings?accepts_incomplete=true")
-  Response createServiceBinding(@Body CreateServiceBinding body);
+  Resource<ServiceBinding> createServiceBinding(@Body CreateServiceBinding body);
 
   @GET("/v2/services")
   Page<Service> findService(@Query("page") Integer page, @Query("q") List<String> queryParams);

clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/model/ErrorDescription.java

@@ -96,7 +96,8 @@ public enum Code {
     ROUTE_PATH_TAKEN("CF-RoutePathTaken"),
     ROUTE_PORT_TAKEN("CF-RoutePortTaken"),
     RESOURCE_NOT_FOUND("CF-ResourceNotFound"),
-    SERVICE_ALREADY_EXISTS("60002");
+    SERVICE_ALREADY_EXISTS("60002"),
+    SERVICE_INSTANCE_ALREADY_BOUND("CF-ServiceBindingAppServiceTaken");
 
     private final String code;
 

clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/model/v2/CreateServiceBinding.java

@@ -16,12 +16,16 @@
 
 package com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v2;
 
+import java.util.Map;
+import lombok.AllArgsConstructor;
 import lombok.Getter;
 import lombok.RequiredArgsConstructor;
 
 @RequiredArgsConstructor
+@AllArgsConstructor
 @Getter
 public class CreateServiceBinding {
   private final String serviceInstanceGuid;
   private final String appGuid;
+  private Map<String, Object> parameters;
 }

clouddriver-cloudfoundry/src/main/java/com/netflix/spinnaker/clouddriver/cloudfoundry/deploy/ops/DeployCloudFoundryServerGroupAtomicOperation.java

@@ -27,6 +27,7 @@
 import com.netflix.spinnaker.clouddriver.cloudfoundry.artifacts.CloudFoundryArtifactCredentials;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.client.CloudFoundryApiException;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.client.CloudFoundryClient;
+import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v2.CreateServiceBinding;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v3.ProcessStats;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.deploy.CloudFoundryServerGroupNameResolver;
 import com.netflix.spinnaker.clouddriver.cloudfoundry.deploy.description.DeployCloudFoundryServerGroupDescription;
@@ -92,18 +93,15 @@ public DeploymentResult operate(List priorOutputs) {
       }
     }
 
+    createServiceBindings(serverGroup, description);
+
     buildDroplet(packageId, serverGroup.getId(), description);
     scaleApplication(serverGroup.getId(), description);
     if (description.getApplicationAttributes().getHealthCheckType() != null
         || description.getApplicationAttributes().getCommand() != null) {
       updateProcess(serverGroup.getId(), description);
     }
 
-    client
-        .getServiceInstances()
-        .createServiceBindingsByName(
-            serverGroup, description.getApplicationAttributes().getServices());
-
     if (!mapRoutes(
         description,
         description.getApplicationAttributes().getRoutes(),
@@ -143,6 +141,53 @@ public DeploymentResult operate(List priorOutputs) {
     return deploymentResult();
   }
 
+  private void createServiceBindings(
+      CloudFoundryServerGroup serverGroup, DeployCloudFoundryServerGroupDescription description) {
+    List<String> serviceNames = description.getApplicationAttributes().getServices();
+    if (serviceNames == null || serviceNames.isEmpty()) return;
+    getTask()
+        .updateStatus(
+            PHASE,
+            "Creating Cloud Foundry service bindings between application '"
+                + description.getServerGroupName()
+                + "' and services: "
+                + description.getApplicationAttributes().getServices());
+
+    List<CreateServiceBinding> bindings =
+        description
+            .getClient()
+            .getServiceInstances()
+            .findAllServicesBySpaceAndNames(
+                serverGroup.getSpace(), description.getApplicationAttributes().getServices())
+            .stream()
+            .map(
+                s ->
+                    new CreateServiceBinding(
+                        s.getMetadata().getGuid(), serverGroup.getId(), Collections.emptyMap()))
+            .collect(toList());
+
+    if (bindings.size() != description.getApplicationAttributes().getServices().size()) {
+      getTask()
+          .updateStatus(
+              PHASE,
+              "Failed to create Cloud Foundry service bindings between application '"
+                  + description.getServerGroupName()
+                  + "' and services: "
+                  + description.getApplicationAttributes().getServices());
+      throw new CloudFoundryApiException(
+          "Number of service instances does not match the number of service names");
+    }
+
+    bindings.forEach(b -> description.getClient().getServiceInstances().createServiceBinding(b));
+    getTask()
+        .updateStatus(
+            PHASE,
+            "Created Cloud Foundry service bindings between application '"
+                + description.getServerGroupName()
+                + "' and services: "
+                + description.getApplicationAttributes().getServices());
+  }
+
   private DeploymentResult deploymentResult() {
     DeploymentResult deploymentResult = new DeploymentResult();
     deploymentResult.setServerGroupNames(

clouddriver-cloudfoundry/src/test/java/com/netflix/spinnaker/clouddriver/cloudfoundry/client/ServiceInstancesTest.java

@@ -23,6 +23,7 @@
 import static com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v2.ServiceInstance.Type.MANAGED_SERVICE_INSTANCE;
 import static com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.v2.ServiceInstance.Type.USER_PROVIDED_SERVICE_INSTANCE;
 import static com.netflix.spinnaker.clouddriver.cloudfoundry.utils.TestUtils.assertThrows;
+import static java.util.Collections.emptyMap;
 import static java.util.Collections.singleton;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.*;
@@ -57,7 +58,7 @@ class ServiceInstancesTest {
   private Organizations orgs = mock(Organizations.class);
   private Spaces spaces = mock(Spaces.class);
   private ServiceInstances serviceInstances =
-      new ServiceInstances(serviceInstanceService, configService, orgs, spaces);
+      new ServiceInstances(serviceInstanceService, configService, spaces);
 
   {
     when(serviceInstanceService.findService(any(), any()))
@@ -67,13 +68,6 @@ class ServiceInstancesTest {
         .thenReturn(Page.singleton(new ServicePlan().setName("ServicePlan1"), "plan-guid"));
   }
 
-  @Test
-  void shouldNotMakeAPICallWhenNoServiceNamesAreProvided() {
-    CloudFoundryServerGroup cloudFoundryServerGroup = CloudFoundryServerGroup.builder().build();
-    serviceInstances.createServiceBindingsByName(cloudFoundryServerGroup, Collections.emptyList());
-    verify(serviceInstanceService, never()).all(any(), any());
-  }
-
   @Test
   void shouldCreateServiceBindingWhenServiceExists() {
     CloudFoundryServerGroup cloudFoundryServerGroup =
@@ -84,6 +78,9 @@ void shouldCreateServiceBindingWhenServiceExists() {
             .build();
 
     Page<ServiceInstance> serviceMappingPageOne = Page.singleton(null, "service-instance-guid");
+    CreateServiceBinding binding =
+        new CreateServiceBinding(
+            "service-instance-guid", cloudFoundryServerGroup.getId(), emptyMap());
     serviceMappingPageOne.setTotalResults(0);
     serviceMappingPageOne.setTotalPages(0);
     when(serviceInstanceService.all(eq(null), any())).thenReturn(serviceMappingPageOne);
@@ -95,10 +92,10 @@ void shouldCreateServiceBindingWhenServiceExists() {
         .thenReturn(userProvidedServiceMappingPageOne);
     when(serviceInstanceService.allUserProvided(eq(1), any()))
         .thenReturn(userProvidedServiceMappingPageOne);
+    when(serviceInstanceService.createServiceBinding(binding))
+        .thenReturn(createServiceBindingResource());
 
-    serviceInstances.createServiceBindingsByName(
-        cloudFoundryServerGroup, Collections.singletonList("service-instance"));
-
+    serviceInstances.createServiceBinding(binding);
     verify(serviceInstanceService, atLeastOnce()).createServiceBinding(any());
   }
 
@@ -112,6 +109,9 @@ void shouldCreateServiceBindingWhenUserProvidedServiceExists() {
             .build();
 
     Page<ServiceInstance> serviceMappingPageOne = createEmptyOsbServiceInstancePage();
+    CreateServiceBinding binding =
+        new CreateServiceBinding(
+            "service-instance-guid", cloudFoundryServerGroup.getId(), emptyMap());
     when(serviceInstanceService.all(eq(null), any())).thenReturn(serviceMappingPageOne);
     when(serviceInstanceService.all(eq(1), any())).thenReturn(serviceMappingPageOne);
 
@@ -123,36 +123,46 @@ void shouldCreateServiceBindingWhenUserProvidedServiceExists() {
         .thenReturn(userProvidedServiceMappingPageOne);
     when(serviceInstanceService.allUserProvided(eq(1), any()))
         .thenReturn(userProvidedServiceMappingPageOne);
+    when(serviceInstanceService.createServiceBinding(binding))
+        .thenReturn(createServiceBindingResource());
 
-    serviceInstances.createServiceBindingsByName(
-        cloudFoundryServerGroup, Collections.singletonList("service-instance"));
+    serviceInstances.createServiceBinding(binding);
 
     verify(serviceInstanceService, atLeastOnce()).createServiceBinding(any());
   }
 
   @Test
-  void shouldThrowAnErrorIfServiceNotFound() {
+  void shouldSucceedServiceBindingWhenServiceBindingExists() {
+    RetrofitError retrofitError = mock(RetrofitError.class);
+
     CloudFoundryServerGroup cloudFoundryServerGroup =
         CloudFoundryServerGroup.builder()
             .account("some-account")
             .id("servergroup-id")
             .space(cloudFoundrySpace)
             .build();
 
-    when(serviceInstanceService.all(any(), any())).thenReturn(createEmptyOsbServiceInstancePage());
+    Page<ServiceInstance> serviceMappingPageOne = Page.singleton(null, "service-instance-guid");
+    CreateServiceBinding binding =
+        new CreateServiceBinding(
+            "service-instance-guid", cloudFoundryServerGroup.getId(), emptyMap());
+    serviceMappingPageOne.setTotalResults(0);
+    serviceMappingPageOne.setTotalPages(0);
+    when(serviceInstanceService.all(eq(null), any())).thenReturn(serviceMappingPageOne);
+    when(serviceInstanceService.all(eq(1), any())).thenReturn(serviceMappingPageOne);
 
     Page<UserProvidedServiceInstance> userProvidedServiceMappingPageOne =
         createEmptyUserProvidedServiceInstancePage();
     when(serviceInstanceService.allUserProvided(eq(null), any()))
         .thenReturn(userProvidedServiceMappingPageOne);
     when(serviceInstanceService.allUserProvided(eq(1), any()))
         .thenReturn(userProvidedServiceMappingPageOne);
-    assertThrows(
-        () ->
-            serviceInstances.createServiceBindingsByName(
-                cloudFoundryServerGroup, Collections.singletonList("service-instance")),
-        CloudFoundryApiException.class,
-        "Cloud Foundry API returned with error(s): Number of service instances does not match the number of service names");
+    when(serviceInstanceService.createServiceBinding(binding)).thenThrow(retrofitError);
+    when(retrofitError.getBodyAs(ErrorDescription.class))
+        .thenReturn(createServiceAlreadyBoundErrorDescription());
+
+    serviceInstances.createServiceBinding(binding);
+    verify(serviceInstanceService, atLeastOnce()).createServiceBinding(any());
   }
 
   @Test
@@ -1190,6 +1200,25 @@ private Resource<ServiceInstance> createServiceInstanceResource() {
     return serviceInstanceResource;
   }
 
+  private Resource<ServiceBinding> createServiceBindingResource() {
+    ServiceBinding serviceBinding = new ServiceBinding();
+    serviceBinding.setAppGuid("servergroup-id");
+    serviceBinding.setName("");
+    serviceBinding.setServiceInstanceGuid("service-instance-guid");
+    Resource<ServiceBinding> serviceBindingResource = new Resource<>();
+    serviceBindingResource.setEntity(serviceBinding);
+    serviceBindingResource.setMetadata(new Resource.Metadata().setGuid("service-binding-guid"));
+    return serviceBindingResource;
+  }
+
+  private ErrorDescription createServiceAlreadyBoundErrorDescription() {
+    ErrorDescription errorDescription = new ErrorDescription();
+    errorDescription.setCode(99999);
+    errorDescription.setErrorCode(ErrorDescription.Code.SERVICE_INSTANCE_ALREADY_BOUND);
+    errorDescription.setDescription("already bound");
+    return errorDescription;
+  }
+
   private Resource<UserProvidedServiceInstance> createUserProvidedServiceInstanceResource() {
     UserProvidedServiceInstance userProvidedServiceInstance = new UserProvidedServiceInstance();
     userProvidedServiceInstance.setName("new-service-instance-name");