Skip to content

Commit

Permalink
refactor: move isReplicaSetReferenced to replicaset.go
Browse files Browse the repository at this point in the history 
Signed-off-by: Jesse Suen <jesse@akuity.io>
  • Loading branch information
@jessesuen
jessesuen committed Oct 17, 2023
1 parent 32f778e commit 86d83ed
Show file tree
Hide file tree
Showing 4 changed files with 237 additions and 235 deletions.
55 changes: 0 additions & 55 deletions rollout/canary.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"sort"

appsv1 "k8s.io/api/apps/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/pointer"

Expand All @@ -15,7 +14,6 @@ import (
"github.com/argoproj/argo-rollouts/utils/record"
replicasetutil "github.com/argoproj/argo-rollouts/utils/replicaset"
rolloututil "github.com/argoproj/argo-rollouts/utils/rollout"
serviceutil "github.com/argoproj/argo-rollouts/utils/service"
)

func (c *rolloutContext) rolloutCanary() error {
Expand Down Expand Up @@ -242,59 +240,6 @@ func (c *rolloutContext) scaleDownOldReplicaSetsForCanary(oldRSs []*appsv1.Repli
return totalScaledDown, nil
}

// isReplicaSetReferenced returns if the given ReplicaSet is still being referenced by any of
// the current, stable, blue-green services. Used to determine if the ReplicaSet can
// safely be scaled to zero, or deleted.
func (c *rolloutContext) isReplicaSetReferenced(rs *appsv1.ReplicaSet) bool {
rsPodHash := replicasetutil.GetPodTemplateHash(rs)
if rsPodHash == "" {
return false
}
ro := c.rollout
referencesToCheck := []string{
ro.Status.StableRS,
ro.Status.CurrentPodHash,
ro.Status.BlueGreen.ActiveSelector,
ro.Status.BlueGreen.PreviewSelector,
}
if ro.Status.Canary.Weights != nil {
referencesToCheck = append(referencesToCheck, ro.Status.Canary.Weights.Canary.PodTemplateHash, ro.Status.Canary.Weights.Stable.PodTemplateHash)
}
for _, ref := range referencesToCheck {
if ref == rsPodHash {
return true
}
}

// The above are static, lightweight checks to see if the selectors we record in our status are
// still referencing the ReplicaSet in question. Those checks aren't always enough. Next, we do
// a deeper check to look up the actual service objects, and see if they are still referencing
// the ReplicaSet. If so, we cannot scale it down.
var servicesToCheck []string
if ro.Spec.Strategy.Canary != nil {
servicesToCheck = []string{ro.Spec.Strategy.Canary.CanaryService, ro.Spec.Strategy.Canary.StableService}
} else {
servicesToCheck = []string{ro.Spec.Strategy.BlueGreen.ActiveService, ro.Spec.Strategy.BlueGreen.PreviewService}
}
for _, svcName := range servicesToCheck {
if svcName == "" {
continue
}
svc, err := c.servicesLister.Services(c.rollout.Namespace).Get(svcName)
if err != nil {
if k8serrors.IsNotFound(err) {
// service doesn't exist
continue
}
return true
}
if serviceutil.GetRolloutSelectorLabel(svc) == rsPodHash {
return true
}
}
return false
}

// isDynamicallyRollingBackToStable returns true if we were in the middle of an canary update with
// dynamic stable scaling, but was interrupted and are now rolling back to stable RS. This is similar
// to, but different than aborting. With abort, desired hash != stable hash and so we know the
Expand Down
180 changes: 0 additions & 180 deletions rollout/canary_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/intstr"
k8sinformers "k8s.io/client-go/informers"
k8sfake "k8s.io/client-go/kubernetes/fake"
Expand Down Expand Up @@ -1893,185 +1892,6 @@ func TestHandleCanaryAbort(t *testing.T) {
})
}

func TestIsReplicaSetReferenced(t *testing.T) {
newRSWithPodTemplateHash := func(hash string) *appsv1.ReplicaSet {
return &appsv1.ReplicaSet{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
v1alpha1.DefaultRolloutUniqueLabelKey: hash,
},
},
}
}

testCases := []struct {
name string
status v1alpha1.RolloutStatus
canaryService string
stableService string
activeService string
previewService string
services []runtime.Object
rsHash string
expectedResult bool
}{
{
name: "empty hash",
status: v1alpha1.RolloutStatus{StableRS: "abc123"},
rsHash: "",
expectedResult: false,
},
{
name: "not referenced",
status: v1alpha1.RolloutStatus{StableRS: "abc123"},
rsHash: "def456",
expectedResult: false,
},
{
name: "stable rs referenced",
status: v1alpha1.RolloutStatus{StableRS: "abc123"},
rsHash: "abc123",
expectedResult: true,
},
{
name: "current rs referenced",
status: v1alpha1.RolloutStatus{CurrentPodHash: "abc123"},
rsHash: "abc123",
expectedResult: true,
},
{
name: "active referenced",
status: v1alpha1.RolloutStatus{BlueGreen: v1alpha1.BlueGreenStatus{ActiveSelector: "abc123"}},
rsHash: "abc123",
expectedResult: true,
},
{
name: "active referenced",
status: v1alpha1.RolloutStatus{BlueGreen: v1alpha1.BlueGreenStatus{PreviewSelector: "abc123"}},
rsHash: "abc123",
expectedResult: true,
},
{
name: "traffic routed canary current pod hash",
status: v1alpha1.RolloutStatus{Canary: v1alpha1.CanaryStatus{Weights: &v1alpha1.TrafficWeights{
Canary: v1alpha1.WeightDestination{
PodTemplateHash: "abc123",
},
}}},
rsHash: "abc123",
expectedResult: true,
},
{
name: "traffic routed canary current pod hash",
status: v1alpha1.RolloutStatus{Canary: v1alpha1.CanaryStatus{Weights: &v1alpha1.TrafficWeights{
Stable: v1alpha1.WeightDestination{
PodTemplateHash: "abc123",
},
}}},
rsHash: "abc123",
expectedResult: true,
},
{
name: "canary service still referenced",
status: v1alpha1.RolloutStatus{
CurrentPodHash: "abc123",
StableRS: "abc123",
},
canaryService: "mysvc",
services: []runtime.Object{newService("mysvc", 80, map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: "def456"}, nil)},
rsHash: "def456",
expectedResult: true,
},
{
name: "stable service still referenced",
status: v1alpha1.RolloutStatus{
CurrentPodHash: "abc123",
StableRS: "abc123",
},
stableService: "mysvc",
services: []runtime.Object{newService("mysvc", 80, map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: "def456"}, nil)},
rsHash: "def456",
expectedResult: true,
},
{
name: "active service still referenced",
status: v1alpha1.RolloutStatus{
CurrentPodHash: "abc123",
StableRS: "abc123",
},
activeService: "mysvc",
services: []runtime.Object{newService("mysvc", 80, map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: "def456"}, nil)},
rsHash: "def456",
expectedResult: true,
},
{
name: "preview service still referenced",
status: v1alpha1.RolloutStatus{
CurrentPodHash: "abc123",
StableRS: "abc123",
},
activeService: "mysvc",
previewService: "mysvc2",
services: []runtime.Object{newService("mysvc2", 80, map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: "def456"}, nil)},
rsHash: "def456",
expectedResult: true,
},
{
name: "service not found",
status: v1alpha1.RolloutStatus{
CurrentPodHash: "abc123",
StableRS: "abc123",
},
activeService: "mysvc",
previewService: "mysvc2",
rsHash: "def456",
expectedResult: false,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {

fake := fake.Clientset{}
k8sfake := k8sfake.NewSimpleClientset(tc.services...)
informers := k8sinformers.NewSharedInformerFactory(k8sfake, 0)
servicesLister := informers.Core().V1().Services().Lister()
stopchan := make(chan struct{})
defer close(stopchan)
informers.Start(stopchan)
informers.WaitForCacheSync(stopchan)

var r *v1alpha1.Rollout
if tc.activeService != "" {
r = newBlueGreenRollout("test", 1, nil, tc.activeService, tc.previewService)
} else {
r = newCanaryRollout("test", 1, nil, nil, nil, intstr.FromInt(0), intstr.FromInt(1))
r.Spec.Strategy.Canary.CanaryService = tc.canaryService
r.Spec.Strategy.Canary.StableService = tc.stableService
}
r.Status = tc.status

roCtx := &rolloutContext{
rollout: r,
log: logutil.WithRollout(r),
reconcilerBase: reconcilerBase{
servicesLister: servicesLister,
argoprojclientset: &fake,
kubeclientset: k8sfake,
recorder: record.NewFakeEventRecorder(),
},
}
rs := newRSWithPodTemplateHash(tc.rsHash)
stillReferenced := roCtx.isReplicaSetReferenced(rs)

assert.Equal(
t,
tc.expectedResult,
stillReferenced,
)
})
}
}

func TestIsDynamicallyRollingBackToStable(t *testing.T) {
newRSWithHashAndReplicas := func(hash string, available int32) *appsv1.ReplicaSet {
return &appsv1.ReplicaSet{
Expand Down
55 changes: 55 additions & 0 deletions rollout/replicaset.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"time"

appsv1 "k8s.io/api/apps/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
patchtypes "k8s.io/apimachinery/pkg/types"
Expand All @@ -15,6 +16,7 @@ import (
"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
"github.com/argoproj/argo-rollouts/utils/defaults"
replicasetutil "github.com/argoproj/argo-rollouts/utils/replicaset"
serviceutil "github.com/argoproj/argo-rollouts/utils/service"
timeutil "github.com/argoproj/argo-rollouts/utils/time"
)

Expand Down Expand Up @@ -296,3 +298,56 @@ func (c *rolloutContext) scaleDownDelayHelper(rs *appsv1.ReplicaSet, annotatione

return annotationedRSs, desiredReplicaCount, nil
}

// isReplicaSetReferenced returns if the given ReplicaSet is still being referenced by any of
// the current, stable, blue-green services. Used to determine if the ReplicaSet can
// safely be scaled to zero, or deleted.
func (c *rolloutContext) isReplicaSetReferenced(rs *appsv1.ReplicaSet) bool {
rsPodHash := replicasetutil.GetPodTemplateHash(rs)
if rsPodHash == "" {
return false
}
ro := c.rollout
referencesToCheck := []string{
ro.Status.StableRS,
ro.Status.CurrentPodHash,
ro.Status.BlueGreen.ActiveSelector,
ro.Status.BlueGreen.PreviewSelector,
}
if ro.Status.Canary.Weights != nil {
referencesToCheck = append(referencesToCheck, ro.Status.Canary.Weights.Canary.PodTemplateHash, ro.Status.Canary.Weights.Stable.PodTemplateHash)
}
for _, ref := range referencesToCheck {
if ref == rsPodHash {
return true
}
}

// The above are static, lightweight checks to see if the selectors we record in our status are
// still referencing the ReplicaSet in question. Those checks aren't always enough. Next, we do
// a deeper check to look up the actual service objects, and see if they are still referencing
// the ReplicaSet. If so, we cannot scale it down.
var servicesToCheck []string
if ro.Spec.Strategy.Canary != nil {
servicesToCheck = []string{ro.Spec.Strategy.Canary.CanaryService, ro.Spec.Strategy.Canary.StableService}
} else {
servicesToCheck = []string{ro.Spec.Strategy.BlueGreen.ActiveService, ro.Spec.Strategy.BlueGreen.PreviewService}
}
for _, svcName := range servicesToCheck {
if svcName == "" {
continue
}
svc, err := c.servicesLister.Services(c.rollout.Namespace).Get(svcName)
if err != nil {
if k8serrors.IsNotFound(err) {
// service doesn't exist
continue
}
return true
}
if serviceutil.GetRolloutSelectorLabel(svc) == rsPodHash {
return true
}
}
return false
}
Loading

0 comments on commit 86d83ed

Please sign in to comment.