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

Minor refactor of http_trigger. Add unit tests #651

Merged
merged 3 commits into from
Mar 27, 2018
Merged
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
1 change: 1 addition & 0 deletions cmd/kubeless/function/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ var deployCmd = &cobra.Command{
"function": funcName,
}
httpTrigger.Spec.FunctionName = funcName
httpTrigger.Spec.RouteName = funcName

err = utils.CreateHTTPTriggerCustomResource(kubelessClient, &httpTrigger)
if err != nil {
Expand Down
1 change: 0 additions & 1 deletion cmd/kubeless/route/routeCreate.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ var routeCreateCmd = &cobra.Command{
httpTrigger.Spec.HostName = hostName
httpTrigger.Spec.TLSAcme = enableTLSAcme
httpTrigger.Spec.RouteName = ingressName
httpTrigger.Spec.EnableIngress = true
httpTrigger.Spec.Path = "/"

err = utils.UpdateHTTPTriggerCustomResource(kubelessClient, httpTrigger)
Expand Down
1 change: 0 additions & 1 deletion cmd/kubeless/route/routeDelete.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ var routeDeleteCmd = &cobra.Command{
}

httpTrigger.Spec.RouteName = routeName
httpTrigger.Spec.EnableIngress = false
err = utils.UpdateHTTPTriggerCustomResource(kubelessClient, httpTrigger)
if err != nil {
logrus.Fatalf("Can't create route: %v", err)
Expand Down
11 changes: 5 additions & 6 deletions pkg/apis/kubeless/v1beta1/http_trigger.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,11 @@ type HTTPTrigger struct {

// HTTPTriggerSpec contains func specification
type HTTPTriggerSpec struct {
FunctionName string `json:"function-name"` // Name of the associated function
HostName string `json:"host-name"`
TLSAcme bool `json:"tls"`
RouteName string `json:"route-name"`
EnableIngress bool `json:"ingress-enabled"`
Path string `json:"path"`
FunctionName string `json:"function-name"` // Name of the associated function
HostName string `json:"host-name"`
TLSAcme bool `json:"tls"`
RouteName string `json:"route-name"`
Path string `json:"path"`
}

// +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
Expand Down
59 changes: 35 additions & 24 deletions pkg/controller/http_trigger_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ import (
"github.com/kubeless/kubeless/pkg/client/informers/externalversions"
kubelessInformers "github.com/kubeless/kubeless/pkg/client/informers/externalversions/kubeless/v1beta1"
"github.com/sirupsen/logrus"
apiequality "k8s.io/apimachinery/pkg/api/equality"
k8sErrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/kubernetes"
Expand Down Expand Up @@ -205,9 +208,7 @@ func (c *HTTPTriggerController) syncHTTPTrigger(key string) error {
}

// remove ingress resource if any. Ignore any error, as ingress resource will be GC'ed
if httpTriggerObj.Spec.EnableIngress {
_ = utils.DeleteIngress(c.clientset, httpTriggerObj.Spec.RouteName, httpTriggerObj.Namespace)
}
_ = utils.DeleteIngress(c.clientset, httpTriggerObj.Spec.RouteName, httpTriggerObj.Namespace)

err = c.httpTriggerObjRemoveFinalizer(httpTriggerObj)
if err != nil {
Expand All @@ -228,44 +229,54 @@ func (c *HTTPTriggerController) syncHTTPTrigger(key string) error {
}

// create ingress resource if required
if httpTriggerObj.Spec.EnableIngress {
c.logger.Infof("Adding ingress resource for http trigger Obj: %s ", key)
err = utils.CreateIngress(c.clientset, httpTriggerObj)
if err != nil {
c.logger.Errorf("Failed to create ingress rule %s corresponding to http trigger Obj: %s due to: %v: ", httpTriggerObj.Spec.RouteName, key, err)
}
c.logger.Infof("Adding ingress resource for http trigger Obj: %s ", key)
err = utils.CreateIngress(c.clientset, httpTriggerObj)
if err != nil && !k8sErrors.IsAlreadyExists(err) {
c.logger.Errorf("Failed to create ingress rule %s corresponding to http trigger Obj: %s due to: %v: ", httpTriggerObj.Spec.RouteName, key, err)
}

// delete ingress resource if not required
if !httpTriggerObj.Spec.EnableIngress {
if httpTriggerObj.Spec.RouteName != "" {
c.logger.Infof("Deleting ingress resource for http trigger Obj: %s ", key)
err = utils.DeleteIngress(c.clientset, httpTriggerObj.Spec.RouteName, httpTriggerObj.Namespace)
if err != nil {
c.logger.Errorf("Failed to remove ingress rule %s corresponding to http trigger Obj: %s due to: %v: ", httpTriggerObj.Spec.RouteName, key, err)
}
}
}
c.logger.Infof("Processed update to HTTPTrigger: %s", key)
return nil
}

// FunctionAddedDeletedUpdated process the updates to Function objects
func (c *HTTPTriggerController) functionAddedDeletedUpdated(obj interface{}, deleted bool) {
func (c *HTTPTriggerController) functionAddedDeletedUpdated(obj interface{}, deleted bool) error {
functionObj, ok := obj.(*kubelessApi.Function)
if !ok {
tombstone, ok := obj.(cache.DeletedFinalStateUnknown)
if !ok {
c.logger.Errorf("Couldn't get object from tombstone %#v", obj)
return
err := fmt.Errorf("Couldn't get object from tombstone %#v", obj)
c.logger.Errorf(err.Error())
return err
}
functionObj, ok = tombstone.Obj.(*kubelessApi.Function)
if !ok {
c.logger.Errorf("Tombstone contained object that is not a Function object %#v", obj)
return
err := fmt.Errorf("Tombstone contained object that is not a Pod %#v", obj)
c.logger.Errorf(err.Error())
return err
}
}

if deleted {
c.logger.Infof("Function %s deleted. Removing associated http trigger", functionObj.Name)
httptList, err := c.kubelessclient.KubelessV1beta1().HTTPTriggers(functionObj.Namespace).List(metav1.ListOptions{})
if err != nil {
return err
}
for _, httpTrigger := range httptList.Items {
if httpTrigger.Spec.FunctionName == functionObj.Name {
err = c.kubelessclient.KubelessV1beta1().HTTPTriggers(functionObj.Namespace).Delete(httpTrigger.Name, &metav1.DeleteOptions{})
if err != nil && !k8sErrors.IsNotFound(err) {
c.logger.Errorf("Failed to delete httptrigger created for the function %s in namespace %s, Error: %s", functionObj.ObjectMeta.Name, functionObj.ObjectMeta.Namespace, err)
return err
}
}
}
}

c.logger.Infof("Successfully processed update to function object %s Namespace: %s", functionObj.Name, functionObj.Namespace)
return nil
}

func (c *HTTPTriggerController) httpTriggerObjHasFinalizer(triggerObj *kubelessApi.HTTPTrigger) bool {
Expand Down Expand Up @@ -316,7 +327,7 @@ func httpTriggerObjChanged(oldObj, newObj *kubelessApi.HTTPTrigger) bool {
newSpec := &newObj.Spec
oldSpec := &oldObj.Spec

if newSpec.HostName != oldSpec.HostName || newSpec.TLSAcme != oldSpec.TLSAcme {
if !apiequality.Semantic.DeepEqual(newSpec, oldSpec) {
return true
}
return false
Expand Down
156 changes: 156 additions & 0 deletions pkg/controller/http_trigger_controller_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
package controller

import (
"testing"
"time"

kubelessApi "github.com/kubeless/kubeless/pkg/apis/kubeless/v1beta1"
kubelessFake "github.com/kubeless/kubeless/pkg/client/clientset/versioned/fake"
"github.com/sirupsen/logrus"
extensionsv1beta1 "k8s.io/api/extensions/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/kubernetes/fake"
)

func TestHTTPFunctionAddedUpdated(t *testing.T) {
myNsFoo := metav1.ObjectMeta{
Namespace: "myns",
Name: "foo",
}

f := kubelessApi.Function{
ObjectMeta: myNsFoo,
}

httpTrigger := kubelessApi.HTTPTrigger{
ObjectMeta: myNsFoo,
}

triggerClientset := kubelessFake.NewSimpleClientset(&f, &httpTrigger)

ingress := extensionsv1beta1.Ingress{
ObjectMeta: myNsFoo,
}
clientset := fake.NewSimpleClientset(&ingress)

controller := HTTPTriggerController{
clientset: clientset,
kubelessclient: triggerClientset,
logger: logrus.WithField("controller", "http-trigger-controller"),
}

// no-op for when the function is not deleted
err := controller.functionAddedDeletedUpdated(&f, false)
if err != nil {
t.Errorf("Unexpected error: %v", err)
}

list, err := controller.kubelessclient.KubelessV1beta1().HTTPTriggers("myns").List(metav1.ListOptions{})
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
if len(list.Items) != 1 || list.Items[0].ObjectMeta.Name != "foo" {
t.Errorf("Missing trigger in list: %v", list.Items)
}
}

func TestHTTPFunctionDeleted(t *testing.T) {
myNsFoo := metav1.ObjectMeta{
Namespace: "myns",
Name: "foo",
}

f := kubelessApi.Function{
ObjectMeta: myNsFoo,
}

httpTrigger := kubelessApi.HTTPTrigger{
ObjectMeta: myNsFoo,
Spec: kubelessApi.HTTPTriggerSpec{
FunctionName: myNsFoo.Name,
},
}

triggerClientset := kubelessFake.NewSimpleClientset(&f, &httpTrigger)

ingress := extensionsv1beta1.Ingress{
ObjectMeta: myNsFoo,
}
clientset := fake.NewSimpleClientset(&ingress)

controller := HTTPTriggerController{
clientset: clientset,
kubelessclient: triggerClientset,
logger: logrus.WithField("controller", "http-trigger-controller"),
}

err := controller.functionAddedDeletedUpdated(&f, true)
if err != nil {
t.Errorf("Unexpected error: %v", err)
}

list, err := controller.kubelessclient.KubelessV1beta1().HTTPTriggers("myns").List(metav1.ListOptions{})
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
if len(list.Items) != 0 {
t.Errorf("Trigger should be deleted from list: %v", list.Items)
}
}

func TestHTTPTriggerObjChanged(t *testing.T) {
type testObj struct {
old *kubelessApi.HTTPTrigger
new *kubelessApi.HTTPTrigger
expectedChanged bool
}
t1 := metav1.Time{
Time: time.Now(),
}
t2 := metav1.Time{
Time: time.Now(),
}
testObjs := []testObj{
{
old: &kubelessApi.HTTPTrigger{ObjectMeta: metav1.ObjectMeta{Name: "foo"}},
new: &kubelessApi.HTTPTrigger{ObjectMeta: metav1.ObjectMeta{Name: "foo"}},
expectedChanged: false,
},
{
old: &kubelessApi.HTTPTrigger{ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &t1}},
new: &kubelessApi.HTTPTrigger{ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &t2}},
expectedChanged: true,
},
{
old: &kubelessApi.HTTPTrigger{ObjectMeta: metav1.ObjectMeta{ResourceVersion: "1"}},
new: &kubelessApi.HTTPTrigger{ObjectMeta: metav1.ObjectMeta{ResourceVersion: "2"}},
expectedChanged: true,
},
{
old: &kubelessApi.HTTPTrigger{Spec: kubelessApi.HTTPTriggerSpec{HostName: "a"}},
new: &kubelessApi.HTTPTrigger{Spec: kubelessApi.HTTPTriggerSpec{HostName: "a"}},
expectedChanged: false,
},
{
old: &kubelessApi.HTTPTrigger{Spec: kubelessApi.HTTPTriggerSpec{HostName: "a"}},
new: &kubelessApi.HTTPTrigger{Spec: kubelessApi.HTTPTriggerSpec{HostName: "b"}},
expectedChanged: true,
},
{
old: &kubelessApi.HTTPTrigger{Spec: kubelessApi.HTTPTriggerSpec{TLSAcme: true}},
new: &kubelessApi.HTTPTrigger{Spec: kubelessApi.HTTPTriggerSpec{TLSAcme: false}},
expectedChanged: true,
},
{
old: &kubelessApi.HTTPTrigger{Spec: kubelessApi.HTTPTriggerSpec{Path: "a"}},
new: &kubelessApi.HTTPTrigger{Spec: kubelessApi.HTTPTriggerSpec{Path: "b"}},
expectedChanged: true,
},
}
for _, to := range testObjs {
changed := httpTriggerObjChanged(to.old, to.new)
if changed != to.expectedChanged {
t.Errorf("%v != %v expected to be %v", to.old, to.new, to.expectedChanged)
}
}
}