Skip to content

Commit 6320625

Browse files
committed
fix: remove log objects due to potential for dropped log lines; correctly handle error (#913)
Signed-off-by: Julie Vogelman <julie_vogelman@intuit.com>
1 parent 872ca9e commit 6320625

File tree

2 files changed

+1
-33
lines changed

2 files changed

+1
-33
lines changed

internal/controller/config/config.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,9 +131,6 @@ type ProgressiveConfig struct {
131131

132132
// timeout duration which AnalysisRuns cannot continue to run after
133133
AnalysisRunTimeout string `json:"analysisRunTimeout" mapstructure:"analysisRunTimeout"`
134-
135-
// when set to true, adds the rollout, promoted, and upgrading objects to all the log messages during progressive upgrade
136-
LogObjects bool `json:"logObjects" mapstructure:"logObjects"`
137134
}
138135

139136
// DefaultAssessmentSchedule defines a default schedule for each Kind

internal/controller/progressive/progressive.go

Lines changed: 1 addition & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -124,21 +124,6 @@ func ProcessResource(
124124
c client.Client,
125125
) (bool, time.Duration, error) {
126126

127-
// Log progressive objects if the related flag is enabled
128-
logObjects, err := getConfigLogObjects()
129-
if err != nil {
130-
return false, 0, err
131-
}
132-
if logObjects {
133-
// Add the rolloutObject and promotedChild to the logger in context and set the name
134-
progressiveLogger := logger.FromContext(ctx).WithName("ProgressiveUpgrade").
135-
WithValues(
136-
"rolloutObject", rolloutObject,
137-
"promotedChild", existingPromotedChild,
138-
)
139-
ctx = logger.WithLogger(ctx, progressiveLogger)
140-
}
141-
142127
// Make sure that our Promoted Child Status reflects the current promoted child
143128
promotedChildStatus := rolloutObject.GetPromotedChildStatus()
144129
if promotedChildStatus == nil || promotedChildStatus.Name != existingPromotedChild.GetName() {
@@ -173,11 +158,6 @@ func ProcessResource(
173158

174159
// There's already an Upgrading child, now process it
175160

176-
// Add the upgradingChild to the logger in context (only if the related flag is enabled)
177-
if logObjects {
178-
ctx = logger.WithLogger(ctx, logger.FromContext(ctx).WithValues("upgradingChild", currentUpgradingChildDef))
179-
}
180-
181161
// get UpgradingChildStatus and reset it if necessary
182162
childStatus, err := getUpgradingChildStatus(ctx, rolloutObject, currentUpgradingChildDef)
183163
if err != nil {
@@ -197,7 +177,7 @@ func ProcessResource(
197177

198178
// determine if we need to replace the Upgrading child with a newer one
199179
needsRequeue, done, err := checkForUpgradeReplacement(ctx, rolloutObject, controller, existingPromotedChild, currentUpgradingChildDef, c)
200-
if needsRequeue {
180+
if needsRequeue || err != nil {
201181
return false, common.DefaultRequeueDelay, err
202182
}
203183
if done {
@@ -308,15 +288,6 @@ func getChildStatusAssessmentSchedule(
308288
return schedule, nil
309289
}
310290

311-
func getConfigLogObjects() (bool, error) {
312-
globalConfig, err := config.GetConfigManagerInstance().GetConfig()
313-
if err != nil {
314-
return false, fmt.Errorf("error getting the global config to retrieve logObjects: %w", err)
315-
}
316-
317-
return globalConfig.Progressive.LogObjects, nil
318-
}
319-
320291
/*
321292
processUpgradingChild handles the assessment and potential update of a child resource during a progressive upgrade.
322293
It evaluates the current status of the upgrading child, determines if an assessment is needed, and processes the

0 commit comments

Comments
 (0)