apb pkg public interface overhaul (openshift#773)

* Initial commit moving things around in apb Introduces the Executor as well. * Remove individual jobs * Add consolidated jobs * Some minor modifications to apb func sigs * Update metrics names Binding -> Bind Unbinding -> Unbind * Use raw structs instead of New* variants The New funcs really are not offering much. * Mark non-bindable APBs finished * Update skipExection APBs with a completion desc * Fix lint errors * Protect statusChan closures with mutex * Use golang friendly getter names * Fix existing tests * Add initial jobs testing * Move to Executor interface for testing * Only set extractedCredentials with no err * Remove old apb run fns * Remove verbose status setting * Fix some of the job report logic We were previously sending a bunch of duplicate messages, now the jobs trust the status messages. This should fix ci issues. * Update executor hook names for more clarity * start -> actionStarted * finishWithSuccess -> actionFinishedWithSuccess * finishWithError -> actionFinishedWithError * Initial tests plus some fixes * Add mock executor and finish jobs_tests * Vendor updates Adds testify * Remove debug var
containerpope · Feb 21, 2018 · 38b79bf · 38b79bf
1 parent 4cb8b78
commit 38b79bf
Show file tree

Hide file tree

Showing 118 changed files with 20,303 additions and 1,472 deletions.
diff --git a/glide.lock b/glide.lock
diff --git a/glide.yaml b/glide.yaml
@@ -1,6 +1,5 @@
 package: github.com/openshift/ansible-service-broker
 import:
-# our dependancies
 - package: github.com/coreos/etcd
   version: ~3.2.9
   subpackages:
@@ -19,9 +18,6 @@ import:
 - package: gopkg.in/yaml.v1
 - package: github.com/pborman/uuid
   version: ^1.0.0
-  #
-  # Need to pin version for both protobuf because a dependancy asks for a lower
-  # version than another dependancy can use.
 - package: github.com/gogo/protobuf
   version: c0656edd0d9eab7c66d1eb0c568f9039345796f7
   subpackages:
@@ -30,11 +26,8 @@ import:
   version: 24f28ae800abfde9310e779f94be606b1a98a3fc
   subpackages:
   - proto
- # Need to pin version because of underlying dependancy mismatch.
 - package: google.golang.org/grpc
   version: dba60db4f499229023f3935df5f2e4ef7b881b00
-
-  # Kubernetes dependancies
 - package: k8s.io/kubernetes
   version: v1.9.1
 - package: k8s.io/client-go
@@ -45,3 +38,5 @@ import:
   version: kubernetes-1.9.1
 - package: github.com/openshift/api
   version: master
+- package: github.com/stretchr/testify
+  version: ^1.2.1
diff --git a/pkg/apb/bind.go b/pkg/apb/bind.go
@@ -20,14 +20,14 @@ import (
 	"fmt"
 
 	"github.com/openshift/ansible-service-broker/pkg/clients"
+	"github.com/openshift/ansible-service-broker/pkg/metrics"
 	"github.com/openshift/ansible-service-broker/pkg/runtime"
 )
 
 // Bind - Will run the APB with the bind action.
-func Bind(instance *ServiceInstance,
-	parameters *Parameters,
-	stateUpdates chan<- JobState) (string, *ExtractedCredentials, error) {
-
+func (e *executor) Bind(
+	instance *ServiceInstance, parameters *Parameters,
+) <-chan StatusMessage {
 	log.Notice("============================================================")
 	log.Notice("                       BINDING                              ")
 	log.Notice("============================================================")
@@ -37,41 +37,56 @@ func Bind(instance *ServiceInstance,
 	log.Notice(fmt.Sprintf("ServiceInstance.Description: %s", instance.Spec.Description))
 	log.Notice("============================================================")
 
-	executionContext, err := ExecuteApb("bind", instance.Spec, instance.Context, parameters)
-	defer runtime.Provider.DestroySandbox(
-		executionContext.PodName,
-		executionContext.Namespace,
-		executionContext.Targets,
-		clusterConfig.Namespace,
-		clusterConfig.KeepNamespace,
-		clusterConfig.KeepNamespaceOnError,
-	)
-	if err != nil {
-		log.Errorf("Problem executing apb [%s] bind", executionContext.PodName)
-		return executionContext.PodName, nil, err
-	}
-	k8scli, err := clients.Kubernetes()
-	if err != nil {
-		log.Error("Something went wrong getting kubernetes client")
-		return executionContext.PodName, nil, err
-	}
+	go func() {
+		e.actionStarted()
+		metrics.ActionStarted("bind")
+		executionContext, err := e.executeApb(
+			"bind", instance.Spec, instance.Context, parameters)
+		defer runtime.Provider.DestroySandbox(
+			executionContext.PodName,
+			executionContext.Namespace,
+			executionContext.Targets,
+			clusterConfig.Namespace,
+			clusterConfig.KeepNamespace,
+			clusterConfig.KeepNamespaceOnError,
+		)
+		if err != nil {
+			log.Errorf("Problem executing apb [%s] bind", executionContext.PodName)
+			e.actionFinishedWithError(err)
+			return
+		}
+		k8scli, err := clients.Kubernetes()
+		if err != nil {
+			log.Error("Something went wrong getting kubernetes client")
+			e.actionFinishedWithError(err)
+			return
+		}
+
+		if instance.Spec.Runtime >= 2 {
+			err := watchPod(executionContext.PodName, executionContext.Namespace,
+				k8scli.Client.CoreV1().Pods(executionContext.Namespace), e.updateDescription)
+			if err != nil {
+				log.Errorf("Bind action failed - %v", err)
+				e.actionFinishedWithError(err)
+				return
+			}
+		}
+
+		creds, err := ExtractCredentials(
+			executionContext.PodName,
+			executionContext.Namespace,
+			instance.Spec.Runtime,
+		)
 
-	if instance.Spec.Runtime >= 2 {
-		err := watchPod(executionContext.PodName, executionContext.Namespace, k8scli.Client.CoreV1().Pods(executionContext.Namespace), stateUpdates)
 		if err != nil {
-			log.Errorf("Bind action failed - %v", err)
-			return executionContext.PodName, nil, err
+			log.Errorf("apb::bind error occurred - %v", err)
+			e.actionFinishedWithError(err)
+			return
 		}
-	}
 
-	creds, err := ExtractCredentials(
-		executionContext.PodName,
-		executionContext.Namespace,
-		instance.Spec.Runtime,
-	)
-	if err != nil {
-		log.Errorf("apb::bind error occurred - %v", err)
-		return executionContext.PodName, creds, err
-	}
-	return executionContext.PodName, creds, err
+		e.extractedCredentials = creds
+		e.actionFinishedWithSuccess()
+	}()
+
+	return e.statusChan
 }
diff --git a/pkg/apb/deprovision.go b/pkg/apb/deprovision.go
@@ -24,7 +24,7 @@ import (
 )
 
 // Deprovision - runs the abp with the deprovision action.
-func Deprovision(instance *ServiceInstance, stateUpdates chan<- JobState) (string, error) {
+func (e *executor) Deprovision(instance *ServiceInstance) <-chan StatusMessage {
 	log.Notice("============================================================")
 	log.Notice("                      DEPROVISIONING                        ")
 	log.Notice("============================================================")
@@ -34,42 +34,49 @@ func Deprovision(instance *ServiceInstance, stateUpdates chan<- JobState) (strin
 	log.Noticef("ServiceInstance.Description: %s", instance.Spec.Description)
 	log.Notice("============================================================")
 
-	// Explicitly error out if image field is missing from instance.Spec
-	// was introduced as a change to the apb instance.Spec to support integration
-	// with the broker and still allow for providing an img path
-	// Legacy ansibleapps will hit this.
-	if instance.Spec.Image == "" {
-		log.Error("No image field found on the apb instance.Spec (apb.yaml)")
-		log.Error("apb instance.Spec requires [name] and [image] fields to be separate")
-		log.Error("Are you trying to run a legacy ansibleapp without an image field?")
-		return "", errors.New("No image field found on instance.Spec")
-	}
+	go func() {
+		e.actionStarted()
+		if instance.Spec.Image == "" {
+			log.Error("No image field found on the apb instance.Spec (apb.yaml)")
+			log.Error("apb instance.Spec requires [name] and [image] fields to be separate")
+			log.Error("Are you trying to run a legacy ansibleapp without an image field?")
+			e.actionFinishedWithError(errors.New("No image field found on instance.Spec"))
+			return
+		}
 
-	// Might need to change up this interface to feed in instance ids
-	metrics.ActionStarted("deprovision")
-	executionContext, err := ExecuteApb("deprovision", instance.Spec, instance.Context, instance.Parameters)
-	defer runtime.Provider.DestroySandbox(
-		executionContext.PodName,
-		executionContext.Namespace,
-		executionContext.Targets,
-		clusterConfig.Namespace,
-		clusterConfig.KeepNamespace,
-		clusterConfig.KeepNamespaceOnError,
-	)
-	if err != nil {
-		log.Errorf("Problem executing apb [%s] deprovision", executionContext.PodName)
-		return executionContext.PodName, err
-	}
-	k8scli, err := clients.Kubernetes()
-	if err != nil {
-		log.Error("Something went wrong getting kubernetes client")
-		return executionContext.PodName, err
-	}
-	err = watchPod(executionContext.PodName, executionContext.Namespace, k8scli.Client.CoreV1().Pods(executionContext.Namespace), stateUpdates)
-	if err != nil {
-		log.Errorf("Deprovision action failed - %v", err)
-		return executionContext.PodName, err
-	}
+		// Might need to change up this interface to feed in instance ids
+		metrics.ActionStarted("deprovision")
+		executionContext, err := e.executeApb("deprovision", instance.Spec,
+			instance.Context, instance.Parameters)
+		defer runtime.Provider.DestroySandbox(
+			executionContext.PodName,
+			executionContext.Namespace,
+			executionContext.Targets,
+			clusterConfig.Namespace,
+			clusterConfig.KeepNamespace,
+			clusterConfig.KeepNamespaceOnError,
+		)
+		if err != nil {
+			log.Errorf("Problem executing apb [%s] deprovision", executionContext.PodName)
+			e.actionFinishedWithError(err)
+			return
+		}
+		k8scli, err := clients.Kubernetes()
+		if err != nil {
+			log.Error("Something went wrong getting kubernetes client")
+			e.actionFinishedWithError(err)
+			return
+		}
+		err = watchPod(executionContext.PodName, executionContext.Namespace,
+			k8scli.Client.CoreV1().Pods(executionContext.Namespace), e.updateDescription)
+		if err != nil {
+			log.Errorf("Deprovision action failed - %v", err)
+			e.actionFinishedWithError(err)
+			return
+		}
 
-	return executionContext.PodName, err
+		e.actionFinishedWithSuccess()
+	}()
+
+	return e.statusChan
 }