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

Perform a graceful shutdown for the function proxy on SIGINT & SIGTERM. #1108

Merged
merged 6 commits into from
Jan 16, 2020
Merged

Perform a graceful shutdown for the function proxy on SIGINT & SIGTERM. #1108

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
18415a5
Perform a graceful shutdown for the function proxy on SIGINT & SIGTERM.
Jan 15, 2020
413403e
Add comment to exported function NewServer.
d0x2f Jan 15, 2020
8246415
gofmt
d0x2f Jan 15, 2020
5d9f243
Read SHUTDOWN_TIMEOUT env var for the shutdown timeout value.
Jan 15, 2020
59da64f
Add comment to exported function.
Jan 15, 2020
7cea83a
replace accidently removed function call.
Jan 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 33 additions & 5 deletions pkg/function-proxy/proxy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ import (
"net/http"
"os"
"os/exec"
"os/signal"
"syscall"
"time"

"github.com/kubeless/kubeless/pkg/function-proxy/utils"

Expand Down Expand Up @@ -80,10 +83,35 @@ func startNativeDaemon() {
}
}

func gracefulShutdown(server *http.Server) {
stop := make(chan os.Signal, 1)
signal.Notify(stop, os.Interrupt, syscall.SIGINT, syscall.SIGTERM)
Copy link
Contributor

Choose a reason for hiding this comment

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

as far as I know, you cannot capture a SIGTERM signal

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually you can capture SIGTERM, and it's what kubernetes sends to a pod when it requests a graceful shutdown. 30 seconds later it'll send SIGKILL which can't be captured, maybe that's what you're thinking of?

It can be tested by running the proxy locally and running kill -15 <pid>.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See https://cloud.google.com/blog/products/gcp/kubernetes-best-practices-terminating-with-grace

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, indeed they suggest so. I tried it with a simple example and it didn't seem to work:

▶ cat t.go 
package main

import (
    "fmt"
    "os"
    "os/signal"
    "syscall"
)

func main() {

    sigs := make(chan os.Signal, 1)
    done := make(chan bool, 1)

    signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM)

    go func() {
        sig := <-sigs
        fmt.Println()
        fmt.Println(sig)
        done <- true
    }()

    fmt.Println("awaiting signal")
    <-done
    fmt.Println("exiting")
}

▶ go run t.go
awaiting signal
^C
interrupt
exiting

▶ go run t.go  
awaiting signal
[1]    17141 terminated  go run t.go

As you can see, in the second execution I am sending kill -15 <pid> and it's not catching the signal (but I may be missing something),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Curiously I tried your example.
When running with go run foo.go it seems to make two processes, when I sent sigterm to the binary in /tmp it worked as expected.
i.e. the second entry in this ps output
image
Compiling with go build foo.go and doing the same with the produced binary should work as well.

Anyway thanks for being so responsive!
I hope to get the node runtime doing the same soon.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I see, good to know, thanks for following up!

<-stop
timeout := 10 * time.Second
Copy link
Contributor

Choose a reason for hiding this comment

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

it may be useful to configure this timeout, maybe we should add a env var to be able to change the default value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added an env var SHUTDOWN_TIMEOUT to control this.

ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()

log.Printf("Shuting down with timeout: %s\n", timeout)
if err := server.Shutdown(ctx); err != nil {
log.Printf("Error: %v\n", err)
} else {
log.Println("Server stopped")
}
}

func main() {
go startNativeDaemon()
http.HandleFunc("/", handler)
http.HandleFunc("/healthz", health)
http.Handle("/metrics", promhttp.Handler())
utils.ListenAndServe()
mux := http.NewServeMux()
mux.HandleFunc("/", handler)
mux.HandleFunc("/healthz", health)
mux.Handle("/metrics", promhttp.Handler())

server := utils.NewServer(mux)

go func() {
if err := server.ListenAndServe(); err != http.ErrServerClosed {
panic(err)
}
}()

gracefulShutdown(server)
}
9 changes: 4 additions & 5 deletions pkg/function-proxy/utils/proxy-utils.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,8 @@ func Handler(w http.ResponseWriter, r *http.Request, h Handle) {
}
}

// ListenAndServe starts an HTTP server in FUNC_PORT using custom logging
func ListenAndServe() {
if err := http.ListenAndServe(fmt.Sprintf(":%s", funcPort), logReq(http.DefaultServeMux)); err != nil {
panic(err)
}
// NewServer returns an HTTP server ready to listen on the configured port
// and with logReq mixed in for logging.
func NewServer(mux *http.ServeMux) *http.Server {
return &http.Server{Addr: fmt.Sprintf(":%s", funcPort), Handler: logReq(mux)}
}