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

fix(experiments): set JSON as default formatter #3977

Open
wants to merge 4 commits into
base: release-1.6
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion .github/workflows/image-reuse.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ jobs:
- name: Install cosign
uses: sigstore/cosign-installer@6e04d228eb30da1757ee4e1dd75a0ec73a653e06 # v3.1.1
with:
cosign-release: 'v2.2.0'
cosign-release: 'v2.4.0'

- uses: docker/setup-qemu-action@2b82ce82d56a2a04d2637cd93a637ae1b359c0a7 # v2.2.0
- uses: docker/setup-buildx-action@4c0219f9ac95b02789c1075625400b2acbff50b1 # v2.9.1
Expand Down
2 changes: 1 addition & 1 deletion cmd/rollouts-controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func newCommand() *cobra.Command {
}
setLogLevel(logLevel)
if logFormat != "" {
log.SetFormatter(createFormatter(logFormat))
log.SetFormatter(createFormatter("json"))
}
logutil.SetKLogLogger(log.New())
logutil.SetKLogLevel(klogLevel)
Expand Down
14 changes: 3 additions & 11 deletions rollout/trafficrouting.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,15 +179,13 @@ func (c *rolloutContext) reconcileTrafficRouting() error {
desiredWeight = c.calculateDesiredWeightOnAbortOrStableRollback()
if (c.rollout.Spec.Strategy.Canary.DynamicStableScale && desiredWeight == 0) || !c.rollout.Spec.Strategy.Canary.DynamicStableScale {
// If we are using dynamic stable scale we need to also make sure that desiredWeight=0 aka we are completely
// done with aborting before resetting the canary service selectors back to stable. For non-dynamic scale we do not check for availability because we are
// fully aborted and stable pods will be there, if we check for availability it causes issues with ALB readiness gates if all stable pods
// have the desired readiness gate on them during an abort we get stuck in a loop because all the stable go unready and rollouts won't be able
// to switch the desired services because there is no ready pods which causes pods to get stuck progressing forever waiting for readiness.
err = c.ensureSVCTargets(c.rollout.Spec.Strategy.Canary.CanaryService, c.stableRS, false)
// done with aborting before resetting the canary service selectors back to stable
err = c.ensureSVCTargets(c.rollout.Spec.Strategy.Canary.CanaryService, c.stableRS, true)
if err != nil {
return err
}
}

err := reconciler.RemoveManagedRoutes()
if err != nil {
return err
Expand Down Expand Up @@ -286,12 +284,6 @@ func (c *rolloutContext) reconcileTrafficRouting() error {
} else {
c.log.Infof("Desired weight (stepIdx: %s) %d not yet verified", indexString, desiredWeight)
c.enqueueRolloutAfter(c.rollout, defaults.GetRolloutVerifyRetryInterval())
// At the end of the rollout we need to verify the weight is correct, and return an error if not because we don't want the rest of the
// reconcile process to continue. We don't need to do this if we are in the middle of the rollout because the rest of the reconcile
// process won't scale down the old replicasets yet due to being in the middle of some steps.
if desiredWeight == weightutil.MaxTrafficWeight(c.rollout) && len(c.rollout.Spec.Strategy.Canary.Steps) >= int(*c.rollout.Status.CurrentStepIndex) {
return fmt.Errorf("end of rollout, desired weight %d not yet verified", desiredWeight)
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion rollout/trafficrouting/alb/alb.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ func (r *Reconciler) VerifyWeight(desiredWeight int32, additionalDestinations ..
return nil, nil
}

if !rolloututil.ShouldVerifyWeight(r.cfg.Rollout, desiredWeight) {
if !rolloututil.ShouldVerifyWeight(r.cfg.Rollout) {
// If we should not verify weight but the ALB status has not been set yet due to a Rollout resource just being
// installed in the cluster we want to actually run the rest of the function, so we do not return if
// r.cfg.Rollout.Status.ALB is nil. However, if we should not verify, and we have already updated the status once
Expand Down
2 changes: 1 addition & 1 deletion test/cmd/trafficrouter-plugin-sample/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func main() {
logCtx := log.WithFields(log.Fields{"plugin": "trafficrouter"})

setLogLevel("debug")
log.SetFormatter(createFormatter("text"))
log.SetFormatter(createFormatter("json"))

rpcPluginImp := &plugin.RpcPlugin{
LogCtx: logCtx,
Expand Down
10 changes: 4 additions & 6 deletions utils/rollout/rolloututil.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import (
"fmt"
"strconv"

"github.com/argoproj/argo-rollouts/utils/weightutil"

replicasetutil "github.com/argoproj/argo-rollouts/utils/replicaset"

"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
Expand Down Expand Up @@ -186,13 +184,13 @@ func CanaryStepString(c v1alpha1.CanaryStep) string {

// ShouldVerifyWeight We use this to test if we should verify weights because weight verification could involve
// API calls to the cloud provider which could incur rate limiting
func ShouldVerifyWeight(ro *v1alpha1.Rollout, desiredWeight int32) bool {
func ShouldVerifyWeight(ro *v1alpha1.Rollout) bool {
currentStep, _ := replicasetutil.GetCurrentCanaryStep(ro)
// If we are in the middle of an update at a setWeight step, also perform weight verification.
// Note that we don't do this every reconciliation because weight verification typically involves
// API calls to the cloud provider which could incur rate limitingq
shouldVerifyWeight := (ro.Status.StableRS != "" && !IsFullyPromoted(ro) && currentStep != nil && currentStep.SetWeight != nil) ||
(ro.Status.StableRS != "" && !IsFullyPromoted(ro) && currentStep == nil && desiredWeight == weightutil.MaxTrafficWeight(ro)) // We are at end of rollout

shouldVerifyWeight := ro.Status.StableRS != "" &&
!IsFullyPromoted(ro) &&
currentStep != nil && currentStep.SetWeight != nil
return shouldVerifyWeight
}
Loading