Skip to content

Commit

Permalink
Fail if service bindings are not injected
Browse files Browse the repository at this point in the history
  • Loading branch information
feloy committed Jul 13, 2022
1 parent 57ef5b4 commit 577fa4f
Show file tree
Hide file tree
Showing 8 changed files with 99 additions and 5 deletions.
55 changes: 55 additions & 0 deletions pkg/binding/binding.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@ import (
"github.com/devfile/library/pkg/devfile/parser"
devfilefs "github.com/devfile/library/pkg/testingutil/filesystem"
"gopkg.in/yaml.v2"
appsv1 "k8s.io/api/apps/v1"
kerrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"

devfilev1alpha2 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
parsercommon "github.com/devfile/library/pkg/devfile/parser/data/v2/common"
Expand Down Expand Up @@ -284,3 +286,56 @@ func (o *BindingClient) getStatusFromSpec(name string) (*api.ServiceBindingStatu
BindingEnvVars: bindingEnvVars,
}, nil
}

func (o *BindingClient) CheckServiceBindingsInjectionDone(componentName string, appName string) (bool, error) {

deployment, err := o.kubernetesClient.GetOneDeployment(componentName, appName)
if err != nil {
// If not deployment yet => all bindings are done
if _, ok := err.(*kclient.DeploymentNotFoundError); ok {
return true, nil
}
return false, err
}
deploymentName := deployment.GetName()

specList, bindingList, err := o.kubernetesClient.ListServiceBindingsFromAllGroups()
if err != nil {
// If ServiceBinding kind is not registered => all bindings are done
if runtime.IsNotRegisteredError(err) {
return true, nil
}
return false, err
}

for _, binding := range bindingList {
app := binding.Spec.Application
if app.Group != appsv1.SchemeGroupVersion.Group ||
app.Version != appsv1.SchemeGroupVersion.Version ||
(app.Kind != "Deployment" && app.Resource != "deployments") {
continue
}
if app.Name != deploymentName {
continue
}
if injected := meta.IsStatusConditionTrue(binding.Status.Conditions, bindingApis.InjectionReady); !injected {
return false, nil
}
}

for _, binding := range specList {
app := binding.Spec.Workload
if app.APIVersion != appsv1.SchemeGroupVersion.String() ||
app.Kind != "Deployment" {
continue
}
if app.Name != deploymentName {
continue
}
if injected := meta.IsStatusConditionTrue(binding.Status.Conditions, bindingApis.InjectionReady); !injected {
return false, nil
}
}

return true, nil
}
3 changes: 3 additions & 0 deletions pkg/binding/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,7 @@ type Client interface {
ValidateRemoveBinding(flags map[string]string) error
// RemoveBinding removes the binding from devfile
RemoveBinding(bindingName string, obj parser.DevfileObj) (parser.DevfileObj, error)

// CheckServiceBindingsInjectionDone checks that all service bindings poiting to component have InjectionReady condition
CheckServiceBindingsInjectionDone(componentName string, appName string) (bool, error)
}
15 changes: 15 additions & 0 deletions pkg/binding/mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion pkg/dev/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"io"

"github.com/redhat-developer/odo/pkg/binding"
"github.com/redhat-developer/odo/pkg/envinfo"
"github.com/redhat-developer/odo/pkg/kclient"
"github.com/redhat-developer/odo/pkg/portForward"
Expand All @@ -22,6 +23,7 @@ type DevClient struct {
prefClient preference.Client
portForwardClient portForward.Client
watchClient watch.Client
bindingClient binding.Client
}

var _ Client = (*DevClient)(nil)
Expand All @@ -31,12 +33,14 @@ func NewDevClient(
prefClient preference.Client,
portForwardClient portForward.Client,
watchClient watch.Client,
bindingClient binding.Client,
) *DevClient {
return &DevClient{
kubernetesClient: kubernetesClient,
prefClient: prefClient,
portForwardClient: portForwardClient,
watchClient: watchClient,
bindingClient: bindingClient,
}
}

Expand All @@ -53,7 +57,7 @@ func (o *DevClient) Start(
) (watch.ComponentStatus, error) {
klog.V(4).Infoln("Creating new adapter")
adapter := component.NewKubernetesAdapter(
o.kubernetesClient, o.prefClient, o.portForwardClient,
o.kubernetesClient, o.prefClient, o.portForwardClient, o.bindingClient,
component.AdapterContext{
ComponentName: devfileObj.GetMetadataName(),
Context: path,
Expand Down
15 changes: 15 additions & 0 deletions pkg/devfile/adapters/kubernetes/component/adapter.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package component

import (
"errors"
"fmt"
"io"
"path/filepath"
Expand All @@ -9,6 +10,7 @@ import (

"k8s.io/utils/pointer"

"github.com/redhat-developer/odo/pkg/binding"
"github.com/redhat-developer/odo/pkg/component"
"github.com/redhat-developer/odo/pkg/devfile/adapters"
"github.com/redhat-developer/odo/pkg/devfile/adapters/kubernetes/storage"
Expand Down Expand Up @@ -44,6 +46,7 @@ type Adapter struct {
kubeClient kclient.ClientInterface
prefClient preference.Client
portForwardClient portForward.Client
bindingClient binding.Client

AdapterContext
logger machineoutput.MachineEventLoggingClient
Expand All @@ -65,6 +68,7 @@ func NewKubernetesAdapter(
kubernetesClient kclient.ClientInterface,
prefClient preference.Client,
portForwardClient portForward.Client,
bindingClient binding.Client,
context AdapterContext,
namespace string,
) Adapter {
Expand All @@ -77,6 +81,7 @@ func NewKubernetesAdapter(
kubeClient: kubernetesClient,
prefClient: prefClient,
portForwardClient: portForwardClient,
bindingClient: bindingClient,
AdapterContext: context,
logger: machineoutput.NewMachineEventLoggingClient(),
}
Expand Down Expand Up @@ -158,6 +163,16 @@ func (a Adapter) Push(parameters adapters.PushParameters, componentStatus *watch
return nil
}

injected, err := a.bindingClient.CheckServiceBindingsInjectionDone(a.ComponentName, a.AppName)
if err != nil {
return err
}

if !injected {
klog.V(4).Infof("Waiting for all service bindings to be injected...\n")
return errors.New("some servicebindings are not injected")
}

// Check if endpoints changed in Devfile
portsToForward, err := a.portForwardClient.GetPortsToForward(a.Devfile)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions pkg/devfile/adapters/kubernetes/component/adapter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func TestCreateOrUpdateComponent(t *testing.T) {
ctrl := gomock.NewController(t)
fakePrefClient := preference.NewMockClient(ctrl)
fakePrefClient.EXPECT().GetEphemeralSourceVolume()
componentAdapter := NewKubernetesAdapter(fkclient, fakePrefClient, nil, adapterCtx, "")
componentAdapter := NewKubernetesAdapter(fkclient, fakePrefClient, nil, nil, adapterCtx, "")
_, _, err := componentAdapter.createOrUpdateComponent(tt.running, tt.envInfo, libdevfile.DevfileCommands{}, 0, nil)

// Checks for unexpected error cases
Expand Down Expand Up @@ -352,7 +352,7 @@ func TestDoesComponentExist(t *testing.T) {
ctrl := gomock.NewController(t)
fakePrefClient := preference.NewMockClient(ctrl)
fakePrefClient.EXPECT().GetEphemeralSourceVolume()
componentAdapter := NewKubernetesAdapter(fkclient, fakePrefClient, nil, adapterCtx, "")
componentAdapter := NewKubernetesAdapter(fkclient, fakePrefClient, nil, nil, adapterCtx, "")
_, _, err := componentAdapter.createOrUpdateComponent(false, tt.envInfo, libdevfile.DevfileCommands{}, 0, nil)

// Checks for unexpected error cases
Expand Down
2 changes: 2 additions & 0 deletions pkg/odo/cli/dev/dev.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ func (o *Handler) regenerateComponentAdapterFromWatchParams(parameters watch.Wat
o.clientset.KubernetesClient,
o.clientset.PreferenceClient,
o.clientset.PortForwardClient,
o.clientset.BindingClient,
kcomponent.AdapterContext{
ComponentName: parameters.ComponentName,
Context: parameters.Path,
Expand Down Expand Up @@ -341,6 +342,7 @@ It forwards endpoints with exposure values 'public' or 'internal' to a port on l
devCmd.Flags().StringVar(&o.runCommandFlag, "run-command", "",
"Alternative run command to execute. The default one will be used if this flag is not set.")
clientset.Add(devCmd,
clientset.BINDING,
clientset.DEV,
clientset.FILESYSTEM,
clientset.INIT,
Expand Down
4 changes: 2 additions & 2 deletions pkg/odo/genericclioptions/clientset/clientset.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ var subdeps map[string][]string = map[string][]string{
ALIZER: {REGISTRY},
DELETE_COMPONENT: {KUBERNETES},
DEPLOY: {KUBERNETES},
DEV: {KUBERNETES, PORT_FORWARD, PREFERENCE, WATCH},
DEV: {BINDING, KUBERNETES, PORT_FORWARD, PREFERENCE, WATCH},
INIT: {ALIZER, FILESYSTEM, PREFERENCE, REGISTRY},
LOGS: {KUBERNETES},
PORT_FORWARD: {KUBERNETES, STATE},
Expand Down Expand Up @@ -180,7 +180,7 @@ func Fetch(command *cobra.Command) (*Clientset, error) {
dep.PortForwardClient = portForward.NewPFClient(dep.KubernetesClient, dep.StateClient)
}
if isDefined(command, DEV) {
dep.DevClient = dev.NewDevClient(dep.KubernetesClient, dep.PreferenceClient, dep.PortForwardClient, dep.WatchClient)
dep.DevClient = dev.NewDevClient(dep.KubernetesClient, dep.PreferenceClient, dep.PortForwardClient, dep.WatchClient, dep.BindingClient)
}

/* Instantiate new clients here. Take care to instantiate after all sub-dependencies */
Expand Down

0 comments on commit 577fa4f

Please sign in to comment.