Connector example #3

zeerorg · 2019-01-15T08:31:07Z

This PR updates the connector example with swarm and kubernetes deployment files.

How it is tested:

Clone this Pull request locally.
Go into the directory: cd ./connector-sdk/cmd/tester/yaml
For openfaas deployed on docker swarm do: docker stack deploy func -c ./docker-compose.yml
For openfaas deployed on kubernetes do: kubectl create -f ./kubernetes --namespace openfaas

Run this command to deploy a sample function and see trigger-func invocation count growing in ui.

faas-cli deploy --image=functions/nodeinfo --name=trigger-func --annotation topic=faas-request

Signed-off-by: Rishabh Gupta [email protected]

alexellis · 2019-01-15T08:45:02Z

cmd/tester/main.go

+	}
+}
+
+func getControllerConfig() *types.ControllerConfig {


If running in a the default is gateway.openfaas:8080 / gateway:8080 on Swarm. We should just not default at all if not passed and return an error (types.ControllerConfig, error)

alexellis · 2019-01-15T08:45:29Z

cmd/tester/main.go

 	}
+	fmt.Println("Gateway Url: ", gURL)


Let's not print this here.

alexellis · 2019-01-15T08:46:01Z

cmd/tester/main.go


 	controller := types.NewController(creds, config)
 	fmt.Println(controller)
 	controller.BeginMapBuilder()

+	topics := getTopics()
+
 	// Simulate events emitting from queue/pub-sub
 	for {
 		time.Sleep(2 * time.Second)


Make this configurable to please.

Should I add an env-var to make this configurable ? Or a simple variable is enough ?

alexellis · 2019-01-15T08:46:06Z

cmd/tester/main.go

-		PrintResponse:   true,
-	}
+	creds := types.GetCredentials() // Get credentials for gateway login
+	config := getControllerConfig()

 	controller := types.NewController(creds, config)
 	fmt.Println(controller)


Remove this line.

alexellis · 2019-01-15T08:46:40Z

cmd/tester/main.go

-		PrintResponse:   true,
-	}
+	creds := types.GetCredentials() // Get credentials for gateway login
+	config := getControllerConfig()

 	controller := types.NewController(creds, config)
 	fmt.Println(controller)


Replace fmt with log for use in the container.

alexellis · 2019-01-15T08:47:06Z

cmd/tester/Dockerfile

@@ -0,0 +1,16 @@
+FROM golang:1.9.2 as builder


Copy the example from the vcenter connector please it has an updated Golang version.

alexellis · 2019-01-15T08:47:26Z

cmd/tester/Dockerfile

+# Stripping via -ldflags "-s -w" 
+RUN CGO_ENABLED=0 GOOS=linux go build -a -ldflags "-s -w" -installsuffix cgo -o ./connector
+
+FROM alpine


Copy the example from the vcenter connector please it has an updated alpine version, we must always pin to 3.8 or whatever version we are targeting.

alexellis · 2019-01-15T08:47:51Z

cmd/tester/Dockerfile

@@ -0,0 +1,16 @@
+FROM golang:1.9.2 as builder
+RUN mkdir -p /go/src/github.com/openfaas-incubator/connector
+WORKDIR /go/src/github.com/openfaas-incubator/connector


The path I guess is connector-sdk right? To match the repo.

Actually I think it's:

/go/src/github.com/openfaas-incubator/connector-sdk/cmd/tester/ correct?

alexellis · 2019-01-15T08:48:14Z

cmd/tester/Dockerfile

+
+COPY main.go    .
+
+RUN go get -d -v ./...


Please remove go get

It should use vendoring via dep or the correct path.

alexellis · 2019-01-15T08:48:28Z

cmd/tester/README.md

+
+1. Clone this repository: `git clone https://github.com/openfaas-incubator/connector-sdk.git`
+2. Go into the directory: `cd ./connector-sdk/cmd/tester/yaml`
+3. For openfaas deployed on docker swarm do: `docker stack deploy func -c ./docker-compose.yml`


openfaas -> OpenFaaS

alexellis · 2019-01-15T08:48:40Z

cmd/tester/README.md

+
+1. Clone this repository: `git clone https://github.com/openfaas-incubator/connector-sdk.git`
+2. Go into the directory: `cd ./connector-sdk/cmd/tester/yaml`
+3. For openfaas deployed on docker swarm do: `docker stack deploy func -c ./docker-compose.yml`


docker swarm -> Docker Swarm

alexellis

Hi there, thanks for working on this PR. I had some mixed feelings on creating and maintaining a separate set of Dockerfiles and Kubernetes YAML files, but since you've done it I'd like to give you some feedback so that we can merge it.

The items are very minor and once done can be merged. Just ping me again when you've worked through the suggestions.

zeerorg · 2019-01-15T16:00:13Z

I've worked through all the suggestions. This should be good to go.

Some major changes:

I removed fetching topics from environment variable (made the example hard to follow). now a single topic is defined.
Also removed print_response environment variable was not doing anything

alexellis · 2019-01-25T11:46:44Z

cmd/tester/main.go

+}
+
+func getControllerConfig() (*types.ControllerConfig, error) {
+	gURL, ok := os.LookupEnv("gateway_url")


Change to gatewayURL

alexellis · 2019-01-25T11:47:12Z

cmd/tester/yaml/kubernetes/connector-dep.yml

+      containers:
+      - name: sample-connector
+        image: zeerorg/connector-sample:1.0
+        env:


Please add topics as an env-var, this is how connectors are intended to work.

alexellis · 2019-01-25T11:48:30Z

The scope of this PR was simply to provide a Dockerfile and Kubernetes YAML, not to change the example behaviour or functionality. Please can you revert the below?

Some major changes:
I removed fetching topics from environment variable (made the example hard to follow). now a single topic is defined.
Also removed print_response environment variable was not doing anything

I think the extracting the configuration loading is fine to keep in the changes, but in a separate PR.

Alex

Signed-off-by: Rishabh Gupta <[email protected]>

Signed-off-by: Rishabh Gupta<[email protected]>

zeerorg · 2019-01-28T08:55:50Z

The previous example had fixed gateway url, I changed it to rely on environment variable and same is done for topic which rely on environment variable.

This pretty much resembles a very trimmed down version of kafka connector.

derek bot added the new-contributor label Jan 15, 2019

alexellis reviewed Jan 15, 2019

View reviewed changes

cmd/tester/main.go Outdated

}

fmt.Println("Gateway Url: ", gURL)

Copy link

Member

alexellis Jan 15, 2019

Choose a reason for hiding this comment

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

Let's not print this here.

alexellis reviewed Jan 15, 2019

View reviewed changes

alexellis requested changes Jan 15, 2019

View reviewed changes

alexellis reviewed Jan 25, 2019

View reviewed changes

alexellis added the blocked/too-many-changes label Jan 25, 2019

zeerorg force-pushed the connector-example branch from 6f97e78 to ddcf919 Compare January 28, 2019 08:45

zeerorg added 5 commits January 28, 2019 14:19

Connector sdk example

7fc17fb

Signed-off-by: Rishabh Gupta <[email protected]>

Referencing dockerhub image zeerorg/connector-sample

ac979d4

Signed-off-by: Rishabh Gupta<[email protected]>

Added README for sample connector

c8a09b1

Signed-off-by: Rishabh Gupta<[email protected]>

Added vendor folder and suggested fixes

8987d86

Signed-off-by: Rishabh Gupta<[email protected]>

Rebase with upstream. Added topic environment variable.

52e384f

Signed-off-by: Rishabh Gupta<[email protected]>

zeerorg force-pushed the connector-example branch from ddcf919 to 52e384f Compare January 28, 2019 08:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Connector example #3

Connector example #3

zeerorg commented Jan 15, 2019

alexellis Jan 15, 2019

alexellis Jan 15, 2019

alexellis Jan 15, 2019

zeerorg Jan 28, 2019

alexellis Jan 15, 2019

alexellis Jan 15, 2019

alexellis Jan 15, 2019

alexellis Jan 15, 2019

alexellis Jan 15, 2019

alexellis Jan 15, 2019

alexellis Jan 15, 2019

alexellis Jan 15, 2019

alexellis Jan 15, 2019

alexellis left a comment

zeerorg commented Jan 15, 2019

alexellis Jan 25, 2019

alexellis Jan 25, 2019

alexellis commented Jan 25, 2019

zeerorg commented Jan 28, 2019

Connector example #3

Are you sure you want to change the base?

Connector example #3

Conversation

zeerorg commented Jan 15, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexellis left a comment

Choose a reason for hiding this comment

zeerorg commented Jan 15, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexellis commented Jan 25, 2019

zeerorg commented Jan 28, 2019