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

Add golang runtime #671

Merged
merged 4 commits into from
Apr 9, 2018
Merged

Add golang runtime #671

merged 4 commits into from
Apr 9, 2018

Conversation

andresmgot
Copy link
Contributor

Issue Ref: #169

Description:

Add Golang as a supported runtime. Review docs for developing a new runtime.

TODOs:

  • Ready to review
  • Automated Tests
  • Docs

Andres added 2 commits April 5, 2018 16:18
@murali-reddy
Copy link
Contributor

General comment. Perhaps we need to think also on the user feedback when function fails to compile.

Right now if I give incorrect go code, pod deployment gets stuck in Init:Error, with no clear error message on what happened. A descriptive termination message would be good.

@andresmgot
Copy link
Contributor Author

Agree, let me check if we can provide an easier way of debugging.

Copy link
Contributor

@murali-reddy murali-reddy left a comment

Choose a reason for hiding this comment

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

some minor comments


// Handling Functions
func health(w http.ResponseWriter, r *http.Request) {
w.Write([]byte("OK"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we not going to handle the pod unhealthy case when go routine running the function times out, and runs forever?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We cannot handle that using goroutines (it is not possible to kill/stop a goroutine from outside). The other option would be that the process kills itself when a timeout is reached (similar to what we do in NodeJS) forcing a restart of the pod. What do you think?


USER 1000

CMD [ "/kubeless/server" ]
Copy link
Contributor

Choose a reason for hiding this comment

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

Newline needed here.

ADD docker/runtime/golang/Gopkg.toml $GOPATH/src/controller/
WORKDIR $GOPATH/src/controller/
ADD docker/runtime/golang/kubeless.tpl.go $GOPATH/src/controller/
RUN dep ensure
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have first hand experiance. But please see dep issues, there are plenty of issues where dep init/ensure gets stuck forever for various reasons. Does it make sense to have a time out?

Copy link
Contributor Author

@andresmgot andresmgot Apr 6, 2018

Choose a reason for hiding this comment

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

I think that it would be difficult to set the proper value, it would depend on the dependencies to install. A timeout of several minutes may be insufficient in some scenarios and a timeout of ~1h would not be that useful since whoever is using it would detect that something is not going well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure. We can not make decisions on what is right time out value. But just looking at dep issues dep ensure can get stuck various reason. But its not this PR's problem. As long as kubeless user can figure whats going wrong.

@murali-reddy
Copy link
Contributor

Overall LGTM.

@andresmgot andresmgot merged commit 65f0ae2 into vmware-archive:master Apr 9, 2018
@allantargino allantargino mentioned this pull request Apr 14, 2018
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants