Connector example by zeerorg · Pull Request #3 · openfaas/connector-sdk

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 r.g.gupta@outlook.com

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

+	}
+}
+
+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

+	if !ok {
+		gURL = "http://127.0.0.1:8080/"
 	}
+	fmt.Println("Gateway Url: ", gURL)


Let's not print this here.

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

+
 	// 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

+	config := getControllerConfig()

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


Remove this line.

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

+	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

@@ -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

+# 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

@@ -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

+
+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

+
+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

+
+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

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


Change to gatewayURL

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

+      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 <r.g.gupta@outlook.com>

Signed-off-by: Rishabh Gupta<r.g.gupta@outlook.com>

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

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 <r.g.gupta@outlook.com>

Referencing dockerhub image zeerorg/connector-sample

ac979d4

Signed-off-by: Rishabh Gupta<r.g.gupta@outlook.com>

Added README for sample connector

c8a09b1

Signed-off-by: Rishabh Gupta<r.g.gupta@outlook.com>

Added vendor folder and suggested fixes

8987d86

Signed-off-by: Rishabh Gupta<r.g.gupta@outlook.com>

Rebase with upstream. Added topic environment variable.

52e384f

Signed-off-by: Rishabh Gupta<r.g.gupta@outlook.com>

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

Conversation

zeerorg commented Jan 15, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexellis left a comment

Choose a reason for hiding this comment

Uh oh!

zeerorg commented Jan 15, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexellis commented Jan 25, 2019

Uh oh!

zeerorg commented Jan 28, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants