-
Notifications
You must be signed in to change notification settings - Fork 7
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
Renew cert fix #173
base: develop
Are you sure you want to change the base?
Renew cert fix #173
Conversation
Signed-off-by: Mykola Kobets <[email protected]>
Signed-off-by: Mykola Kobets <[email protected]>
Signed-off-by: Mykola Kobets <[email protected]>
e8d042b
to
9a84c96
Compare
Signed-off-by: Mykola Kobets <[email protected]>
Signed-off-by: Mykola Kobets <[email protected]>
9a84c96
to
694b9b6
Compare
Quality Gate passedIssues Measures |
} | ||
|
||
if err := controller.startGRPCServer(); err != nil { | ||
log.WithField("err", err).Error("SMController failed to start GRPC server") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SM controller
@@ -29,6 +29,8 @@ import ( | |||
"sync" | |||
"time" | |||
|
|||
"golang.org/x/exp/slices" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to duplicate module name in commit message.
enterPrefix + stateIdle: umCtrl.processIdleState, | ||
enterPrefix + stateReconnecting: umCtrl.processEnterReconnectingState, | ||
leavePrefix + stateReconnecting: umCtrl.processLeaveReconnectingState, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove empty line
"sync" | ||
"sync/atomic" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto for commit message
// GetNodeInfo returns node info. | ||
func (client *Client) GetNodeInfo(nodeID string) (nodeInfo cloudprotocol.NodeInfo, err error) { | ||
client.Lock() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
grpc calls are thread-safe why this lock is required here and in others API?
log.WithField("err", err).Error("CMServer failed to start GRPC server") | ||
|
||
server.restartTimer = time.AfterFunc(cmRestartInterval, func() { | ||
server.restartTimer = nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be guarded with mutex.
|
||
eventChannel chan umCtrlInternalMsg | ||
nodeInfoProvider NodeInfoProvider | ||
nodeInfoChannel <-chan cloudprotocol.NodeInfo | ||
stopChannel chan bool | ||
componentDir string | ||
certChan <-chan *iamanager.CertInfo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
certChannel
eventChannel: make(chan umCtrlInternalMsg), | ||
nodeInfoProvider: nodeInfoProvider, | ||
nodeInfoChannel: nodeInfoProvider.SubscribeNodeInfoChange(), | ||
certChan: make(<-chan *iamanager.CertInfo), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove make
if !insecure { | ||
if ch, err := localClient.SubscribeCertChanged(config.CertStorage); err != nil { | ||
return nil, aoserrors.Wrap(err) | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need else
after return.
|
||
secureOpt = grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig)) | ||
func (client *Client) reconnect() { | ||
if client.disableReconnect.CompareAndSwap(false, true) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is unclear why disableReconnect is required. Need to discuss.
No description provided.