Skip to content
This repository has been archived by the owner on Mar 11, 2021. It is now read-only.

deletes pipelines on space removal #2352

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 2 additions & 118 deletions controller/deployments.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import (
"context"
"encoding/json"
"io"
"net/url"
"os"
"time"

"github.com/fabric8-services/fabric8-wit/app"
Expand All @@ -17,10 +15,10 @@ import (

"github.com/goadesign/goa"
errs "github.com/pkg/errors"
uuid "github.com/satori/go.uuid"
"github.com/satori/go.uuid"
"golang.org/x/net/websocket"
metaV1 "k8s.io/apimachinery/pkg/apis/meta/v1"
v1 "k8s.io/client-go/pkg/api/v1"
"k8s.io/client-go/pkg/api/v1"
"k8s.io/client-go/tools/cache"
)

Expand All @@ -31,17 +29,6 @@ type DeploymentsController struct {
ClientGetter
}

// ClientGetter creates an instances of clients used by this controller
type ClientGetter interface {
GetKubeClient(ctx context.Context) (kubernetes.KubeClientInterface, error)
GetAndCheckOSIOClient(ctx context.Context) (OpenshiftIOClient, error)
}

// Default implementation of KubeClientGetter and OSIOClientGetter used by NewDeploymentsController
type defaultClientGetter struct {
config *configuration.Registry
}

// NewDeploymentsController creates a deployments controller.
func NewDeploymentsController(service *goa.Service, config *configuration.Registry) *DeploymentsController {
return &DeploymentsController{
Expand All @@ -61,41 +48,6 @@ func tostring(item interface{}) string {
return string(bytes)
}

func (g *defaultClientGetter) GetAndCheckOSIOClient(ctx context.Context) (OpenshiftIOClient, error) {

// defaults
host := "localhost"
scheme := "https"

req := goa.ContextRequest(ctx)
if req != nil {
// Note - it's probably more efficient to force a loopback host, and only use the port number here
// (on some systems using a non-loopback interface forces a network stack traverse)
host = req.Host
scheme = req.URL.Scheme
}

// The deployments API communicates with the rest of WIT via the stnadard WIT API.
// This environment variable is used for local development of the deployments API, to point ot a remote WIT.
witURLStr := os.Getenv("FABRIC8_WIT_API_URL")
if witURLStr != "" {
witurl, err := url.Parse(witURLStr)
if err != nil {
log.Error(ctx, map[string]interface{}{
"FABRIC8_WIT_API_URL": witURLStr,
"err": err,
}, "cannot parse FABRIC8_WIT_API_URL: %s", witURLStr)
return nil, errs.Wrapf(err, "cannot parse FABRIC8_WIT_API_URL: %s", witURLStr)
}
host = witurl.Host
scheme = witurl.Scheme
}

oc := NewOSIOClient(ctx, scheme, host)

return oc, nil
}

// getSpaceNameFromSpaceID() converts an OSIO Space UUID to an OpenShift space name.
// will return an error if the space is not found.
func (c *DeploymentsController) getSpaceNameFromSpaceID(ctx context.Context, spaceID uuid.UUID) (*string, error) {
Expand All @@ -116,74 +68,6 @@ func (c *DeploymentsController) getSpaceNameFromSpaceID(ctx context.Context, spa
return osioSpace.Attributes.Name, nil
}

func (g *defaultClientGetter) getNamespaceName(ctx context.Context) (*string, error) {

osioclient, err := g.GetAndCheckOSIOClient(ctx)
if err != nil {
return nil, err
}

kubeSpaceAttr, err := osioclient.GetNamespaceByType(ctx, nil, "user")
if err != nil {
return nil, errs.Wrap(err, "unable to retrieve 'user' namespace")
}
if kubeSpaceAttr == nil {
return nil, errors.NewNotFoundError("namespace", "user")
}

return kubeSpaceAttr.Name, nil
}

// GetKubeClient creates a kube client for the appropriate cluster assigned to the current user
func (g *defaultClientGetter) GetKubeClient(ctx context.Context) (kubernetes.KubeClientInterface, error) {

kubeNamespaceName, err := g.getNamespaceName(ctx)
if err != nil {
log.Error(ctx, map[string]interface{}{
"err": err,
}, "could not retrieve namespace name")
return nil, errs.Wrap(err, "could not retrieve namespace name")
}

osioclient, err := g.GetAndCheckOSIOClient(ctx)
if err != nil {
log.Error(ctx, map[string]interface{}{
"err": err,
}, "could not create OSIO client")
return nil, err
}

baseURLProvider, err := NewURLProvider(ctx, g.config, osioclient)
if err != nil {
log.Error(ctx, map[string]interface{}{
"err": err,
}, "could not retrieve tenant data")
return nil, errs.Wrap(err, "could not retrieve tenant data")
}

/* Timeout used per HTTP request to Kubernetes/OpenShift API servers.
* Communication with Hawkular currently uses a hard-coded 30 second
* timeout per request, and does not use this parameter. */
// create the cluster API client
kubeConfig := &kubernetes.KubeClientConfig{
BaseURLProvider: baseURLProvider,
UserNamespace: *kubeNamespaceName,
Timeout: g.config.GetDeploymentsHTTPTimeoutSeconds(),
}
kc, err := kubernetes.NewKubeClient(kubeConfig)
if err != nil {
url, _ := baseURLProvider.GetAPIURL()
log.Error(ctx, map[string]interface{}{
"err": err,
"user_namespace": *kubeNamespaceName,
"cluster": *url,
}, "could not create Kubernetes client object")
return nil, errs.Wrap(err, "could not create Kubernetes client object")
}
return kc, nil
}

// SetDeployment runs the setDeployment action.
func (c *DeploymentsController) SetDeployment(ctx *app.SetDeploymentDeploymentsContext) error {

// we double check podcount here, because in the future we might have different query parameters
Expand Down
11 changes: 11 additions & 0 deletions controller/deployments_blackbox_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ type testOSIOClient struct {
controller.OpenshiftIOClient
}

type testOSClient struct {
fixture *deploymentsTestFixture
kubernetes.OpenShiftRESTAPI
}

func (kc *testKubeClient) Close() {
kc.closed = true
}
Expand Down Expand Up @@ -97,6 +102,12 @@ func (fixture *deploymentsTestFixture) GetAndCheckOSIOClient(ctx context.Context
}, nil
}

func (fixture *deploymentsTestFixture) GetOSClient(ctx context.Context) (kubernetes.OpenShiftRESTAPI, error) {
return &testOSClient{
fixture: fixture,
}, nil
}

func (c *testOSIOClient) GetSpaceByID(ctx context.Context, spaceID uuid.UUID) (*app.Space, error) {
var spaceName *string
uuidString := spaceID.String()
Expand Down
92 changes: 92 additions & 0 deletions controller/pipelines.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package controller

import (
"fmt"
"github.com/fabric8-services/fabric8-wit/app"
"github.com/fabric8-services/fabric8-wit/configuration"
"github.com/fabric8-services/fabric8-wit/errors"
"github.com/fabric8-services/fabric8-wit/jsonapi"
"github.com/fabric8-services/fabric8-wit/log"
"github.com/goadesign/goa"
errs "github.com/pkg/errors"
"github.com/satori/go.uuid"
)

// pipeline implements the pipeline resource.
type PipelinesController struct {
*goa.Controller
Config *configuration.Registry
ClientGetter
}

func NewPipelineController(service *goa.Service, config *configuration.Registry) *PipelinesController {
return &PipelinesController{
Controller: service.NewController("PipelinesController"),
Config: config,
ClientGetter: &defaultClientGetter{
config: config,
},
}
}

// Delete a pipelines from given space
func (c *PipelinesController) Delete(ctx *app.DeletePipelinesContext) error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plz add tests for Delete().


osioClient, err := c.GetAndCheckOSIOClient(ctx)
if err != nil {
return jsonapi.JSONErrorResponse(ctx, err)
}

k8sSpace, err := osioClient.GetNamespaceByType(ctx, nil, "user")
if err != nil {
return jsonapi.JSONErrorResponse(ctx, errs.Wrap(err, "unable to retrieve 'user' namespace"))
}
if k8sSpace == nil {
return jsonapi.JSONErrorResponse(ctx, errors.NewNotFoundError("namespace", "user"))
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be 500 rather 404 ??

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if resource not found then it should be 404 right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thinking is like .. user ask to delete_space .. if space not found then 404 is fine but here it's namespace and if that not found then returning 404 gives the impression that space_not_found. so to me namesapce_not_found it's internal_error when one is trying to deleting_space. This is feedback comment.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hrishin I am fine with either 404 or 500.


osc, err := c.GetOSClient(ctx)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For OSIO client, it's osioClient.. While for OS client, it's osc.. we can have consistency between these names here :-)

if err != nil {
return jsonapi.JSONErrorResponse(ctx, err)
}

userNS := *k8sSpace.Name
spacename, err := c.getSpaceNameFromSpaceID(ctx.SpaceID)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can reuse osioClient by passing it to getSpaceNameFromSpaceID().

if err != nil {
return jsonapi.JSONErrorResponse(ctx, err)
}

resp, err := osc.DeleteBuildConfig(userNS, map[string]string{"space": *spacename})
if err != nil {
log.Error(ctx, map[string]interface{}{
"err": err,
"space_name": *spacename,
}, "error occurred while deleting pipeline")
return jsonapi.JSONErrorResponse(ctx, err)
}

log.Info(ctx, map[string]interface{}{"response": resp}, "deleted pipelines :")

return ctx.OK([]byte{})
}

func (c *PipelinesController) getSpaceNameFromSpaceID(spaceID uuid.UUID) (*string, error) {
// use WIT API to convert Space UUID to Space name
osioclient, err := c.GetAndCheckOSIOClient(c.Context)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure c.Context will do, but it's always better to use request context (here it's DeletePipelinesContext)

if err != nil {
return nil, err
}

osioSpace, err := osioclient.GetSpaceByID(c.Context, spaceID)
fmt.Printf("spcae %#v", osioSpace)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

plz remove/replace fmt; check space text.


if err != nil {
return nil, errs.Wrapf(err, "unable to convert space UUID %s to space name", spaceID)
}

if osioSpace == nil || osioSpace.Attributes == nil || osioSpace.Attributes.Name == nil {
return nil, errs.Errorf("space UUID %s is not valid a space", spaceID)
}

return osioSpace.Attributes.Name, nil
}
Loading