Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added grpc interceptor and singleton changes #130

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 18 additions & 0 deletions pkg/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package client

import (
"context"
"fmt"
"os"
"strings"
Expand All @@ -15,7 +16,9 @@ import (
"google.golang.org/grpc/keepalive"
log "k8s.io/klog"

"github.com/microsoft/moc-sdk-for-go/pkg/constant"
"github.com/microsoft/moc/pkg/auth"
"github.com/microsoft/moc/pkg/errors"
)

const (
Expand All @@ -29,6 +32,20 @@ var (
connectionCache map[string]*grpc.ClientConn
)

func clientConnOptionsInterceptor() grpc.UnaryClientInterceptor {
return func(ctx context.Context, method string, req, reply interface{}, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error {
err := invoker(ctx, method, req, reply, cc, opts...)
if err != nil {
grpcConnFailure := errors.IsGRPCUnavailable(err) || errors.IsGRPCDeadlineExceeded(err)
exitProcess := !constant.GetClientOpts().NoExitOnConnFailure
if grpcConnFailure && exitProcess {
log.Fatalf("Communication with cloud agent failed. Exiting Process with error: %+v\n", err)
}
}
return err
}
}

func init() {
connectionCache = map[string]*grpc.ClientConn{}
}
Expand Down Expand Up @@ -107,6 +124,7 @@ func getClientConnection(serverAddress *string, authorizer auth.Authorizer) (*gr
}

opts := getDefaultDialOption(authorizer)
opts = append(opts, grpc.WithUnaryInterceptor(clientConnOptionsInterceptor()))
conn, err := grpc.Dial(endpoint, opts...)
if err != nil {
log.Fatalf("Failed to dial: %v", err)
Expand Down
22 changes: 22 additions & 0 deletions pkg/constant/constant.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
package constant

import (
"sync"
"time"
)

Expand All @@ -12,3 +13,24 @@ const (
CertificateValidityThreshold float64 = (30.0 / 100.0)
RenewalBackoff float64 = (2.0 / 100.0)
)

type ClientOpts struct {
NoExitOnConnFailure bool
}

var clientOptsSingleton *ClientOpts
var once sync.Once

//If GetClientOpts called before SetClientOpts, it will return a default ClientOpts
func GetClientOpts() *ClientOpts {
once.Do(func() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't this cause an issue if set is called before get. The clientOptsSingleton would be nil

Copy link
Contributor

Choose a reason for hiding this comment

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

if once.Do(f) is called multiple times, only the first call will invoke f, even if f has a different value in each invocation. A new instance of Once is required for each function to execute.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So the expected behaviour is to call Set before Get, which the developer needs to ensure. In case if Set is not called and Get is invoked first, the default value of the NoExitOnConnFailure will be set to false as once.Do will execute once and set it once finally. In case Get is called before, the value will not be nil. As the NoExitOnConnFailure flag is set at the beginning of the mocctl process, I am ensuring that for mocctl the Set is being called before any Get operations occur

Copy link
Contributor

Choose a reason for hiding this comment

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

Cant we just check if clientOptsSingleton is not nil and return default if nil?

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 can do that as well, but we would need to take care of thread safety explicitly. I used once.Do because it is inherently thread safe. Do you want me to implement the nil checking logic?

clientOptsSingleton = &ClientOpts{}
})
return clientOptsSingleton
}

func SetClientOpts(clientOpts *ClientOpts) {
once.Do(func() {
clientOptsSingleton = clientOpts
})
}