Bootstrap the manager process #1525
Conversation
|
This PR has been released (on staging) and is available for download with a embedded-cluster-smoke-test-staging-app license ID. Online Installer: Airgap Installer (may take a few minutes before the airgap bundle is built): Happy debugging! |
| } | ||
|
|
||
| func GetSocketPath() (string, error) { | ||
| tmpDir := os.TempDir() |
There was a problem hiding this comment.
shouldnt you use /var/run for this?
There was a problem hiding this comment.
Can you add a comment here justifying why its not in the run dir and explaining that this will be created in our data directory rather than /tmp
sgalsaleh
force-pushed
the
bootstrap-manager
branch
from
fa1c5a5
to
f868f71
Compare
| } | ||
|
|
||
| func GetSocketPath() (string, error) { | ||
| tmpDir := os.TempDir() |
There was a problem hiding this comment.
Can you add a comment here justifying why its not in the run dir and explaining that this will be created in our data directory rather than /tmp
pkg/socket/server.go
Outdated
| if _, err := os.Stat(socketFile); err == nil { | ||
| if err := os.Remove(socketFile); err != nil { |
There was a problem hiding this comment.
could you combine this so it is atomic and use os.Remove and handle os.ErrNotExist?
pkg/websocket/server.go
Outdated
| "k8s.io/apimachinery/pkg/types" | ||
| ) | ||
|
|
||
| func ConnectToKOTSWebSocket(ctx context.Context) error { |
There was a problem hiding this comment.
| func ConnectToKOTSWebSocket(ctx context.Context) error { | |
| func ConnectToKOTSWebSocket(ctx context.Context) { |
| logrus.Infof("Sending ping message '%s' to server", pingMsg) | ||
|
|
||
| if err := conn.WriteControl(gwebsocket.PingMessage, []byte(pingMsg), time.Now().Add(1*time.Second)); err != nil { | ||
| return errors.Wrap(err, "send ping message") |
There was a problem hiding this comment.
should this really exit the loop? this is happening in a go routine. dont you want to just log and retry?
There was a problem hiding this comment.
this error typically happens when the connection is closed. when this returns, the manager will try to reconnect to kots again.
|
|
||
| func wsPingHandler(conn *gwebsocket.Conn) func(message string) error { | ||
| return func(message string) error { | ||
| logrus.Infof("Received ping message '%s' from server", message) |
There was a problem hiding this comment.
This is going to cause too much noise in the logs. My philosophy on logging is anything that is happening on a schedule should probably be more highly scrutinized when logging at higher levels. Could you move this to trace or debug level?
|
|
||
| func wsPongHandler() func(message string) error { | ||
| return func(message string) error { | ||
| logrus.Infof("Received pong message '%s' from server", message) |
| logrus.Infof("Received ping message '%s' from server", message) | ||
| logrus.Infof("Sending pong message '%s' from server", message) | ||
| if err := conn.WriteControl(gwebsocket.PongMessage, []byte(message), time.Now().Add(1*time.Second)); err != nil { | ||
| logrus.Errorf("Failed to send pong message to server: %v", err) |
There was a problem hiding this comment.
Why log and not return the error here? Im not saying it is correct. im just assuming the client will not receive the error so im wondering what is the desired behavior here.
| Use: "status", | ||
| Short: fmt.Sprintf("Get the status of the %s cluster manager", name), | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| return getServerStatus(ctx) |
There was a problem hiding this comment.
| return getServerStatus(ctx) | |
| return printServerStatus(ctx) |
| return cmd | ||
| } | ||
|
|
||
| func getServerStatus(ctx context.Context) error { |
There was a problem hiding this comment.
| func getServerStatus(ctx context.Context) error { | |
| func printServerStatus(ctx context.Context) error { |
No description provided.