Lock IAMClient requests when connection lost #186
Conversation
Signed-off-by: Mykola Kobets <mykola_kobets@epam.com>
Only grpc calls required to be locked, streams are wrapped by the corresponding clients Signed-off-by: Mykola Kobets <mykola_kobets@epam.com>
mykola-kobets-epam
force-pushed
the
iamclient-lock-requests-on-reconnection
branch
2 times, most recently
from
773ba70 to
9214832
Compare
| "google.golang.org/grpc" | ||
| ) | ||
|
|
||
| /*********************************************************************************************************************** |
There was a problem hiding this comment.
Are these empty sections (consts,types) needed?
There was a problem hiding this comment.
no, removed.
iamclient/iamcertprovider.go
Outdated
| * Public | ||
| **********************************************************************************************************************/ | ||
|
|
||
| // New creates new certificate provider instance. |
There was a problem hiding this comment.
| // New creates new certificate provider instance. | |
| // NewCertProvider creates new certificate provider instance. |
| @@ -34,7 +33,6 @@ import ( | |||
| // CertProvider certificate provider interface. | |||
| type CertProvider interface { | |||
| GetCertificate(certType string, issuer []byte, serial string) (certURL, keyURL string, err error) | |||
| SubscribeCertChanged(certType string) (<-chan *iamanager.CertInfo, error) | |||
There was a problem hiding this comment.
Why it is removed? Not required?
There was a problem hiding this comment.
yes, Interface used by Create(M)TLSCredentials, they dont require Subscribe method
| @@ -35,6 +35,7 @@ type GRPCConn struct { | |||
| sync.Mutex | |||
|
|
|||
| grpcConn *grpc.ClientConn | |||
| started bool | |||
| } | ||
| client.publicConnection.Set(publicConn) | ||
|
|
||
| defer func() { |
There was a problem hiding this comment.
Usually, no need to put good path into defer for readability reason.
| defer func() { | |
| defer func() { | |
| if err != nil { | |
| publicConn.Close() | |
| } | |
| } | |
| ... | |
| client.publicConnection.Start() |
There was a problem hiding this comment.
client.publicConnection.Start should be done in the end of the function execution to ensure connection succeded. Otherwise we unlock pending requests earlier & they can fail.
Current function beside error check has also
(client.isProtectedConnEnabled) logic, which means we should duplicate
client.publicConnection.Start() in
if (!client.isProtectedConnEnabled) {
}
and in the end of the function.
In order to avoid that I decided to add both cases to defer() function.
| if err != nil { | ||
| return aoserrors.Wrap(err) | ||
| } | ||
|
|
||
| client.protectedConnection.Set(protectedConn) | ||
|
|
||
| defer func() { |
There was a problem hiding this comment.
for unification maded the same way as for the public connection
Updating grpc connection shouldn't unlock requests. Signed-off-by: Mykola Kobets <mykola_kobets@epam.com>
Signed-off-by: Mykola Kobets <mykola_kobets@epam.com>
mykola-kobets-epam
force-pushed
the
iamclient-lock-requests-on-reconnection
branch
from
9214832 to
26bfefe
Compare
|
No description provided.