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.

Sign up for GitHub

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

Jump to bottom

Automated cherry pick of #41851 and #41961 upstream release 1.5 #41958

Merged
merged 1 commit into from
Feb 27, 2017
Merged

Automated cherry pick of #41851 and #41961 upstream release 1.5 #41958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 13 additions & 10 deletions pkg/controller/deployment/util/deployment_util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -604,13 +604,17 @@ func equalIgnoreHash(template1, template2 api.PodTemplateSpec) (bool, error) {
// FindNewReplicaSet returns the new RS this given deployment targets (the one with the same pod template).
func FindNewReplicaSet(deployment *extensions.Deployment, rsList []*extensions.ReplicaSet) (*extensions.ReplicaSet, error) {
newRSTemplate := GetNewReplicaSetTemplate(deployment)
sort.Sort(controller.ReplicaSetsByCreationTimestamp(rsList))
for i := range rsList {
equal, err := equalIgnoreHash(rsList[i].Spec.Template, newRSTemplate)
if err != nil {
return nil, err
}
if equal {
// This is the new ReplicaSet.
// In rare cases, such as after cluster upgrades, Deployment may end up with
// having more than one new ReplicaSets that have the same template as its template,
// see https://github.com/kubernetes/kubernetes/issues/40415
// We deterministically choose the oldest new ReplicaSet.
return rsList[i], nil
}
}
Expand All @@ -625,20 +629,21 @@ func FindOldReplicaSets(deployment *extensions.Deployment, rsList []*extensions.
// All pods and replica sets are labeled with pod-template-hash to prevent overlapping
oldRSs := map[string]*extensions.ReplicaSet{}
allOldRSs := map[string]*extensions.ReplicaSet{}
newRSTemplate := GetNewReplicaSetTemplate(deployment)
requiredRSs := []*extensions.ReplicaSet{}
allRSs := []*extensions.ReplicaSet{}
newRS, err := FindNewReplicaSet(deployment, rsList)
if err != nil {
return requiredRSs, allRSs, err
}
for _, pod := range podList.Items {
podLabelsSelector := labels.Set(pod.ObjectMeta.Labels)
for _, rs := range rsList {
rsLabelsSelector, err := unversioned.LabelSelectorAsSelector(rs.Spec.Selector)
if err != nil {
return nil, nil, fmt.Errorf("invalid label selector: %v", err)
}
// Filter out replica set that has the same pod template spec as the deployment - that is the new replica set.
equal, err := equalIgnoreHash(rs.Spec.Template, newRSTemplate)
if err != nil {
return nil, nil, err
}
if equal {
// Filter out new replica set
if newRS != nil && rs.UID == newRS.UID {
continue
}
allOldRSs[rs.ObjectMeta.Name] = rs
Expand All @@ -647,12 +652,10 @@ func FindOldReplicaSets(deployment *extensions.Deployment, rsList []*extensions.
}
}
}
requiredRSs := []*extensions.ReplicaSet{}
for key := range oldRSs {
value := oldRSs[key]
requiredRSs = append(requiredRSs, value)
}
allRSs := []*extensions.ReplicaSet{}
for key := range allOldRSs {
value := allOldRSs[key]
allRSs = append(allRSs, value)
Expand Down
60 changes: 56 additions & 4 deletions pkg/controller/deployment/util/deployment_util_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ package util

import (
"fmt"
"math/rand"
"reflect"
"sort"
"strconv"
"testing"
"time"

Expand All @@ -29,7 +32,9 @@ import (
"k8s.io/kubernetes/pkg/apis/extensions"
"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
"k8s.io/kubernetes/pkg/client/testing/core"
"k8s.io/kubernetes/pkg/controller"
"k8s.io/kubernetes/pkg/runtime"
"k8s.io/kubernetes/pkg/types"
"k8s.io/kubernetes/pkg/util/intstr"
)

Expand Down Expand Up @@ -159,6 +164,7 @@ func generateRS(deployment extensions.Deployment) extensions.ReplicaSet {
template := GetNewReplicaSetTemplate(&deployment)
return extensions.ReplicaSet{
ObjectMeta: api.ObjectMeta{
UID: randomUID(),
Name: api.SimpleNameGenerator.GenerateName("replicaset"),
Labels: template.Labels,
},
Expand All @@ -169,6 +175,10 @@ func generateRS(deployment extensions.Deployment) extensions.ReplicaSet {
}
}

func randomUID() types.UID {
return types.UID(strconv.FormatInt(rand.Int63(), 10))
}

// generateDeployment creates a deployment, with the input image as its template
func generateDeployment(image string) extensions.Deployment {
podLabels := map[string]string{"name": image}
Expand Down Expand Up @@ -466,9 +476,18 @@ func TestEqualIgnoreHash(t *testing.T) {
}

func TestFindNewReplicaSet(t *testing.T) {
now := unversioned.Now()
later := unversioned.Time{Time: now.Add(time.Minute)}

deployment := generateDeployment("nginx")
newRS := generateRS(deployment)
newRS.Labels[extensions.DefaultDeploymentUniqueLabelKey] = "different-hash"
newRS.Labels[extensions.DefaultDeploymentUniqueLabelKey] = "hash"
newRS.CreationTimestamp = later

newRSDup := generateRS(deployment)
newRSDup.Labels[extensions.DefaultDeploymentUniqueLabelKey] = "different-hash"
newRSDup.CreationTimestamp = now

oldDeployment := generateDeployment("nginx")
oldDeployment.Spec.Template.Spec.Containers[0].Name = "nginx-old-1"
oldRS := generateRS(oldDeployment)
Expand All @@ -481,11 +500,17 @@ func TestFindNewReplicaSet(t *testing.T) {
expected *extensions.ReplicaSet
}{
{
test: "Get new ReplicaSet with the same spec but different pod-template-hash value",
test: "Get new ReplicaSet with the same template as Deployment spec but different pod-template-hash value",
deployment: deployment,
rsList: []*extensions.ReplicaSet{&newRS, &oldRS},
expected: &newRS,
},
{
test: "Get the oldest new ReplicaSet when there are more than one ReplicaSet with the same template",
deployment: deployment,
rsList: []*extensions.ReplicaSet{&newRS, &oldRS, &newRSDup},
expected: &newRSDup,
},
{
test: "Get nil new ReplicaSet",
deployment: deployment,
Expand All @@ -502,13 +527,25 @@ func TestFindNewReplicaSet(t *testing.T) {
}

func TestFindOldReplicaSets(t *testing.T) {
now := unversioned.Now()
later := unversioned.Time{Time: now.Add(time.Minute)}
before := unversioned.Time{Time: now.Add(-time.Minute)}

deployment := generateDeployment("nginx")
newRS := generateRS(deployment)
newRS.Labels[extensions.DefaultDeploymentUniqueLabelKey] = "different-hash"
newRS.Labels[extensions.DefaultDeploymentUniqueLabelKey] = "hash"
newRS.CreationTimestamp = later

newRSDup := generateRS(deployment)
newRSDup.Labels[extensions.DefaultDeploymentUniqueLabelKey] = "different-hash"
newRSDup.CreationTimestamp = now

oldDeployment := generateDeployment("nginx")
oldDeployment.Spec.Template.Spec.Containers[0].Name = "nginx-old-1"
oldRS := generateRS(oldDeployment)
oldRS.Status.FullyLabeledReplicas = oldRS.Spec.Replicas
oldRS.CreationTimestamp = before

newPod := generatePodFromRS(newRS)
oldPod := generatePodFromRS(oldRS)

Expand Down Expand Up @@ -542,6 +579,18 @@ func TestFindOldReplicaSets(t *testing.T) {
},
expected: []*extensions.ReplicaSet{&oldRS},
},
{
test: "Get old ReplicaSets with two new ReplicaSets, only the oldest new ReplicaSet is seen as new ReplicaSet",
deployment: deployment,
rsList: []*extensions.ReplicaSet{&oldRS, &newRS, &newRSDup},
podList: &api.PodList{
Items: []api.Pod{
newPod,
oldPod,
},
},
expected: []*extensions.ReplicaSet{&oldRS, &newRS},
},
{
test: "Get empty old ReplicaSets",
deployment: deployment,
Expand All @@ -556,7 +605,10 @@ func TestFindOldReplicaSets(t *testing.T) {
}

for _, test := range tests {
if old, _, err := FindOldReplicaSets(&test.deployment, test.rsList, test.podList); !reflect.DeepEqual(old, test.expected) || err != nil {
old, _, err := FindOldReplicaSets(&test.deployment, test.rsList, test.podList)
sort.Sort(controller.ReplicaSetsByCreationTimestamp(old))
sort.Sort(controller.ReplicaSetsByCreationTimestamp(test.expected))
if !reflect.DeepEqual(old, test.expected) || err != nil {
t.Errorf("In test case %q, expected %#v, got %#v: %v", test.test, test.expected, old, err)
}
}
Expand Down