Skip to content

Commit 50e0d1c

Browse files
ldmonsterldmonster
authored
[chore] reduce coupling to global vars (#862)
Signed-off-by: Pavel Okhlopkov <pavel.okhlopkov@flant.com>
1 parent 37b7c71 commit 50e0d1c

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

47 files changed

+768
-394
lines changed

‎pkg/app/debug.go‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,7 @@ var DebugUnixSocket = "/var/run/shell-operator/debug.socket"
1010

1111
var DebugHttpServerAddr = ""
1212

13-
var (
14-
DebugKeepTmpFilesVar = "no"
15-
DebugKeepTmpFiles = false
16-
)
13+
var DebugKeepTmpFiles = false
1714

1815
var DebugKubernetesAPI = false
1916

@@ -27,11 +24,11 @@ func DefineDebugFlags(kpApp *kingpin.Application, cmd *kingpin.CmdClause) {
2724
Default(DebugHttpServerAddr).
2825
StringVar(&DebugHttpServerAddr)
2926

30-
cmd.Flag("debug-keep-tmp-files", "set to yes to disable cleanup of temporary files").
27+
cmd.Flag("debug-keep-tmp-files", "set to true to disable cleanup of temporary files").
3128
Envar("DEBUG_KEEP_TMP_FILES").
3229
Hidden().
33-
Default(DebugKeepTmpFilesVar).
34-
StringVar(&DebugKeepTmpFilesVar)
30+
Default("false").
31+
BoolVar(&DebugKeepTmpFiles)
3532

3633
cmd.Flag("debug-kubernetes-api", "enable client-go debug messages").
3734
Envar("DEBUG_KUBERNETES_API").

‎pkg/app/debug_test.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package app
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
)
8+
9+
func TestDebugKeepTmpFiles_DefaultIsFalse(t *testing.T) {
10+
assert.False(t, DebugKeepTmpFiles, "DebugKeepTmpFiles should default to false")
11+
}
12+
13+
// TestDebugKeepTmpFilesIsBool verifies the variable is the bool type
14+
// and that simple assignment behaves correctly (no string comparison needed).
15+
func TestDebugKeepTmpFilesIsBool(t *testing.T) {
16+
saved := DebugKeepTmpFiles
17+
defer func() { DebugKeepTmpFiles = saved }()
18+
19+
DebugKeepTmpFiles = true
20+
assert.True(t, DebugKeepTmpFiles)
21+
22+
DebugKeepTmpFiles = false
23+
assert.False(t, DebugKeepTmpFiles)
24+
}

‎pkg/config/config.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99
"time"
1010

1111
"github.com/deckhouse/deckhouse/pkg/log"
12+
13+
pkg "github.com/flant/shell-operator/pkg"
1214
)
1315

1416
/**
@@ -52,7 +54,7 @@ func NewConfig(logger *log.Logger) *Config {
5254
values: make(map[string]string),
5355
temporalValues: make(map[string]*TemporalValue),
5456
errors: make(map[string]error),
55-
logger: logger.With(slog.String("component", "runtimeConfig")),
57+
logger: logger.With(slog.String(pkg.LogKeyComponent, "runtimeConfig")),
5658
}
5759
}
5860

@@ -261,7 +263,7 @@ func (c *Config) expireOverrides() {
261263

262264
for _, expire := range expires {
263265
name, oldValue, newValue := expire[0], expire[1], expire[2]
264-
c.logger.Debug("Parameter is expired", slog.String("parameter", name))
266+
c.logger.Debug("Parameter is expired", slog.String(pkg.LogKeyParameter, name))
265267
c.callOnChange(name, oldValue, newValue)
266268
}
267269
}
@@ -274,7 +276,7 @@ func (c *Config) callOnChange(name string, oldValue string, newValue string) {
274276
err := c.params[name].onChange(oldValue, newValue)
275277
if err != nil {
276278
c.logger.Error("OnChange handler failed for parameter during value change values",
277-
slog.String("parameter", name), slog.String("old_value", oldValue), slog.String("new_value", newValue), log.Err(err))
279+
slog.String(pkg.LogKeyParameter, name), slog.String(pkg.LogKeyOldValue, oldValue), slog.String(pkg.LogKeyNewValue, newValue), log.Err(err))
278280
}
279281
c.m.Lock()
280282
delete(c.errors, name)

‎pkg/debug/server.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"github.com/go-chi/chi/v5/middleware"
1818
"gopkg.in/yaml.v3"
1919

20+
pkg "github.com/flant/shell-operator/pkg"
2021
utils "github.com/flant/shell-operator/pkg/utils/file"
2122
structuredLogger "github.com/flant/shell-operator/pkg/utils/structured-logger"
2223
)
@@ -70,7 +71,7 @@ func (s *Server) Init() error {
7071
return fmt.Errorf("Debug HTTP server fail to listen on '%s': %w", address, err)
7172
}
7273

73-
s.logger.Info("Debug endpoint listen on address", slog.String("address", address))
74+
s.logger.Info("Debug endpoint listen on address", slog.String(pkg.LogKeyAddress, address))
7475

7576
go func() {
7677
if err := http.Serve(listener, s.Router); err != nil {
@@ -145,7 +146,7 @@ func handleFormattedOutput(writer http.ResponseWriter, request *http.Request, ha
145146
format = strings.TrimPrefix(format, ".")
146147
}
147148

148-
structuredLogger.GetLogEntry(request).Debug("used format", slog.String("format", format))
149+
structuredLogger.GetLogEntry(request).Debug("used format", slog.String(pkg.LogKeyFormat, format))
149150

150151
switch format {
151152
case "text":

‎pkg/executor/executor.go‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/deckhouse/deckhouse/pkg/log"
1717
"go.opentelemetry.io/otel"
1818

19+
pkg "github.com/flant/shell-operator/pkg"
1920
utils "github.com/flant/shell-operator/pkg/utils/labels"
2021
)
2122

@@ -26,7 +27,7 @@ const (
2627
func Run(cmd *exec.Cmd) error {
2728
// TODO context: hook name, hook phase, hook binding
2829
// TODO observability
29-
log.Debug("Executing command", slog.String("command", strings.Join(cmd.Args, " ")), slog.String("dir", cmd.Dir))
30+
log.Debug("Executing command", slog.String(pkg.LogKeyCommand, strings.Join(cmd.Args, " ")), slog.String(pkg.LogKeyDir, cmd.Dir))
3031

3132
return cmd.Run()
3233
}
@@ -100,8 +101,8 @@ func NewExecutor(dir string, entrypoint string, args []string, envs []string) *E
100101

101102
func (e *Executor) Output() ([]byte, error) {
102103
e.logger.Debug("Executing command",
103-
slog.String("command", strings.Join(e.cmd.Args, " ")),
104-
slog.String("dir", e.cmd.Dir))
104+
slog.String(pkg.LogKeyCommand, strings.Join(e.cmd.Args, " ")),
105+
slog.String(pkg.LogKeyDir, e.cmd.Dir))
105106
return e.cmd.Output()
106107
}
107108

@@ -117,12 +118,12 @@ func (e *Executor) RunAndLogLines(ctx context.Context, logLabels map[string]stri
117118

118119
stdErr := bytes.NewBuffer(nil)
119120
logEntry := utils.EnrichLoggerWithLabels(e.logger, logLabels)
120-
stdoutLogEntry := logEntry.With("output", "stdout")
121-
stderrLogEntry := logEntry.With("output", "stderr")
121+
stdoutLogEntry := logEntry.With(pkg.LogKeyOutput, "stdout")
122+
stderrLogEntry := logEntry.With(pkg.LogKeyOutput, "stderr")
122123

123124
log.Debug("Executing command",
124-
slog.String("command", strings.Join(e.cmd.Args, " ")),
125-
slog.String("dir", e.cmd.Dir))
125+
slog.String(pkg.LogKeyCommand, strings.Join(e.cmd.Args, " ")),
126+
slog.String(pkg.LogKeyDir, e.cmd.Dir))
126127

127128
plo := &proxyLogger{
128129
ctx: ctx,
@@ -208,7 +209,7 @@ func (pl *proxyLogger) Write(p []byte) (int, error) {
208209
}()
209210

210211
if !ok {
211-
pl.logger.Debug("json log line not map[string]interface{}", slog.Any("line", line))
212+
pl.logger.Debug("json log line not map[string]interface{}", slog.Any(pkg.LogKeyLine, line))
212213

213214
// fall back to using the logger
214215
pl.logger.Info(string(p))
@@ -308,7 +309,7 @@ func (pl *proxyLogger) mergeAndLogInputLog(ctx context.Context, inputLog map[str
308309
if len(logLine) > 10000 {
309310
logLine = fmt.Sprintf("%s:truncated", logLine[:10000])
310311

311-
logger.Log(ctx, lvl.Level(), msg, slog.Any("hook", map[string]any{
312+
logger.Log(ctx, lvl.Level(), msg, slog.Any(pkg.LogKeyHook, map[string]any{
312313
"truncated": logLine,
313314
}))
314315

‎pkg/hook/controller/admission_bindings_controller.go‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"github.com/deckhouse/deckhouse/pkg/log"
88
v1 "k8s.io/api/admission/v1"
99

10+
pkg "github.com/flant/shell-operator/pkg"
1011
bctx "github.com/flant/shell-operator/pkg/hook/binding_context"
1112
htypes "github.com/flant/shell-operator/pkg/hook/types"
1213
"github.com/flant/shell-operator/pkg/webhook/admission"
@@ -71,8 +72,8 @@ func (c *AdmissionBindingsController) EnableValidatingBindings() {
7172
}
7273
if config.Webhook.Metadata.ConfigurationId != confId {
7374
log.Error("Possible bug!!! kubernetesValidating has non-unique configurationIds",
74-
slog.String("configurationID", config.Webhook.Metadata.ConfigurationId),
75-
slog.String("confID", confId))
75+
slog.String(pkg.LogKeyConfigurationID, config.Webhook.Metadata.ConfigurationId),
76+
slog.String(pkg.LogKeyConfID, confId))
7677
}
7778
}
7879
c.ConfigurationId = confId
@@ -107,8 +108,8 @@ func (c *AdmissionBindingsController) EnableMutatingBindings() {
107108
}
108109
if config.Webhook.Metadata.ConfigurationId != confId {
109110
log.Error("Possible bug!!! kubernetesMutating has non-unique configurationIds",
110-
slog.String("configurationID", config.Webhook.Metadata.ConfigurationId),
111-
slog.String("confID", confId))
111+
slog.String(pkg.LogKeyConfigurationID, config.Webhook.Metadata.ConfigurationId),
112+
slog.String(pkg.LogKeyConfID, confId))
112113
}
113114
}
114115
c.ConfigurationId = confId
@@ -145,8 +146,8 @@ func (c *AdmissionBindingsController) CanHandleEvent(event admission.Event) bool
145146
func (c *AdmissionBindingsController) HandleEvent(_ context.Context, event admission.Event) BindingExecutionInfo {
146147
if c.ConfigurationId != event.ConfigurationId {
147148
log.Error("Possible bug!!! Unknown validating event: no binding for configurationId",
148-
slog.String("configurationId", event.ConfigurationId),
149-
slog.String("webhookId", event.WebhookId))
149+
slog.String(pkg.LogKeyConfigId, event.ConfigurationId),
150+
slog.String(pkg.LogKeyWebhookId, event.WebhookId))
150151
return BindingExecutionInfo{
151152
BindingContext: []bctx.BindingContext{},
152153
AllowFailure: false,
@@ -156,8 +157,8 @@ func (c *AdmissionBindingsController) HandleEvent(_ context.Context, event admis
156157
link, hasKey := c.AdmissionLinks[event.WebhookId]
157158
if !hasKey {
158159
log.Error("Possible bug!!! Unknown validating event: no binding for configurationId",
159-
slog.String("configurationId", event.ConfigurationId),
160-
slog.String("webhookId", event.WebhookId))
160+
slog.String(pkg.LogKeyConfigId, event.ConfigurationId),
161+
slog.String(pkg.LogKeyWebhookId, event.WebhookId))
161162
return BindingExecutionInfo{
162163
BindingContext: []bctx.BindingContext{},
163164
AllowFailure: false,

‎pkg/hook/controller/conversion_bindings_controller.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"github.com/deckhouse/deckhouse/pkg/log"
88
v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
99

10+
pkg "github.com/flant/shell-operator/pkg"
1011
bctx "github.com/flant/shell-operator/pkg/hook/binding_context"
1112
htypes "github.com/flant/shell-operator/pkg/hook/types"
1213
"github.com/flant/shell-operator/pkg/webhook/conversion"
@@ -64,7 +65,7 @@ func (c *ConversionBindingsController) EnableConversionBindings() {
6465
}
6566
}
6667
log.Info("conversion binding controller: add webhook from config",
67-
slog.String("name", config.Webhook.Metadata.Name))
68+
slog.String(pkg.LogKeyName, config.Webhook.Metadata.Name))
6869
c.webhookManager.AddWebhook(config.Webhook)
6970
}
7071
}
@@ -85,7 +86,7 @@ func (c *ConversionBindingsController) CanHandleEvent(crdName string, _ *v1.Conv
8586
func (c *ConversionBindingsController) HandleEvent(_ context.Context, crdName string, request *v1.ConversionRequest, rule conversion.Rule) BindingExecutionInfo {
8687
_, hasKey := c.Links[crdName]
8788
if !hasKey {
88-
log.Error("Possible bug!!! No binding for conversion event for crd", slog.String("crd", crdName))
89+
log.Error("Possible bug!!! No binding for conversion event for crd", slog.String(pkg.LogKeyCRD, crdName))
8990
return BindingExecutionInfo{
9091
BindingContext: []bctx.BindingContext{},
9192
AllowFailure: false,
@@ -94,8 +95,8 @@ func (c *ConversionBindingsController) HandleEvent(_ context.Context, crdName st
9495
link, has := c.Links[crdName][rule]
9596
if !has {
9697
log.Error("Possible bug!!! Event has an unknown conversion rule: no binding was registered",
97-
slog.String("rule", rule.String()),
98-
slog.String("crd", crdName))
98+
slog.String(pkg.LogKeyRule, rule.String()),
99+
slog.String(pkg.LogKeyCRD, crdName))
99100
return BindingExecutionInfo{
100101
BindingContext: []bctx.BindingContext{},
101102
AllowFailure: false,

‎pkg/hook/controller/kubernetes_bindings_controller.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88

99
"github.com/deckhouse/deckhouse/pkg/log"
1010

11+
pkg "github.com/flant/shell-operator/pkg"
1112
bctx "github.com/flant/shell-operator/pkg/hook/binding_context"
1213
htypes "github.com/flant/shell-operator/pkg/hook/types"
1314
kubeeventsmanager "github.com/flant/shell-operator/pkg/kube_events_manager"
@@ -132,9 +133,9 @@ func (c *kubernetesBindingsController) UpdateMonitor(monitorId string, kind, api
132133

133134
utils.EnrichLoggerWithLabels(c.logger, link.BindingConfig.Monitor.Metadata.LogLabels).
134135
Info("Monitor is recreated",
135-
slog.String("bindingName", link.BindingConfig.BindingName),
136-
slog.String("kind", link.BindingConfig.Monitor.Kind),
137-
slog.String("apiVersion", link.BindingConfig.Monitor.ApiVersion))
136+
slog.String(pkg.LogKeyBindingName, link.BindingConfig.BindingName),
137+
slog.String(pkg.LogKeyKind, link.BindingConfig.Monitor.Kind),
138+
slog.String(pkg.LogKeyAPIVersion, link.BindingConfig.Monitor.ApiVersion))
138139

139140
// Synchronization has no meaning for UpdateMonitor. Just emit Added event to handle objects of
140141
// a new kind.
@@ -166,7 +167,7 @@ func (c *kubernetesBindingsController) UnlockEvents() {
166167
func (c *kubernetesBindingsController) UnlockEventsFor(monitorID string) {
167168
m := c.kubeEventsManager.GetMonitor(monitorID)
168169
if m == nil {
169-
log.Warn("monitor was not found", slog.String("monitorID", monitorID))
170+
log.Warn("monitor was not found", slog.String(pkg.LogKeyMonitorID, monitorID))
170171
return
171172
}
172173
m.EnableKubeEventCb()
@@ -223,7 +224,7 @@ func (c *kubernetesBindingsController) setBindingMonitorLinks(monitorId string,
223224
func (c *kubernetesBindingsController) HandleEvent(_ context.Context, kubeEvent kemtypes.KubeEvent) BindingExecutionInfo {
224225
link, hasKey := c.getBindingMonitorLinksById(kubeEvent.MonitorId)
225226
if !hasKey {
226-
log.Error("Possible bug!!! Unknown kube event: no such monitor id registered", slog.String("monitorID", kubeEvent.MonitorId))
227+
log.Error("Possible bug!!! Unknown kube event: no such monitor id registered", slog.String(pkg.LogKeyMonitorID, kubeEvent.MonitorId))
227228
return BindingExecutionInfo{
228229
BindingContext: []bctx.BindingContext{},
229230
AllowFailure: false,

‎pkg/hook/hook.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616
"go.opentelemetry.io/otel/attribute"
1717
"golang.org/x/time/rate"
1818

19-
"github.com/flant/shell-operator/pkg/app"
2019
"github.com/flant/shell-operator/pkg/executor"
2120
bctx "github.com/flant/shell-operator/pkg/hook/binding_context"
2221
"github.com/flant/shell-operator/pkg/hook/config"
@@ -139,7 +138,7 @@ func (h *Hook) Run(ctx context.Context, _ htypes.BindingType, context []bctx.Bin
139138

140139
// remove tmp file on hook exit
141140
defer func() {
142-
if app.DebugKeepTmpFilesVar != "yes" {
141+
if !h.KeepTemporaryHookFiles {
143142
_ = os.Remove(contextPath)
144143
_ = os.Remove(metricsPath)
145144
_ = os.Remove(conversionPath)

0 commit comments

Comments
 (0)