Move serviceSpec to function object. Fix service modifications

cmd/kubeless/function/deploy.go

@@ -26,9 +26,7 @@ import (
 	"github.com/robfig/cron"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
-	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/util/intstr"
 )
 
 var deployCmd = &cobra.Command{
@@ -181,7 +179,7 @@ var deployCmd = &cobra.Command{
 			"function":   funcName,
 		}
 
-		f, err := getFunctionDescription(cli, funcName, ns, handler, file, funcDeps, runtime, runtimeImage, mem, cpu, timeout, envs, labels, secrets, defaultFunctionSpec)
+		f, err := getFunctionDescription(cli, funcName, ns, handler, file, funcDeps, runtime, runtimeImage, mem, cpu, timeout, port, headless, envs, labels, secrets, defaultFunctionSpec)
 
 		if err != nil {
 			logrus.Fatal(err)
@@ -233,27 +231,6 @@ var deployCmd = &cobra.Command{
 			}
 			httpTrigger.Spec.FunctionName = funcName
 
-			svcSpec := v1.ServiceSpec{
-				Ports: []v1.ServicePort{
-					{
-						Name:     "http-function-port",
-						NodePort: 0,
-						Protocol: v1.ProtocolTCP,
-					},
-				},
-				Selector: f.ObjectMeta.Labels,
-				Type:     v1.ServiceTypeClusterIP,
-			}
-
-			if headless {
-				svcSpec.ClusterIP = v1.ClusterIPNone
-			}
-
-			if port != 0 {
-				svcSpec.Ports[0].Port = port
-				svcSpec.Ports[0].TargetPort = intstr.FromInt(int(port))
-			}
-			httpTrigger.Spec.ServiceSpec = svcSpec
 			err = utils.CreateHTTPTriggerCustomResource(kubelessClient, &httpTrigger)
 			if err != nil {
 				logrus.Fatalf("Failed to deploy HTTP job trigger %s. Received:\n%s", funcName, err)

cmd/kubeless/function/function.go

@@ -33,6 +33,7 @@ import (
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/client-go/kubernetes"
 )
 
@@ -133,7 +134,7 @@ func getContentType(filename string, fbytes []byte) string {
 	return contentType
 }
 
-func getFunctionDescription(cli kubernetes.Interface, funcName, ns, handler, file, deps, runtime, runtimeImage, mem, cpu, timeout string, envs, labels []string, secrets []string, defaultFunction kubelessApi.Function) (*kubelessApi.Function, error) {
+func getFunctionDescription(cli kubernetes.Interface, funcName, ns, handler, file, deps, runtime, runtimeImage, mem, cpu, timeout string, port int32, headless bool, envs, labels []string, secrets []string, defaultFunction kubelessApi.Function) (*kubelessApi.Function, error) {
 
 	function := defaultFunction
 	function.TypeMeta = metav1.TypeMeta{
@@ -234,6 +235,28 @@ func getFunctionDescription(cli kubernetes.Interface, funcName, ns, handler, fil
 		function.Spec.Deployment.Spec.Template.Spec.Containers[0].VolumeMounts = defaultFunction.Spec.Deployment.Spec.Template.Spec.Containers[0].VolumeMounts
 	}
 
+	svcSpec := v1.ServiceSpec{
+		Ports: []v1.ServicePort{
+			{
+				Name:     "http-function-port",
+				NodePort: 0,
+				Protocol: v1.ProtocolTCP,
+			},
+		},
+		Selector: funcLabels,
+		Type:     v1.ServiceTypeClusterIP,
+	}
+
+	if headless {
+		svcSpec.ClusterIP = v1.ClusterIPNone
+	}
+
+	if port != 0 {
+		svcSpec.Ports[0].Port = port
+		svcSpec.Ports[0].TargetPort = intstr.FromInt(int(port))
+	}
+	function.Spec.ServiceSpec = svcSpec
+
 	for _, secret := range secrets {
 		function.Spec.Deployment.Spec.Template.Spec.Volumes = append(function.Spec.Deployment.Spec.Template.Spec.Volumes, v1.Volume{
 			Name: secret + "-vol",

cmd/kubeless/function/function_test.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/api/extensions/v1beta1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/client-go/kubernetes/fake"
 )
 
@@ -99,7 +100,7 @@ func TestGetFunctionDescription(t *testing.T) {
 	file.Close()
 	defer os.Remove(file.Name()) // clean up
 
-	result, err := getFunctionDescription(fake.NewSimpleClientset(), "test", "default", "file.handler", file.Name(), "dependencies", "runtime", "test-image", "128Mi", "", "10", []string{"TEST=1"}, []string{"test=1"}, []string{"secretName"}, kubelessApi.Function{})
+	result, err := getFunctionDescription(fake.NewSimpleClientset(), "test", "default", "file.handler", file.Name(), "dependencies", "runtime", "test-image", "128Mi", "", "10", 8080, false, []string{"TEST=1"}, []string{"test=1"}, []string{"secretName"}, kubelessApi.Function{})
 
 	if err != nil {
 		t.Error(err)
@@ -169,14 +170,23 @@ func TestGetFunctionDescription(t *testing.T) {
 					},
 				},
 			},
+			ServiceSpec: v1.ServiceSpec{
+				Ports: []v1.ServicePort{
+					{Name: "http-function-port", Protocol: "TCP", Port: 8080, TargetPort: intstr.FromInt(8080)},
+				},
+				Selector: map[string]string{
+					"test": "1",
+				},
+				Type: v1.ServiceTypeClusterIP,
+			},
 		},
 	}
 	if !reflect.DeepEqual(expectedFunction, *result) {
 		t.Errorf("Unexpected result. Expecting:\n %+v\nReceived:\n %+v", expectedFunction, *result)
 	}
 
 	// It should take the default values
-	result2, err := getFunctionDescription(fake.NewSimpleClientset(), "test", "default", "", "", "", "", "", "", "", "", []string{}, []string{}, []string{}, expectedFunction)
+	result2, err := getFunctionDescription(fake.NewSimpleClientset(), "test", "default", "", "", "", "", "", "", "", "", 8080, false, []string{}, []string{}, []string{}, expectedFunction)
 
 	if err != nil {
 		t.Error(err)
@@ -197,7 +207,7 @@ func TestGetFunctionDescription(t *testing.T) {
 	file.Close()
 	defer os.Remove(file.Name()) // clean up
 
-	result3, err := getFunctionDescription(fake.NewSimpleClientset(), "test", "default", "file.handler2", file.Name(), "dependencies2", "runtime2", "test-image2", "256Mi", "100m", "20", []string{"TEST=2"}, []string{"test=2"}, []string{"secret2"}, expectedFunction)
+	result3, err := getFunctionDescription(fake.NewSimpleClientset(), "test", "default", "file.handler2", file.Name(), "dependencies2", "runtime2", "test-image2", "256Mi", "100m", "20", 8080, false, []string{"TEST=2"}, []string{"test=2"}, []string{"secret2"}, expectedFunction)
 
 	if err != nil {
 		t.Error(err)
@@ -277,6 +287,15 @@ func TestGetFunctionDescription(t *testing.T) {
 					},
 				},
 			},
+			ServiceSpec: v1.ServiceSpec{
+				Ports: []v1.ServicePort{
+					{Name: "http-function-port", Protocol: "TCP", Port: 8080, TargetPort: intstr.FromInt(8080)},
+				},
+				Selector: map[string]string{
+					"test": "2",
+				},
+				Type: v1.ServiceTypeClusterIP,
+			},
 		},
 	}
 	if !reflect.DeepEqual(newFunction, *result3) {
@@ -313,7 +332,7 @@ func TestGetFunctionDescription(t *testing.T) {
 	file.Close()
 	zipW.Close()
 
-	result4, err := getFunctionDescription(fake.NewSimpleClientset(), "test", "default", "file.handler", newfile.Name(), "dependencies", "runtime", "", "", "", "", []string{}, []string{}, []string{}, expectedFunction)
+	result4, err := getFunctionDescription(fake.NewSimpleClientset(), "test", "default", "file.handler", newfile.Name(), "dependencies", "runtime", "", "", "", "", 8080, false, []string{}, []string{}, []string{}, expectedFunction)
 	if err != nil {
 		t.Error(err)
 	}
@@ -322,7 +341,7 @@ func TestGetFunctionDescription(t *testing.T) {
 	}
 
 	// It should maintain previous HPA definition
-	result5, err := getFunctionDescription(fake.NewSimpleClientset(), "test", "default", "file.handler", file.Name(), "dependencies", "runtime", "test-image", "128Mi", "", "10", []string{"TEST=1"}, []string{"test=1"}, []string{}, kubelessApi.Function{
+	result5, err := getFunctionDescription(fake.NewSimpleClientset(), "test", "default", "file.handler", file.Name(), "dependencies", "runtime", "test-image", "128Mi", "", "10", 8080, false, []string{"TEST=1"}, []string{"test=1"}, []string{}, kubelessApi.Function{
 
 		Spec: kubelessApi.FunctionSpec{
 			HorizontalPodAutoscaler: v2beta1.HorizontalPodAutoscaler{
@@ -335,4 +354,20 @@ func TestGetFunctionDescription(t *testing.T) {
 	if result5.Spec.HorizontalPodAutoscaler.ObjectMeta.Name != "previous-hpa" {
 		t.Error("should maintain previous HPA definition")
 	}
+
+	// It should set the Port and headless service properly
+	result6, err := getFunctionDescription(fake.NewSimpleClientset(), "test", "default", "file.handler", file.Name(), "dependencies", "runtime", "test-image", "128Mi", "", "", 9091, true, []string{}, []string{}, []string{}, kubelessApi.Function{})
+	expectedPort := v1.ServicePort{
+		Name:       "http-function-port",
+		Port:       9091,
+		TargetPort: intstr.FromInt(9091),
+		NodePort:   0,
+		Protocol:   v1.ProtocolTCP,
+	}
+	if !reflect.DeepEqual(result6.Spec.ServiceSpec.Ports[0], expectedPort) {
+		t.Errorf("Unexpected port definition: %v", result6.Spec.ServiceSpec.Ports[0])
+	}
+	if result6.Spec.ServiceSpec.ClusterIP != v1.ClusterIPNone {
+		t.Errorf("Unexpected clusterIP %v", result6.Spec.ServiceSpec.ClusterIP)
+	}
 }

cmd/kubeless/function/update.go

@@ -24,7 +24,6 @@ import (
 	"github.com/kubeless/kubeless/pkg/utils"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
-	"github.com/spf13/pflag"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -128,35 +127,24 @@ var updateCmd = &cobra.Command{
 			}
 			funcDeps = string(bytes)
 		}
-		var headless *bool = nil
-		var port *int32 = nil
-		cmd.Flags().Visit(func(flag *pflag.Flag) {
-			switch flag.Name {
-			case "headless":
-				val, err := cmd.Flags().GetBool("headless")
-				headless = &val
-				if err != nil {
-					logrus.Fatal(err)
-				}
-			case "port":
-				val, err := cmd.Flags().GetInt32("port")
-				port = &val
-				if err != nil {
-					logrus.Fatal(err)
-				}
-			}
-		})
-
-		previousFunction, err := utils.GetFunction(funcName, ns)
+		headless, err := cmd.Flags().GetBool("headless")
+		if err != nil {
+			logrus.Fatal(err)
+		}
+		port, err := cmd.Flags().GetInt32("port")
 		if err != nil {
 			logrus.Fatal(err)
 		}
+		if port <= 0 || port > 65535 {
+			logrus.Fatalf("Invalid port number %d specified", port)
+		}
 
-		if port != nil && (*port <= 0 || *port > 65535) {
-			logrus.Fatalf("Invalid port number %d specified", *port)
+		previousFunction, err := utils.GetFunction(funcName, ns)
+		if err != nil {
+			logrus.Fatal(err)
 		}
 
-		f, err := getFunctionDescription(cli, funcName, ns, handler, file, funcDeps, runtime, runtimeImage, mem, cpu, timeout, envs, labels, secrets, previousFunction)
+		f, err := getFunctionDescription(cli, funcName, ns, handler, file, funcDeps, runtime, runtimeImage, mem, cpu, timeout, port, headless, envs, labels, secrets, previousFunction)
 		if err != nil {
 			logrus.Fatal(err)
 		}

examples/Makefile

@@ -23,6 +23,7 @@ get-python-custom-port:
 	kubeless function deploy get-python-custom-port --trigger-http --runtime python2.7 --handler helloget.foo --from-file python/helloget.py --port 8081
 
 get-python-custom-port-verify:
+	kubectl get svc get-python-custom-port -o yaml | grep 'targetPort: 8081'
 	kubeless function call get-python-custom-port |egrep hello.world
 
 get-python-deps-update:
@@ -86,6 +87,7 @@ get-nodejs-custom-port:
 	kubeless function deploy get-nodejs-custom-port --trigger-http --runtime nodejs6 --handler helloget.foo --from-file nodejs/helloget.js --port 8083
 
 get-nodejs-custom-port-verify:
+	kubectl get svc get-nodejs-custom-port -o yaml | grep 'targetPort: 8083'
 	kubeless function call get-nodejs-custom-port |egrep hello.world
 
 timeout-nodejs:
@@ -134,6 +136,7 @@ get-ruby-custom-port:
 	kubeless function deploy get-ruby-custom-port --trigger-http --runtime ruby2.4 --handler helloget.foo --from-file ruby/helloget.rb --port 8082
 
 get-ruby-custom-port-verify:
+	kubectl get svc get-ruby-custom-port -o yaml | grep 'targetPort: 8082'
 	kubeless function call get-ruby-custom-port |egrep hello.world
 
 get-php:
@@ -231,6 +234,7 @@ post-python-custom-port:
 	kubeless function deploy post-python-custom-port --trigger-http --runtime python2.7 --handler hellowithdata.handler --from-file python/hellowithdata.py --port 8081
 
 post-python-custom-port-verify:
+	kubectl get svc post-python-custom-portport -o yaml | grep 'targetPort: 8081'
 	kubeless function call post-python-custom-port --data '{"it-s": "alive"}'|egrep "it.*alive"
 
 post-nodejs:

pkg/apis/kubeless/v1beta1/function.go

@@ -18,6 +18,7 @@ package v1beta1
 
 import (
 	"k8s.io/api/autoscaling/v2beta1"
+	"k8s.io/api/core/v1"
 	"k8s.io/api/extensions/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
@@ -42,6 +43,7 @@ type FunctionSpec struct {
 	Timeout                 string                          `json:"timeout"`               // Maximum timeout for the function to complete its execution
 	Deps                    string                          `json:"deps"`                  // Function dependencies
 	Deployment              v1beta1.Deployment              `json:"deployment" protobuf:"bytes,3,opt,name=template"`
+	ServiceSpec             v1.ServiceSpec                  `json:"service"`
 	HorizontalPodAutoscaler v2beta1.HorizontalPodAutoscaler `json:"horizontalPodAutoscaler" protobuf:"bytes,3,opt,name=horizontalPodAutoscaler"`
 }
 

pkg/apis/kubeless/v1beta1/http_trigger.go

@@ -17,7 +17,6 @@ limitations under the License.
 package v1beta1
 
 import (
-	"k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -33,12 +32,11 @@ type HTTPTrigger struct {
 
 // HTTPTriggerSpec contains func specification
 type HTTPTriggerSpec struct {
-	FunctionName  string         `json:"function-name"` // Name of the associated function
-	ServiceSpec   v1.ServiceSpec `json:"service"`
-	HostName      string         `json:"host-name"`
-	TLSAcme       bool           `json:"tls"`
-	RouteName     string         `json:"route-name"`
-	EnableIngress bool           `json:"ingress-enabled"`
+	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"`
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

pkg/apis/kubeless/v1beta1/zz_generated.deepcopy.go

@@ -174,6 +174,7 @@ func (in *FunctionList) DeepCopyObject() runtime.Object {
 func (in *FunctionSpec) DeepCopyInto(out *FunctionSpec) {
 	*out = *in
 	in.Deployment.DeepCopyInto(&out.Deployment)
+	in.ServiceSpec.DeepCopyInto(&out.ServiceSpec)
 	in.HorizontalPodAutoscaler.DeepCopyInto(&out.HorizontalPodAutoscaler)
 	return
 }
@@ -193,7 +194,7 @@ func (in *HTTPTrigger) DeepCopyInto(out *HTTPTrigger) {
 	*out = *in
 	out.TypeMeta = in.TypeMeta
 	in.ObjectMeta.DeepCopyInto(&out.ObjectMeta)
-	in.Spec.DeepCopyInto(&out.Spec)
+	out.Spec = in.Spec
 	return
 }
 
@@ -256,7 +257,6 @@ func (in *HTTPTriggerList) DeepCopyObject() runtime.Object {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *HTTPTriggerSpec) DeepCopyInto(out *HTTPTriggerSpec) {
 	*out = *in
-	in.ServiceSpec.DeepCopyInto(&out.ServiceSpec)
 	return
 }
 

pkg/controller/function_controller.go

@@ -25,6 +25,7 @@ import (
 	"k8s.io/api/autoscaling/v2beta1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/api/extensions/v1beta1"
+	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"
@@ -380,8 +381,9 @@ func functionObjChanged(oldFunctionObj, newFunctionObj *kubelessApi.Function) bo
 		return true
 	}
 
-	if fmt.Sprintf("%v", newSpec.Deployment) != fmt.Sprintf("%v", oldSpec.Deployment) ||
-		fmt.Sprintf("%v", newSpec.HorizontalPodAutoscaler) != fmt.Sprintf("%v", oldSpec.HorizontalPodAutoscaler) {
+	if !apiequality.Semantic.DeepEqual(newSpec.ServiceSpec, oldSpec.ServiceSpec) ||
+		!apiequality.Semantic.DeepEqual(newSpec.HorizontalPodAutoscaler, oldSpec.HorizontalPodAutoscaler) ||
+		!apiequality.Semantic.DeepEqual(newSpec.ServiceSpec, oldSpec.ServiceSpec) {
 		return true
 	}
 	return false

pkg/controller/http_trigger_controller.go

@@ -18,7 +18,6 @@ package controller
 
 import (
 	"fmt"
-	"reflect"
 	"time"
 
 	"github.com/kubeless/kubeless/pkg/client/informers/externalversions"
@@ -320,8 +319,5 @@ func httpTriggerObjChanged(oldObj, newObj *kubelessApi.HTTPTrigger) bool {
 	if newSpec.HostName != oldSpec.HostName || newSpec.TLSAcme != oldSpec.TLSAcme {
 		return true
 	}
-	if !reflect.DeepEqual(newSpec.ServiceSpec, oldSpec.ServiceSpec) {
-		return true
-	}
 	return false
 }