Skip to content

Commit 3ec6188

Browse files
v0lkanv0lkan
authored
🌟 enhancement(VSecM Sentinel): refactored forever loops (#946)
updated infinite loops with early crashes instead Signed-off-by: Volkan Özçelik <[email protected]>
1 parent 77cbb17 commit 3ec6188

File tree

7 files changed

+92
-167
lines changed

7 files changed

+92
-167
lines changed

‎app/sentinel/background/initialization/connectivity.go‎

Lines changed: 49 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -13,94 +13,85 @@ package initialization
1313
import (
1414
"context"
1515
"github.com/pkg/errors"
16+
"github.com/spiffe/go-spiffe/v2/workloadapi"
1617
"time"
1718

1819
"github.com/vmware-tanzu/secrets-manager/app/sentinel/internal/safe"
1920
"github.com/vmware-tanzu/secrets-manager/core/backoff"
20-
"github.com/vmware-tanzu/secrets-manager/core/env"
2121
log "github.com/vmware-tanzu/secrets-manager/core/log/std"
2222
"github.com/vmware-tanzu/secrets-manager/core/spiffe"
2323
)
2424

2525
func ensureApiConnectivity(ctx context.Context, cid *string) {
26-
terminateAsap := env.TerminateSentinelOnInitCommandConnectivityFailure()
27-
2826
log.TraceLn(cid, "Before checking api connectivity")
2927

30-
for {
31-
s := backoffStrategy()
32-
33-
err := backoff.Retry("RunInitCommands:CheckConnectivity", func() error {
34-
log.TraceLn(cid, "RunInitCommands:CheckConnectivity: checking connectivity to safe")
28+
s := backoffStrategy()
29+
err := backoff.Retry("RunInitCommands:CheckConnectivity", func() error {
30+
log.TraceLn(cid, "RunInitCommands:CheckConnectivity: checking connectivity to safe")
3531

36-
src, acquired := spiffe.AcquireSourceForSentinel(ctx)
37-
if !acquired {
38-
log.TraceLn(cid, "RunInitCommands:CheckConnectivity: failed to acquire source.")
39-
if terminateAsap {
40-
panic("RunInitCommands:CheckConnectivity: failed to acquire source")
41-
}
32+
src, acquired := spiffe.AcquireSourceForSentinel(ctx)
33+
if !acquired {
34+
log.TraceLn(cid, "RunInitCommands:CheckConnectivity: failed to acquire source.")
4235

43-
return errors.New("RunInitCommands:CheckConnectivity: failed to acquire source")
44-
}
36+
return errors.New("RunInitCommands:CheckConnectivity: failed to acquire source")
37+
}
4538

46-
log.TraceLn(cid, "RunInitCommands:CheckConnectivity: acquired source successfully")
39+
log.TraceLn(cid, "RunInitCommands:CheckConnectivity: acquired source successfully")
4740

48-
if err := safe.Check(ctx, src); err != nil {
49-
log.TraceLn(cid, "RunInitCommands:CheckConnectivity: failed to verify connection to safe:", err.Error())
50-
if terminateAsap {
51-
panic("RunInitCommands:CheckConnectivity: failed to verify connection to safe")
52-
}
41+
if err := safe.Check(ctx, src); err != nil {
42+
log.TraceLn(cid, "RunInitCommands:CheckConnectivity: failed to verify connection to safe:", err.Error())
5343

54-
return errors.Wrap(err, "RunInitCommands:CheckConnectivity: cannot establish connection to safe 001")
55-
}
44+
return errors.Wrap(err, "RunInitCommands:CheckConnectivity: cannot establish connection to safe 001")
45+
}
5646

57-
log.TraceLn(cid, "RunInitCommands:CheckConnectivity: success")
58-
return nil
59-
}, s)
47+
log.TraceLn(cid, "RunInitCommands:CheckConnectivity: success")
48+
return nil
49+
}, s)
6050

61-
if err == nil {
62-
log.TraceLn(cid, "exiting backoffs")
63-
break
64-
}
51+
if err == nil {
52+
log.TraceLn(cid, "exiting backoffs")
53+
return
6554
}
55+
56+
// I shouldn't be here.
57+
panic("RunInitCommands:CheckConnectivity: failed to verify connection to safe")
6658
}
6759

68-
func ensureSourceAcquisition(ctx context.Context, cid *string) {
60+
func ensureSourceAcquisition(ctx context.Context, cid *string) *workloadapi.X509Source {
6961
// If `true`, instead of retrying with a backoff, kill the pod, and let the
7062
// deployment controller restart it to initiate a new retry.
71-
terminateAsap := env.TerminateSentinelOnInitCommandConnectivityFailure()
7263

73-
waitInterval := env.InitCommandRunnerWaitIntervalForSentinel()
74-
time.Sleep(waitInterval)
64+
log.TraceLn(cid, "RunInitCommands: acquiring source 001")
7565

76-
for {
77-
log.TraceLn(cid, "RunInitCommands: acquiring source 001")
66+
s := backoff.Strategy{
67+
MaxRetries: 20,
68+
Delay: 1000,
69+
Exponential: true,
70+
MaxDuration: 30 * time.Second,
71+
}
7872

79-
s := backoff.Strategy{
80-
MaxRetries: 20,
81-
Delay: 1000,
82-
Exponential: true,
83-
MaxDuration: 30 * time.Second,
84-
}
73+
var src *workloadapi.X509Source
8574

86-
err := backoff.Retry("RunInitCommands:AcquireSource", func() error {
87-
log.TraceLn(cid, "RunInitCommands:AcquireSource: acquireSourceForSentinel: 000")
88-
_, acquired := spiffe.AcquireSourceForSentinel(ctx)
89-
if !acquired {
90-
log.TraceLn(cid, "RunInitCommands:AcquireSource: failed to acquire source.")
91-
if terminateAsap {
92-
panic("RunInitCommands:AcquireSource: failed to acquire source")
93-
}
75+
err := backoff.Retry("RunInitCommands:AcquireSource", func() error {
76+
log.TraceLn(cid, "RunInitCommands:AcquireSource: acquireSourceForSentinel: 000")
9477

95-
return errors.New("RunInitCommands:AcquireSource: failed to acquire source 000")
96-
}
78+
acq, acquired := spiffe.AcquireSourceForSentinel(ctx)
79+
src = acq
9780

98-
return nil
99-
}, s)
81+
if !acquired {
82+
log.TraceLn(cid, "RunInitCommands:AcquireSource: failed to acquire source.")
10083

101-
if err == nil {
102-
log.TraceLn(cid, "RunInitCommands:AcquireSource: got source. breaking.")
103-
break
84+
return errors.New("RunInitCommands:AcquireSource: failed to acquire source 000")
10485
}
86+
87+
return nil
88+
}, s)
89+
90+
if err == nil {
91+
log.TraceLn(cid, "RunInitCommands:AcquireSource: got source. breaking.")
92+
return src
10593
}
94+
95+
// I shouldn't be here.
96+
panic("RunInitCommands:AcquireSource: failed to acquire source")
10697
}

‎app/sentinel/background/initialization/io.go‎

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,9 @@ func parseCommandsFile(ctx context.Context, cid *string, scanner *bufio.Scanner)
4545
log.TraceLn(cid, "Before parsing commands 002")
4646

4747
sc := entity.SentinelCommand{}
48-
terminateAsap := env.TerminateSentinelOnInitCommandConnectivityFailure()
4948

5049
if scanner == nil {
51-
if terminateAsap {
52-
log.ErrorLn(cid, "RunInitCommands: error scanning commands file")
53-
panic("RunInitCommands: error scanning commands file")
54-
}
55-
56-
return
50+
panic("RunInitCommands: error scanning commands file")
5751
}
5852

5953
log.TraceLn(cid, "beginning scan")
@@ -97,10 +91,8 @@ dance:
9791
"RunInitCommands:ProcessCommandBlock:error:",
9892
err.Error(),
9993
)
100-
if terminateAsap {
101-
panic("RunInitCommands:ProcessCommandBlock failed")
102-
}
10394
}
95+
10496
return err
10597
}, s)
10698

@@ -110,9 +102,10 @@ dance:
110102
"RunInitCommands: error processing command block: ",
111103
err.Error(),
112104
)
113-
if terminateAsap {
114-
panic("RunInitCommands: error processing command block")
115-
}
105+
106+
// If command failed, then the initialization is not totally successful.
107+
// Thus, it is best to crash the container to restart the initialization.
108+
panic("RunInitCommands:ProcessCommandBlock failed")
116109
}
117110

118111
log.TraceLn(cid, "scanner: after delimiter")
@@ -181,8 +174,9 @@ dance:
181174
"RunInitCommands: Error reading initialization file: ",
182175
err.Error(),
183176
)
184-
if terminateAsap {
185-
panic("RunInitCommands: Error reading initialization file")
186-
}
177+
178+
// If command failed, then the initialization is not totally successful.
179+
// Thus, it is best to crash the container to restart the initialization.
180+
panic("RunInitCommands: Error in scanning the file")
187181
}
188182
}

‎app/sentinel/background/initialization/keystone.go‎

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,10 @@ import (
1515

1616
"github.com/vmware-tanzu/secrets-manager/core/backoff"
1717
entity "github.com/vmware-tanzu/secrets-manager/core/entity/data/v1"
18-
"github.com/vmware-tanzu/secrets-manager/core/env"
1918
log "github.com/vmware-tanzu/secrets-manager/core/log/std"
2019
)
2120

2221
func markKeystone(ctx context.Context, cid *string) bool {
23-
terminateAsap := env.TerminateSentinelOnInitCommandConnectivityFailure()
24-
2522
s := backoffStrategy()
2623
err := backoff.Retry("RunInitCommands:MarkKeystone", func() error {
2724
log.TraceLn(cid, "RunInitCommands:MarkKeystone: retrying with exponential backoff")
@@ -33,22 +30,13 @@ func markKeystone(ctx context.Context, cid *string) bool {
3330
Secret: "keystone-init",
3431
})
3532

36-
if err != nil {
37-
if terminateAsap {
38-
panic("RunInitCommands: error setting keystone secret")
39-
}
40-
}
41-
4233
return err
4334
}, s)
4435

45-
if err != nil {
46-
log.ErrorLn(cid, "RunInitCommands: error setting keystone secret: ", err.Error())
47-
if terminateAsap {
48-
panic("RunInitCommands: error setting keystone secret")
49-
}
50-
return false
36+
if err == nil {
37+
return true
5138
}
5239

53-
return true
40+
log.ErrorLn(cid, "RunInitCommands: error setting keystone secret: ", err.Error())
41+
panic("RunInitCommands: error setting keystone secret")
5442
}

‎app/sentinel/background/initialization/run.go‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ package initialization
1212

1313
import (
1414
"context"
15-
"github.com/spiffe/go-spiffe/v2/workloadapi"
1615
"os"
1716
"time"
1817

@@ -47,9 +46,19 @@ import (
4746
// If the file cannot be opened, the function logs an informational message and
4847
// returns early. Errors encountered while reading the file or closing it are
4948
// logged as errors.
50-
func RunInitCommands(ctx context.Context, source *workloadapi.X509Source) {
49+
func RunInitCommands(ctx context.Context) {
5150
cid := ctx.Value("correlationId").(*string)
5251

52+
waitInterval := env.InitCommandRunnerWaitBeforeExecIntervalForSentinel()
53+
time.Sleep(waitInterval)
54+
55+
// Ensure that we can acquire a source before proceeding.
56+
source := ensureSourceAcquisition(ctx, cid)
57+
58+
// Now, we are sure that we can acquire a source.
59+
// Try to do a VSecM Safe API request with the source.
60+
ensureApiConnectivity(ctx, cid)
61+
5362
// No need to proceed if initialization has been completed already.
5463
if initCommandsExecutedAlready(ctx, source) {
5564
log.TraceLn(cid, "RunInitCommands: executed already. exiting")
@@ -58,12 +67,6 @@ func RunInitCommands(ctx context.Context, source *workloadapi.X509Source) {
5867

5968
log.TraceLn(cid, "RunInitCommands: starting the init flow")
6069

61-
// Ensure that we can acquire a source before proceeding.
62-
ensureSourceAcquisition(ctx, cid)
63-
// Now, we are sure that we can acquire a source.
64-
// Try to do a VSecM Safe API request with the source.
65-
ensureApiConnectivity(ctx, cid)
66-
6770
// Now we know that we can establish a connection to VSecM Safe
6871
// and execute API requests. So, we can safely run init commands.
6972

@@ -100,13 +103,14 @@ func RunInitCommands(ctx context.Context, source *workloadapi.X509Source) {
100103
if !success {
101104
log.TraceLn(cid, "RunInitCommands: failed to mark keystone. exiting")
102105

103-
// If we cannot set the keystone secret, we should not proceed.
106+
// If we cannot set the keystone secret, better to retry everything.
107+
panic("RunInitCommands: failed to set keystone secret")
104108
return
105109
}
106110

107111
// Wait before notifying Keystone. This way, if there are things that
108112
// take time to reconcile, they have a chance to do so.
109-
waitInterval := env.InitCommandRunnerWaitIntervalBeforeInitComplete()
113+
waitInterval = env.InitCommandRunnerWaitIntervalBeforeInitComplete()
110114
time.Sleep(waitInterval)
111115

112116
// Everything is set up.

‎app/sentinel/background/initialization/validation.go‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,24 +23,24 @@ func initCommandsExecutedAlready(ctx context.Context, src *workloadapi.X509Sourc
2323

2424
log.TraceLn(cid, "check:initCommandsExecutedAlready")
2525

26-
s := backoffStrategy()
2726
initialized := false
28-
for {
29-
err := backoff.Retry("RunInitCommands:CheckConnectivity", func() error {
30-
i, err := safe.CheckInitialization(ctx, src)
31-
if err != nil {
32-
return err
33-
}
34-
initialized = i
35-
return nil
36-
}, s)
3727

28+
s := backoffStrategy()
29+
err := backoff.Retry("RunInitCommands:CheckConnectivity", func() error {
30+
i, err := safe.CheckInitialization(ctx, src)
3831
if err != nil {
39-
log.ErrorLn(cid, "check:backoff:error", err.Error())
40-
continue
32+
return err
4133
}
4234

43-
log.TraceLn(cid, "check:return initialized:", initialized)
35+
initialized = i
36+
37+
return nil
38+
}, s)
39+
40+
if err == nil {
4441
return initialized
4542
}
43+
44+
// I shouldn't be here.
45+
panic("RunInitCommands:initCommandsExecutedAlready: failed to check command initialization")
4646
}

0 commit comments

Comments
 (0)