Repository: apache/camel-k
Pull request #1724 (2020-10-01 08:19:24)

fix: Adapt Knative service definition to new KnativeEnvironment definition
This PR takes into account the changes introduced in https://github.com/apache/camel-k-runtime/pull/501.

It primary aims at fixing CI tests to validate other PRs.

**Release Note**
```release-note
NONE
```


astefanutti: CamelSource controller creates CamelEnvironment with the old Host and Port fields, so this is more involved.
lburgazzoli: @astefanutti should we bring back the host & port on the runtime part till we change the CamelSource ? 
astefanutti: It might be possible to do the conversion in the Knative trait. I'm exploring that approach.
nicolaferraro: > It might be possible to do the conversion in the Knative trait. I'm exploring that approach. Even if that is temporary, I'd like to fix CI tests to validate other PRs.

Thanks, I'll change the approach for CamelSource 0.19
astefanutti: Now that I've got the CamelSource tests working, the `TestRunBroker` e2e test fails 🤕.

It seems the path part gets duplicated for some reasons:
```
Error:     dump.go:146:     > 2020-10-02 09:17:57,938 WARN  [org.apa.cam.com.tim.TimerConsumer] (vert.x-eventloop-thread-0) Error processing exchange. Exchange[8C338EDC8A55717-000000000000024B]. Caused by: [org.apache.camel.CamelException - HTTP operation failed invoking http://broker-ingress.knative-eventing.svc.cluster.local/test-fb340f4a-c955-4254-b03c-dbe275d467d3/default/test-fb340f4a-c955-4254-b03c-dbe275d467d3/default with statusCode: 400, statusMessage: Bad Request]: org.apache.camel.CamelException: HTTP operation failed invoking http://broker-ingress.knative-eventing.svc.cluster.local/test-fb340f4a-c955-4254-b03c-dbe275d467d3/default/test-fb340f4a-c955-4254-b03c-dbe275d467d3/default with statusCode: 400, statusMessage: Bad Request
Error:     dump.go:146:     > 	at org.apache.camel.component.knative.http.KnativeHttpProducer.lambda$process$0(KnativeHttpProducer.java:135)
Error:     dump.go:146:     > 	at io.vertx.ext.web.client.impl.HttpContext.handleDispatchResponse(HttpContext.java:313)
Error:     dump.go:146:     > 	at io.vertx.ext.web.client.impl.HttpContext.execute(HttpContext.java:300)
Error:     dump.go:146:     > 	at io.vertx.ext.web.client.impl.HttpContext.next(HttpContext.java:275)
Error:     dump.go:146:     > 	at io.vertx.ext.web.client.impl.predicate.PredicateInterceptor.handle(PredicateInterceptor.java:69)
Error:     dump.go:146:     > 	at io.vertx.ext.web.client.impl.predicate.PredicateInterceptor.handle(PredicateInterceptor.java:32)
Error:     dump.go:146:     > 	at io.vertx.ext.web.client.impl.HttpContext.next(HttpContext.java:272)
Error:     dump.go:146:     > 	at io.vertx.ext.web.client.impl.HttpContext.fire(HttpContext.java:282)
Error:     dump.go:146:     > 	at io.vertx.ext.web.client.impl.HttpContext.dispatchResponse(HttpContext.java:243)
Error:     dump.go:146:     > 	at io.vertx.ext.web.client.impl.HttpContext.lambda$null$2(HttpContext.java:373)
Error:     dump.go:146:     > 	at io.vertx.core.impl.ContextImpl.executeTask(ContextImpl.java:366)
Error:     dump.go:146:     > 	at io.vertx.core.impl.EventLoopContext.lambda$executeAsync$0(EventLoopContext.java:38)
Error:     dump.go:146:     > 	at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:164)
Error:     dump.go:146:     > 	at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:472)
Error:     dump.go:146:     > 	at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:497)
Error:     dump.go:146:     > 	at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:989)
Error:     dump.go:146:     > 	at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
Error:     dump.go:146:     > 	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
Error:     dump.go:146:     > 	at java.base/java.lang.Thread.run(Thread.java:834)
```

lburgazzoli: @astefanutti that could be related to a latest change on the runtime so if you set both the uri and path metadata, they are joined
nicolaferraro: So, I think there are changes in runtime that should make tests not to pass the "yaks" job.. but I see it green.
(i.e. Kamelet start and end endpoint should be renamed into "kamelet:source" and "kamelet:sink"... "direct:{{routeId}}" should no longer work).

So I've noticed that only two out of 4 dirs are executed by Yaks? Any idea why @christophd ?
astefanutti: @nicolaferraro @christophd YAKS call from the Knative workflow had to be changed from an _all directories_ to an _include directories_ with https://github.com/apache/camel-k/commit/e5fdc4ba4b9907b899312947958beca96b0119c3#diff-8bd69437e2586424cb7447c8eed690e7R320-R321. Ideally, the monitoring tests, that can only be run on OpenShift 4 should be excluded, but it seems there is no way to specific an _exclude_ list to the YAKS CLI. 
astefanutti: @lburgazzoli Thanks, must be that indeed:

```
{
   "services":[
      {
         "type":"event",
         "name":"default",
         "url":"http://broker-ingress.knative-eventing.svc.cluster.local/camel-k/default",
         "metadata":{
            "camel.endpoint.kind":"sink",
            "knative.apiVersion":"eventing.knative.dev/v1",
            "knative.kind":"Broker",
            "service.path":"/camel-k/default"
         }
      }
   ]
}
```
Does the runtime expect the URL to be path-less?
lburgazzoli: @astefanutti no it assumes that if you provide a path, that info should be used to compute the full URI so you either provide a full `uri` and no `service.path` or you provide a base `path` and `a relative service.path`
astefanutti: @lburgazzoli thanks, I've removed the extra `path` metadata.

Patch:
diff --git a/pkg/apis/camel/v1/knative/types.go b/pkg/apis/camel/v1/knative/types.go
index 430c2239a..c4147d6a8 100644
--- a/pkg/apis/camel/v1/knative/types.go
+++ b/pkg/apis/camel/v1/knative/types.go
@@ -33,11 +33,14 @@ func NewCamelEnvironment() CamelEnvironment {
 
 // CamelServiceDefinition defines the parameters to connect to Knative service. It's also used for exposed services
 type CamelServiceDefinition struct {
-	ServiceType CamelServiceType  `json:"type"`
-	Name        string            `json:"name"`
-	Host        string            `json:"host,omitempty"`
-	Port        *int              `json:"port,omitempty"`
-	Metadata    map[string]string `json:"metadata,omitempty"`
+	ServiceType CamelServiceType `json:"type"`
+	Name        string           `json:"name"`
+	// Deprecated: use URL instead
+	Host string `json:"host,omitempty"`
+	// Deprecated: use URL instead
+	Port     *int              `json:"port,omitempty"`
+	URL      string            `json:"url,omitempty"`
+	Metadata map[string]string `json:"metadata,omitempty"`
 }
 
 // CamelEndpointKind --
@@ -73,11 +76,6 @@ func (s CamelServiceType) ResourceDescription(subject string) string {
 // Meta Options
 const (
 	CamelMetaServicePath = "service.path"
-	CamelMetaServiceID   = "service.id"
-	CamelMetaServiceName = "service.name"
-	CamelMetaServiceHost = "service.host"
-	CamelMetaServicePort = "service.port"
-	CamelMetaServiceZone = "service.zone"
 
 	CamelMetaKnativeKind       = "knative.kind"
 	CamelMetaKnativeAPIVersion = "knative.apiVersion"
diff --git a/pkg/apis/camel/v1/knative/types_support.go b/pkg/apis/camel/v1/knative/types_support.go
index 796b89d3b..f98677436 100644
--- a/pkg/apis/camel/v1/knative/types_support.go
+++ b/pkg/apis/camel/v1/knative/types_support.go
@@ -20,19 +20,15 @@ package knative
 import (
 	"encoding/json"
 	"net/url"
-	"strconv"
 )
 
 // BuildCamelServiceDefinition creates a CamelServiceDefinition from a given URL
 func BuildCamelServiceDefinition(name string, endpointKind CamelEndpointKind, serviceType CamelServiceType,
 	serviceURL url.URL, apiVersion, kind string) (CamelServiceDefinition, error) {
 
-	port := 80
-
 	definition := CamelServiceDefinition{
 		Name:        name,
-		Host:        serviceURL.Host,
-		Port:        &port,
+		URL:         serviceURL.String(),
 		ServiceType: serviceType,
 		Metadata: map[string]string{
 			CamelMetaEndpointKind:      string(endpointKind),
@@ -40,17 +36,6 @@ func BuildCamelServiceDefinition(name string, endpointKind CamelEndpointKind, se
 			CamelMetaKnativeKind:       kind,
 		},
 	}
-	portStr := serviceURL.Port()
-	if portStr != "" {
-		port, err := strconv.Atoi(portStr)
-		if err != nil {
-			return CamelServiceDefinition{}, err
-		}
-		definition.Port = &port
-	}
-	if serviceURL.Path != "" {
-		definition.Metadata[CamelMetaServicePath] = serviceURL.Path
-	}
 
 	return definition, nil
 }
diff --git a/pkg/trait/knative.go b/pkg/trait/knative.go
index 5ff9c46cd..ed41b6b62 100644
--- a/pkg/trait/knative.go
+++ b/pkg/trait/knative.go
@@ -21,15 +21,16 @@ import (
 	"fmt"
 	"net/url"
 	"reflect"
+	"strconv"
 	"strings"
 
-	"github.com/apache/camel-k/pkg/util/kubernetes"
 	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
 	knativeapi "github.com/apache/camel-k/pkg/apis/camel/v1/knative"
 	"github.com/apache/camel-k/pkg/metadata"
 	"github.com/apache/camel-k/pkg/util"
 	"github.com/apache/camel-k/pkg/util/envvar"
 	knativeutil "github.com/apache/camel-k/pkg/util/knative"
+	"github.com/apache/camel-k/pkg/util/kubernetes"
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
@@ -243,6 +244,21 @@ func (t *knativeTrait) Apply(e *Environment) error {
 			}
 		}
 
+		// Convert deprecated Host and Port fields to URL field
+		// Can be removed once CamelSource controller migrate to the new API
+		for i, service := range env.Services {
+			if service.URL == "" {
+				URL := "http://" + service.Host
+				if service.Port != nil {
+					URL = URL + ":" + strconv.Itoa(*service.Port)
+				}
+				service.URL = URL
+				service.Host = ""
+				service.Port = nil
+				env.Services[i] = service
+			}
+		}
+
 		if err := t.configureChannels(e, &env); err != nil {
 			return err
 		}


Test patch:
diff --git a/pkg/trait/knative_test.go b/pkg/trait/knative_test.go
index 63586c95a..6bea95333 100644
--- a/pkg/trait/knative_test.go
+++ b/pkg/trait/knative_test.go
@@ -123,28 +123,28 @@ func TestKnativeEnvConfigurationFromTrait(t *testing.T) {
 
 	cSource1 := ne.FindService("channel-source-1", knativeapi.CamelEndpointKindSource, knativeapi.CamelServiceTypeChannel, "messaging.knative.dev/v1beta1", "Channel")
 	assert.NotNil(t, cSource1)
-	assert.Empty(t, cSource1.Host)
+	assert.Empty(t, cSource1.URL)
 
 	cSink1 := ne.FindService("channel-sink-1", knativeapi.CamelEndpointKindSink, knativeapi.CamelServiceTypeChannel, "messaging.knative.dev/v1beta1", "Channel")
 	assert.NotNil(t, cSink1)
-	assert.Equal(t, "channel-sink-1.host", cSink1.Host)
+	assert.Equal(t, "http://channel-sink-1.host/", cSink1.URL)
 
 	eSource1 := ne.FindService("endpoint-source-1", knativeapi.CamelEndpointKindSource, knativeapi.CamelServiceTypeEndpoint, "serving.knative.dev/v1", "Service")
 	assert.NotNil(t, eSource1)
-	assert.Empty(t, eSource1.Host)
+	assert.Empty(t, eSource1.URL)
 
 	eSink1 := ne.FindService("endpoint-sink-1", knativeapi.CamelEndpointKindSink, knativeapi.CamelServiceTypeEndpoint, "serving.knative.dev/v1", "Service")
 	assert.NotNil(t, eSink1)
-	assert.Equal(t, "endpoint-sink-1.host", eSink1.Host)
+	assert.Equal(t, "http://endpoint-sink-1.host/", eSink1.URL)
 	eSink2 := ne.FindService("endpoint-sink-2", knativeapi.CamelEndpointKindSink, knativeapi.CamelServiceTypeEndpoint, "serving.knative.dev/v1", "Service")
 	assert.NotNil(t, eSink2)
-	assert.Equal(t, "endpoint-sink-2.host", eSink2.Host)
+	assert.Equal(t, "http://endpoint-sink-2.host/", eSink2.URL)
 
 	eEventSource := ne.FindService("default", knativeapi.CamelEndpointKindSource, knativeapi.CamelServiceTypeEvent, "eventing.knative.dev/v1beta1", "Broker")
 	assert.NotNil(t, eEventSource)
 	eEventSink := ne.FindService("default", knativeapi.CamelEndpointKindSink, knativeapi.CamelServiceTypeEvent, "eventing.knative.dev/v1beta1", "Broker")
 	assert.NotNil(t, eEventSink)
-	assert.Equal(t, "broker-default.host", eEventSink.Host)
+	assert.Equal(t, "http://broker-default.host/", eEventSink.URL)
 }
 
 func TestKnativeEnvConfigurationFromSource(t *testing.T) {
@@ -243,8 +243,7 @@ func TestKnativeEnvConfigurationFromSource(t *testing.T) {
 
 	source := ne.FindService("s3fileMover1", knativeapi.CamelEndpointKindSource, knativeapi.CamelServiceTypeEndpoint, "serving.knative.dev/v1", "Service")
 	assert.NotNil(t, source)
-	assert.Empty(t, source.Host)
-	assert.Nil(t, source.Port)
+	assert.Empty(t, source.URL)
 	assert.Empty(t, source.Metadata[knativeapi.CamelMetaKnativeReply])
 
 	channel := ne.FindService("channel-source-1", knativeapi.CamelEndpointKindSource, knativeapi.CamelServiceTypeChannel, "", "")