From 617e5d0a47a4e34624d5a2bf969726cf11f6cf5a Mon Sep 17 00:00:00 2001 From: Zach Aller Date: Thu, 25 Jan 2024 09:26:28 -0600 Subject: [PATCH 1/2] chore: add more logging Signed-off-by: Zach Aller --- rollout/bluegreen.go | 8 +++- rollout/canary.go | 10 +++-- rollout/replicaset.go | 4 +- rollout/step_plugins.go | 91 +++++++++++++++++++++++++++++++++++++++++ rollout/steps/plugin.go | 23 +++++++++++ rollout/sync.go | 11 +++-- 6 files changed, 136 insertions(+), 11 deletions(-) create mode 100644 rollout/step_plugins.go create mode 100644 rollout/steps/plugin.go diff --git a/rollout/bluegreen.go b/rollout/bluegreen.go index f1bcf7a7bb..9904de9dab 100644 --- a/rollout/bluegreen.go +++ b/rollout/bluegreen.go @@ -1,6 +1,7 @@ package rollout import ( + "fmt" "math" "sort" @@ -22,7 +23,7 @@ func (c *rolloutContext) rolloutBlueGreen() error { } c.newRS, err = c.getAllReplicaSetsAndSyncRevision(true) if err != nil { - return err + return fmt.Errorf("failed to getAllReplicaSetsAndSyncRevision in rolloutBlueGreen create true: %w", err) } // This must happen right after the new replicaset is created @@ -82,6 +83,9 @@ func (c *rolloutContext) reconcileBlueGreenStableReplicaSet(activeSvc *corev1.Se 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) + } return err } @@ -243,7 +247,7 @@ func (c *rolloutContext) scaleDownOldReplicaSetsForBlueGreen(oldRSs []*appsv1.Re // Scale down. _, _, err = c.scaleReplicaSetAndRecordEvent(targetRS, desiredReplicaCount) if err != nil { - return false, err + return false, fmt.Errorf("failed to scaleReplicaSetAndRecordEvent in scaleDownOldReplicaSetsForBlueGreen: %w", err) } hasScaled = true } diff --git a/rollout/canary.go b/rollout/canary.go index f37e03bab1..a003b092a3 100644 --- a/rollout/canary.go +++ b/rollout/canary.go @@ -1,6 +1,7 @@ package rollout import ( + "fmt" "sort" appsv1 "k8s.io/api/apps/v1" @@ -21,14 +22,14 @@ func (c *rolloutContext) rolloutCanary() error { 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) } 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) } err = c.podRestarter.Reconcile(c) @@ -110,6 +111,9 @@ func (c *rolloutContext) reconcileCanaryStableReplicaSet() (bool, error) { _, 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) + } return scaled, err } @@ -230,7 +234,7 @@ func (c *rolloutContext) scaleDownOldReplicaSetsForCanary(oldRSs []*appsv1.Repli // Scale down. _, _, err = c.scaleReplicaSetAndRecordEvent(targetRS, desiredReplicaCount) if err != nil { - return totalScaledDown, err + return totalScaledDown, fmt.Errorf("failed to scaleReplicaSetAndRecordEvent in scaleDownOldReplicaSetsForCanary: %w", err) } scaleDownCount := *targetRS.Spec.Replicas - desiredReplicaCount maxScaleDown -= scaleDownCount diff --git a/rollout/replicaset.go b/rollout/replicaset.go index 912417ca1a..c16ce6f037 100644 --- a/rollout/replicaset.go +++ b/rollout/replicaset.go @@ -174,7 +174,7 @@ func (c *rolloutContext) reconcileNewReplicaSet() (bool, error) { scaled, _, err := c.scaleReplicaSetAndRecordEvent(c.newRS, newReplicasCount) if err != nil { - return scaled, err + return scaled, fmt.Errorf("failed to scaleReplicaSetAndRecordEvent in reconcileNewReplicaSet: %w", err) } revision, _ := replicasetutil.Revision(c.newRS) @@ -279,7 +279,7 @@ func (c *rolloutContext) cleanupUnhealthyReplicas(oldRSs []*appsv1.ReplicaSet) ( } _, 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) } totalScaledDown += scaledDownCount oldRSs[i] = updatedOldRS diff --git a/rollout/step_plugins.go b/rollout/step_plugins.go new file mode 100644 index 0000000000..73b0bfab3c --- /dev/null +++ b/rollout/step_plugins.go @@ -0,0 +1,91 @@ +package rollout + +// +//import ( +// "fmt" +// "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" +// metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +//) +// +//type stepContext struct { +// rollout *v1alpha1.Rollout +// pluginType string +// calledAt metav1.Time +// currentStepStatus v1alpha1.StepPluginStatuses +//} +// +//func newStepContext(rollout *v1alpha1.Rollout, pluginType string) *stepContext { +// sc := &stepContext{ +// rollout: rollout, +// pluginType: pluginType, +// } +// +// //mapStatuses := map[string]v1alpha1.StepPluginStatuses{} +// //json.Unmarshal(rollout.Status.SPluginStatus, &mapStatuses) +// for _, status := range sc.rollout.Status.StepPluginStatus { +// if status.Name == fmt.Sprintf("%s.%d", pluginType, *rollout.Status.CurrentStepIndex) { +// sc.currentStepStatus = status +// } +// } +// +// return sc +//} +// +//func (c *rolloutContext) reconcileStepPlugins() error { +// +// +// //currentStep, index := replicasetutil.GetCurrentCanaryStep(c.rollout) +// //if currentStep == nil { +// // return nil +// //} +// //if index == nil { +// // var ii int32 = 0 +// // index = &ii +// //} +// //revision, revisionFound := annotations.GetRevisionAnnotation(c.rollout) +// //if currentStep != nil && (revisionFound && revision <= 1) { +// // log.Printf("Skipping Step Plugin Reconcile for Rollout %s/%s, revision %d", c.rollout.Namespace, c.rollout.Name, revision) +// // return nil +// //} +// // +// //sps := steps.NewStepPluginReconcile(currentStep) +// //for _, plugin := range sps { +// // c.stepContext = newStepContext(c.rollout, plugin.Type()) +// // +// // c.newStatus = *c.rollout.Status.DeepCopy() +// // +// // if c.controllerStartTime.After(c.stepContext.calledAt.Time) { +// // log.Printf("Controller Running Step: %d, Plugin: %s", *index, plugin.Type()) +// // res, e := plugin.RunStep(*c.rollout, c.stepContext.currentStepStatus) +// // c.stepContext.currentStepStatus.HasBeenCalled = true +// // c.stepContext.currentStepStatus.CalledAt = metav1.Now() +// // if e != nil { +// // fmt.Println(e) +// // } +// // +// // if c.stepContext.currentStepStatus.IsEmpty() { +// // c.stepContext.currentStepStatus = v1alpha1.StepPluginStatuses{ +// // Name: fmt.Sprintf("%s.%d", plugin.Type(), *index), +// // StepIndex: index, +// // Status: v1alpha1.Object{Value: res}, +// // } +// // } +// // if res != nil { +// // c.stepContext.currentStepStatus.Status = v1alpha1.Object{Value: res} +// // } +// // } +// // +// // c.newStatus.StepPluginStatus[fmt.Sprintf("%s.%d", plugin.Type(), *index)] = c.stepContext.currentStepStatus +// //} +// +// return nil +//} +// +//func (c *rolloutContext) setStepCondition(pluginType string, status v1alpha1.StepPluginStatuses) { +// //c.newStatus = *c.rollout.Status.DeepCopy() +// //if c.newStatus.SPluginStatus == nil { +// // c.newStatus.SPluginStatus = map[string]v1alpha1.StepPluginStatuses{} +// //} +// //c.newStatus.SPluginStatus[fmt.Sprintf("%s.%d", pluginType, *c.rollout.Status.CurrentStepIndex)] = status +// //c.syncRolloutStatusCanary() +//} diff --git a/rollout/steps/plugin.go b/rollout/steps/plugin.go new file mode 100644 index 0000000000..bc1610081b --- /dev/null +++ b/rollout/steps/plugin.go @@ -0,0 +1,23 @@ +package steps + +//import ( +// "encoding/json" +// "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" +//) +// +//type StepPlugin interface { +// RunStep(rollout v1alpha1.Rollout, currentStepStatus v1alpha1.StepPluginStatuses) (json.RawMessage, error) +// IsStepCompleted(rollout v1alpha1.Rollout, currentStatus v1alpha1.StepPluginStatuses) (bool, json.RawMessage, error) +// Type() string +//} +// +//func NewStepPluginReconcile(currentStep *v1alpha1.CanaryStep) []StepPlugin { +// stepPlugins := make([]StepPlugin, 0) +// for pluginName, _ := range currentStep.RunPlugins { +// switch pluginName { +// case "consolelogger": +// //stepPlugins = append(stepPlugins, consolelogger.NewConsoleLoggerStep()) +// } +// } +// return stepPlugins +//} diff --git a/rollout/sync.go b/rollout/sync.go index a37627f680..39c0d39665 100644 --- a/rollout/sync.go +++ b/rollout/sync.go @@ -283,7 +283,7 @@ func (c *rolloutContext) syncReplicasOnly() error { var err error c.newRS, err = c.getAllReplicaSetsAndSyncRevision(false) if err != nil { - return err + return fmt.Errorf("failed to getAllReplicaSetsAndSyncRevision in syncReplicasOnly: %w", err) } newStatus := c.rollout.Status.DeepCopy() @@ -296,7 +296,7 @@ func (c *rolloutContext) syncReplicasOnly() error { 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) } activeRS, _ := replicasetutil.GetReplicaSetByTemplateHash(c.allRSs, newStatus.BlueGreen.ActiveSelector) if activeRS != nil { @@ -314,7 +314,7 @@ func (c *rolloutContext) syncReplicasOnly() error { 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) } newStatus.AvailableReplicas = replicasetutil.GetAvailableReplicaCountForReplicaSets(c.allRSs) newStatus.HPAReplicas = replicasetutil.GetActualReplicaCountForReplicaSets(c.allRSs) @@ -330,7 +330,7 @@ func (c *rolloutContext) isScalingEvent() (bool, error) { 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) } for _, rs := range controller.FilterActiveReplicaSets(c.allRSs) { @@ -357,6 +357,9 @@ func (c *rolloutContext) scaleReplicaSetAndRecordEvent(rs *appsv1.ReplicaSet, ne 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) + } return scaled, newRS, err } From f368ae5b7bf8fd1535e21808324af96f73429a66 Mon Sep 17 00:00:00 2001 From: Zach Aller Date: Thu, 25 Jan 2024 09:27:27 -0600 Subject: [PATCH 2/2] chore: remove accidental files Signed-off-by: Zach Aller --- rollout/step_plugins.go | 91 ----------------------------------------- rollout/steps/plugin.go | 23 ----------- 2 files changed, 114 deletions(-) delete mode 100644 rollout/step_plugins.go delete mode 100644 rollout/steps/plugin.go diff --git a/rollout/step_plugins.go b/rollout/step_plugins.go deleted file mode 100644 index 73b0bfab3c..0000000000 --- a/rollout/step_plugins.go +++ /dev/null @@ -1,91 +0,0 @@ -package rollout - -// -//import ( -// "fmt" -// "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" -// metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" -//) -// -//type stepContext struct { -// rollout *v1alpha1.Rollout -// pluginType string -// calledAt metav1.Time -// currentStepStatus v1alpha1.StepPluginStatuses -//} -// -//func newStepContext(rollout *v1alpha1.Rollout, pluginType string) *stepContext { -// sc := &stepContext{ -// rollout: rollout, -// pluginType: pluginType, -// } -// -// //mapStatuses := map[string]v1alpha1.StepPluginStatuses{} -// //json.Unmarshal(rollout.Status.SPluginStatus, &mapStatuses) -// for _, status := range sc.rollout.Status.StepPluginStatus { -// if status.Name == fmt.Sprintf("%s.%d", pluginType, *rollout.Status.CurrentStepIndex) { -// sc.currentStepStatus = status -// } -// } -// -// return sc -//} -// -//func (c *rolloutContext) reconcileStepPlugins() error { -// -// -// //currentStep, index := replicasetutil.GetCurrentCanaryStep(c.rollout) -// //if currentStep == nil { -// // return nil -// //} -// //if index == nil { -// // var ii int32 = 0 -// // index = &ii -// //} -// //revision, revisionFound := annotations.GetRevisionAnnotation(c.rollout) -// //if currentStep != nil && (revisionFound && revision <= 1) { -// // log.Printf("Skipping Step Plugin Reconcile for Rollout %s/%s, revision %d", c.rollout.Namespace, c.rollout.Name, revision) -// // return nil -// //} -// // -// //sps := steps.NewStepPluginReconcile(currentStep) -// //for _, plugin := range sps { -// // c.stepContext = newStepContext(c.rollout, plugin.Type()) -// // -// // c.newStatus = *c.rollout.Status.DeepCopy() -// // -// // if c.controllerStartTime.After(c.stepContext.calledAt.Time) { -// // log.Printf("Controller Running Step: %d, Plugin: %s", *index, plugin.Type()) -// // res, e := plugin.RunStep(*c.rollout, c.stepContext.currentStepStatus) -// // c.stepContext.currentStepStatus.HasBeenCalled = true -// // c.stepContext.currentStepStatus.CalledAt = metav1.Now() -// // if e != nil { -// // fmt.Println(e) -// // } -// // -// // if c.stepContext.currentStepStatus.IsEmpty() { -// // c.stepContext.currentStepStatus = v1alpha1.StepPluginStatuses{ -// // Name: fmt.Sprintf("%s.%d", plugin.Type(), *index), -// // StepIndex: index, -// // Status: v1alpha1.Object{Value: res}, -// // } -// // } -// // if res != nil { -// // c.stepContext.currentStepStatus.Status = v1alpha1.Object{Value: res} -// // } -// // } -// // -// // c.newStatus.StepPluginStatus[fmt.Sprintf("%s.%d", plugin.Type(), *index)] = c.stepContext.currentStepStatus -// //} -// -// return nil -//} -// -//func (c *rolloutContext) setStepCondition(pluginType string, status v1alpha1.StepPluginStatuses) { -// //c.newStatus = *c.rollout.Status.DeepCopy() -// //if c.newStatus.SPluginStatus == nil { -// // c.newStatus.SPluginStatus = map[string]v1alpha1.StepPluginStatuses{} -// //} -// //c.newStatus.SPluginStatus[fmt.Sprintf("%s.%d", pluginType, *c.rollout.Status.CurrentStepIndex)] = status -// //c.syncRolloutStatusCanary() -//} diff --git a/rollout/steps/plugin.go b/rollout/steps/plugin.go deleted file mode 100644 index bc1610081b..0000000000 --- a/rollout/steps/plugin.go +++ /dev/null @@ -1,23 +0,0 @@ -package steps - -//import ( -// "encoding/json" -// "github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1" -//) -// -//type StepPlugin interface { -// RunStep(rollout v1alpha1.Rollout, currentStepStatus v1alpha1.StepPluginStatuses) (json.RawMessage, error) -// IsStepCompleted(rollout v1alpha1.Rollout, currentStatus v1alpha1.StepPluginStatuses) (bool, json.RawMessage, error) -// Type() string -//} -// -//func NewStepPluginReconcile(currentStep *v1alpha1.CanaryStep) []StepPlugin { -// stepPlugins := make([]StepPlugin, 0) -// for pluginName, _ := range currentStep.RunPlugins { -// switch pluginName { -// case "consolelogger": -// //stepPlugins = append(stepPlugins, consolelogger.NewConsoleLoggerStep()) -// } -// } -// return stepPlugins -//}