Skip to content
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

chore: add logging context around replicaset updates #3326

Merged
merged 2 commits into from
Jan 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions rollout/bluegreen.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package rollout

import (
"fmt"
"math"
"sort"

Expand All @@ -22,7 +23,7 @@
}
c.newRS, err = c.getAllReplicaSetsAndSyncRevision(true)
if err != nil {
return err
return fmt.Errorf("failed to getAllReplicaSetsAndSyncRevision in rolloutBlueGreen create true: %w", err)

Check warning on line 26 in rollout/bluegreen.go

View check run for this annotation

Codecov / codecov/patch

rollout/bluegreen.go#L26

Added line #L26 was not covered by tests
}

// This must happen right after the new replicaset is created
Expand Down Expand Up @@ -82,6 +83,9 @@

c.log.Infof("Reconciling stable ReplicaSet '%s'", activeRS.Name)
_, _, err := c.scaleReplicaSetAndRecordEvent(activeRS, defaults.GetReplicasOrDefault(c.rollout.Spec.Replicas))
if err != nil {
return fmt.Errorf("failed to scaleReplicaSetAndRecordEvent in reconcileBlueGreenStableReplicaSet: %w", err)
}

Check warning on line 88 in rollout/bluegreen.go

View check run for this annotation

Codecov / codecov/patch

rollout/bluegreen.go#L87-L88

Added lines #L87 - L88 were not covered by tests
return err
}

Expand Down Expand Up @@ -243,7 +247,7 @@
// Scale down.
_, _, err = c.scaleReplicaSetAndRecordEvent(targetRS, desiredReplicaCount)
if err != nil {
return false, err
return false, fmt.Errorf("failed to scaleReplicaSetAndRecordEvent in scaleDownOldReplicaSetsForBlueGreen: %w", err)

Check warning on line 250 in rollout/bluegreen.go

View check run for this annotation

Codecov / codecov/patch

rollout/bluegreen.go#L250

Added line #L250 was not covered by tests
}
hasScaled = true
}
Expand Down
10 changes: 7 additions & 3 deletions rollout/canary.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package rollout

import (
"fmt"
"sort"

appsv1 "k8s.io/api/apps/v1"
Expand All @@ -21,14 +22,14 @@
if replicasetutil.PodTemplateOrStepsChanged(c.rollout, c.newRS) {
c.newRS, err = c.getAllReplicaSetsAndSyncRevision(false)
if err != nil {
return err
return fmt.Errorf("failed to getAllReplicaSetsAndSyncRevision in rolloutCanary with PodTemplateOrStepsChanged: %w", err)

Check warning on line 25 in rollout/canary.go

View check run for this annotation

Codecov / codecov/patch

rollout/canary.go#L25

Added line #L25 was not covered by tests
}
return c.syncRolloutStatusCanary()
}

c.newRS, err = c.getAllReplicaSetsAndSyncRevision(true)
if err != nil {
return err
return fmt.Errorf("failed to getAllReplicaSetsAndSyncRevision in rolloutCanary create true: %w", err)

Check warning on line 32 in rollout/canary.go

View check run for this annotation

Codecov / codecov/patch

rollout/canary.go#L32

Added line #L32 was not covered by tests
}

err = c.podRestarter.Reconcile(c)
Expand Down Expand Up @@ -110,6 +111,9 @@
_, desiredStableRSReplicaCount = replicasetutil.CalculateReplicaCountsForTrafficRoutedCanary(c.rollout, c.rollout.Status.Canary.Weights)
}
scaled, _, err := c.scaleReplicaSetAndRecordEvent(c.stableRS, desiredStableRSReplicaCount)
if err != nil {
return scaled, fmt.Errorf("failed to scaleReplicaSetAndRecordEvent in reconcileCanaryStableReplicaSet:L %w", err)
}

Check warning on line 116 in rollout/canary.go

View check run for this annotation

Codecov / codecov/patch

rollout/canary.go#L115-L116

Added lines #L115 - L116 were not covered by tests
return scaled, err
}

Expand Down Expand Up @@ -230,7 +234,7 @@
// Scale down.
_, _, err = c.scaleReplicaSetAndRecordEvent(targetRS, desiredReplicaCount)
if err != nil {
return totalScaledDown, err
return totalScaledDown, fmt.Errorf("failed to scaleReplicaSetAndRecordEvent in scaleDownOldReplicaSetsForCanary: %w", err)

Check warning on line 237 in rollout/canary.go

View check run for this annotation

Codecov / codecov/patch

rollout/canary.go#L237

Added line #L237 was not covered by tests
}
scaleDownCount := *targetRS.Spec.Replicas - desiredReplicaCount
maxScaleDown -= scaleDownCount
Expand Down
4 changes: 2 additions & 2 deletions rollout/replicaset.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@
scaled, _, err := c.scaleReplicaSetAndRecordEvent(c.newRS, newReplicasCount)

if err != nil {
return scaled, err
return scaled, fmt.Errorf("failed to scaleReplicaSetAndRecordEvent in reconcileNewReplicaSet: %w", err)

Check warning on line 177 in rollout/replicaset.go

View check run for this annotation

Codecov / codecov/patch

rollout/replicaset.go#L177

Added line #L177 was not covered by tests
}

revision, _ := replicasetutil.Revision(c.newRS)
Expand Down Expand Up @@ -279,7 +279,7 @@
}
_, updatedOldRS, err := c.scaleReplicaSetAndRecordEvent(targetRS, newReplicasCount)
if err != nil {
return nil, totalScaledDown, err
return nil, totalScaledDown, fmt.Errorf("failed to scaleReplicaSetAndRecordEvent in cleanupUnhealthyReplicas: %w", err)

Check warning on line 282 in rollout/replicaset.go

View check run for this annotation

Codecov / codecov/patch

rollout/replicaset.go#L282

Added line #L282 was not covered by tests
}
totalScaledDown += scaledDownCount
oldRSs[i] = updatedOldRS
Expand Down
11 changes: 7 additions & 4 deletions rollout/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@
var err error
c.newRS, err = c.getAllReplicaSetsAndSyncRevision(false)
if err != nil {
return err
return fmt.Errorf("failed to getAllReplicaSetsAndSyncRevision in syncReplicasOnly: %w", err)

Check warning on line 286 in rollout/sync.go

View check run for this annotation

Codecov / codecov/patch

rollout/sync.go#L286

Added line #L286 was not covered by tests
}
newStatus := c.rollout.Status.DeepCopy()

Expand All @@ -296,7 +296,7 @@
if err := c.reconcileBlueGreenReplicaSets(activeSvc); err != nil {
// If we get an error while trying to scale, the rollout will be requeued
// so we can abort this resync
return err
return fmt.Errorf("failed to reconcileBlueGreenReplicaSets in syncReplicasOnly: %w", err)

Check warning on line 299 in rollout/sync.go

View check run for this annotation

Codecov / codecov/patch

rollout/sync.go#L299

Added line #L299 was not covered by tests
}
activeRS, _ := replicasetutil.GetReplicaSetByTemplateHash(c.allRSs, newStatus.BlueGreen.ActiveSelector)
if activeRS != nil {
Expand All @@ -314,7 +314,7 @@
if _, err := c.reconcileCanaryReplicaSets(); err != nil {
// If we get an error while trying to scale, the rollout will be requeued
// so we can abort this resync
return err
return fmt.Errorf("failed to reconcileCanaryReplicaSets in syncReplicasOnly: %w", err)

Check warning on line 317 in rollout/sync.go

View check run for this annotation

Codecov / codecov/patch

rollout/sync.go#L317

Added line #L317 was not covered by tests
}
newStatus.AvailableReplicas = replicasetutil.GetAvailableReplicaCountForReplicaSets(c.allRSs)
newStatus.HPAReplicas = replicasetutil.GetActualReplicaCountForReplicaSets(c.allRSs)
Expand All @@ -330,7 +330,7 @@
var err error
c.newRS, err = c.getAllReplicaSetsAndSyncRevision(false)
if err != nil {
return false, err
return false, fmt.Errorf("failed to getAllReplicaSetsAndSyncRevision in isScalingEvent: %w", err)

Check warning on line 333 in rollout/sync.go

View check run for this annotation

Codecov / codecov/patch

rollout/sync.go#L333

Added line #L333 was not covered by tests
}

for _, rs := range controller.FilterActiveReplicaSets(c.allRSs) {
Expand All @@ -357,6 +357,9 @@
scalingOperation = "down"
}
scaled, newRS, err := c.scaleReplicaSet(rs, newScale, c.rollout, scalingOperation)
if err != nil {
return scaled, newRS, fmt.Errorf("failed to scaleReplicaSet in scaleReplicaSetAndRecordEvent: %w", err)
}

Check warning on line 362 in rollout/sync.go

View check run for this annotation

Codecov / codecov/patch

rollout/sync.go#L361-L362

Added lines #L361 - L362 were not covered by tests
return scaled, newRS, err
}

Expand Down
Loading