github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2025-07-23 19:56:01 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #43508 from shiywang/first-deploy

Browse Source 
Automatic merge from submit-queue

Fix scaled down deployments cannot identify old replica sets

Fixes https://github.com/kubernetes/kubernetes/issues/42570
This commit is contained in:
Kubernetes Submit Queue 2017-03-29 20:42:31 -07:00 committed by GitHub
commit 05cd33b29e
4 changed files with 62 additions and 139 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
pkg/controller/deployment/sync.go
View File

@ -111,11 +111,11 @@ func (dc *DeploymentController) checkPausedConditions(d *extensions.Deployment)
// This may lead to stale reads of replica sets, thus incorrect deployment status.
func (dc *DeploymentController) getAllReplicaSetsAndSyncRevision(d *extensions.Deployment, rsList []*extensions.ReplicaSet, podMap map[types.UID]*v1.PodList, createIfNotExisted bool) (*extensions.ReplicaSet, []*extensions.ReplicaSet, error) {
// List the deployment's RSes & Pods and apply pod-template-hash info to deployment's adopted RSes/Pods
rsList, podList, err := dc.rsAndPodsWithHashKeySynced(d, rsList, podMap)
rsList, err := dc.rsAndPodsWithHashKeySynced(d, rsList, podMap)
if err != nil {
return nil, nil, fmt.Errorf("error labeling replica sets and pods with pod-template-hash: %v", err)
}
_, allOldRSs, err := deploymentutil.FindOldReplicaSets(d, rsList, podList)
_, allOldRSs, err := deploymentutil.FindOldReplicaSets(d, rsList)
if err != nil {
return nil, nil, err
}
@ -134,7 +134,7 @@ func (dc *DeploymentController) getAllReplicaSetsAndSyncRevision(d *extensions.D
//
// rsList should come from getReplicaSetsForDeployment(d).
// podMap should come from getPodMapForDeployment(d, rsList).
func (dc *DeploymentController) rsAndPodsWithHashKeySynced(d *extensions.Deployment, rsList []*extensions.ReplicaSet, podMap map[types.UID]*v1.PodList) ([]*extensions.ReplicaSet, *v1.PodList, error) {
func (dc *DeploymentController) rsAndPodsWithHashKeySynced(d *extensions.Deployment, rsList []*extensions.ReplicaSet, podMap map[types.UID]*v1.PodList) ([]*extensions.ReplicaSet, error) {
syncedRSList := []*extensions.ReplicaSet{}
for _, rs := range rsList {
// Add pod-template-hash information if it's not in the RS.
@ -142,16 +142,11 @@ func (dc *DeploymentController) rsAndPodsWithHashKeySynced(d *extensions.Deploym
// that aren't constrained by the pod-template-hash.
syncedRS, err := dc.addHashKeyToRSAndPods(rs, podMap[rs.UID])
if err != nil {
return nil, nil, err
return nil, err
}
syncedRSList = append(syncedRSList, syncedRS)
}
// Put all Pods from podMap into one list.
syncedPodList := &v1.PodList{}
for _, podList := range podMap {
syncedPodList.Items = append(syncedPodList.Items, podList.Items...)
}
return syncedRSList, syncedPodList, nil
return syncedRSList, nil
}
// addHashKeyToRSAndPods adds pod-template-hash information to the given rs, if it's not already there, with the following steps:

1
pkg/controller/deployment/util/BUILD
View File

@ -35,7 +35,6 @@ go_library(
"//vendor:k8s.io/apimachinery/pkg/api/equality",
"//vendor:k8s.io/apimachinery/pkg/api/meta",
"//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",
"//vendor:k8s.io/apimachinery/pkg/labels",
"//vendor:k8s.io/apimachinery/pkg/runtime",
"//vendor:k8s.io/apimachinery/pkg/types",
"//vendor:k8s.io/apimachinery/pkg/util/errors",

65
pkg/controller/deployment/util/deployment_util.go
View File

@ -28,7 +28,6 @@ import (
apiequality "k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/errors"
@ -503,11 +502,7 @@ func GetAllReplicaSets(deployment *extensions.Deployment, c clientset.Interface)
if err != nil {
return nil, nil, nil, err
}
podList, err := ListPods(deployment, rsList, podListFromClient(c))
if err != nil {
return nil, nil, nil, err
}
oldRSes, allOldRSes, err := FindOldReplicaSets(deployment, rsList, podList)
oldRSes, allOldRSes, err := FindOldReplicaSets(deployment, rsList)
if err != nil {
return nil, nil, nil, err
}
@ -526,11 +521,7 @@ func GetAllReplicaSetsV15(deployment *extensions.Deployment, c clientset.Interfa
if err != nil {
return nil, nil, nil, err
}
podList, err := ListPodsV15(deployment, rsList, podListFromClient(c))
if err != nil {
return nil, nil, nil, err
}
oldRSes, allOldRSes, err := FindOldReplicaSets(deployment, rsList, podList)
oldRSes, allOldRSes, err := FindOldReplicaSets(deployment, rsList)
if err != nil {
return nil, nil, nil, err
}
@ -548,11 +539,7 @@ func GetOldReplicaSets(deployment *extensions.Deployment, c clientset.Interface)
if err != nil {
return nil, nil, err
}
podList, err := ListPods(deployment, rsList, podListFromClient(c))
if err != nil {
return nil, nil, err
}
return FindOldReplicaSets(deployment, rsList, podList)
return FindOldReplicaSets(deployment, rsList)
}
// GetNewReplicaSet returns a replica set that matches the intent of the given deployment; get ReplicaSetList from client interface.
@ -794,44 +781,24 @@ func FindNewReplicaSet(deployment *extensions.Deployment, rsList []*extensions.R
return nil, nil
}
// FindOldReplicaSets returns the old replica sets targeted by the given Deployment, with the given PodList and slice of RSes.
// FindOldReplicaSets returns the old replica sets targeted by the given Deployment, with the given slice of RSes.
// Note that the first set of old replica sets doesn't include the ones with no pods, and the second set of old replica sets include all old replica sets.
func FindOldReplicaSets(deployment *extensions.Deployment, rsList []*extensions.ReplicaSet, podList *v1.PodList) ([]*extensions.ReplicaSet, []*extensions.ReplicaSet, error) {
// Find all pods whose labels match deployment.Spec.Selector, and corresponding replica sets for pods in podList.
// All pods and replica sets are labeled with pod-template-hash to prevent overlapping
oldRSs := map[string]*extensions.ReplicaSet{}
allOldRSs := map[string]*extensions.ReplicaSet{}
requiredRSs := []*extensions.ReplicaSet{}
allRSs := []*extensions.ReplicaSet{}
func FindOldReplicaSets(deployment *extensions.Deployment, rsList []*extensions.ReplicaSet) ([]*extensions.ReplicaSet, []*extensions.ReplicaSet, error) {
var requiredRSs []*extensions.ReplicaSet
var allRSs []*extensions.ReplicaSet
newRS, err := FindNewReplicaSet(deployment, rsList)
if err != nil {
return requiredRSs, allRSs, err
return nil, nil, err
}
for _, pod := range podList.Items {
podLabelsSelector := labels.Set(pod.ObjectMeta.Labels)
for _, rs := range rsList {
rsLabelsSelector, err := metav1.LabelSelectorAsSelector(rs.Spec.Selector)
if err != nil {
return nil, nil, fmt.Errorf("invalid label selector: %v", err)
}
// Filter out new replica set
if newRS != nil && rs.UID == newRS.UID {
continue
}
// TODO: If there are no pods for a deployment, we will never return old replica sets....!
allOldRSs[rs.ObjectMeta.Name] = rs
if rsLabelsSelector.Matches(podLabelsSelector) {
oldRSs[rs.ObjectMeta.Name] = rs
}
for _, rs := range rsList {
// Filter out new replica set
if newRS != nil && rs.UID == newRS.UID {
continue
}
allRSs = append(allRSs, rs)
if *(rs.Spec.Replicas) != 0 {
requiredRSs = append(requiredRSs, rs)
}
}
for key := range oldRSs {
value := oldRSs[key]
requiredRSs = append(requiredRSs, value)
}
for key := range allOldRSs {
value := allOldRSs[key]
allRSs = append(allRSs, value)
}
return requiredRSs, allRSs, nil
}

120
pkg/controller/deployment/util/deployment_util_test.go
View File

@ -241,7 +241,7 @@ func generateDeployment(image string) extensions.Deployment {
}
}
func TestGetNewRC(t *testing.T) {
func TestGetNewRS(t *testing.T) {
newDeployment := generateDeployment("nginx")
newRC := generateRS(newDeployment)
@ -297,27 +297,23 @@ func TestGetNewRC(t *testing.T) {
}
}
func TestGetOldRCs(t *testing.T) {
func TestGetOldRSs(t *testing.T) {
newDeployment := generateDeployment("nginx")
newRS := generateRS(newDeployment)
newRS.Status.FullyLabeledReplicas = *(newRS.Spec.Replicas)
newPod := generatePodFromRS(newRS)
// create 2 old deployments and related replica sets/pods, with the same labels but different template
oldDeployment := generateDeployment("nginx")
oldDeployment.Spec.Template.Spec.Containers[0].Name = "nginx-old-1"
oldRS := generateRS(oldDeployment)
oldRS.Status.FullyLabeledReplicas = *(oldRS.Spec.Replicas)
oldPod := generatePodFromRS(oldRS)
oldDeployment2 := generateDeployment("nginx")
oldDeployment2.Spec.Template.Spec.Containers[0].Name = "nginx-old-2"
oldRS2 := generateRS(oldDeployment2)
oldRS2.Status.FullyLabeledReplicas = *(oldRS2.Spec.Replicas)
oldPod2 := generatePodFromRS(oldRS2)
// create 1 ReplicaSet that existed before the deployment,
// with the same labels as the deployment, but no ControllerRef.
existedPod := generatePod(newDeployment.Spec.Template.Labels, "foo")
existedRS := generateRSWithLabel(newDeployment.Spec.Template.Labels, "foo")
existedRS.Status.FullyLabeledReplicas = *(existedRS.Spec.Replicas)
@ -329,13 +325,6 @@ func TestGetOldRCs(t *testing.T) {
{
"No old ReplicaSets",
[]runtime.Object{
&v1.PodList{
Items: []v1.Pod{
generatePod(newDeployment.Spec.Template.Labels, "foo"),
generatePod(newDeployment.Spec.Template.Labels, "bar"),
newPod,
},
},
&extensions.ReplicaSetList{
Items: []extensions.ReplicaSet{
generateRS(generateDeployment("foo")),
@ -344,21 +333,11 @@ func TestGetOldRCs(t *testing.T) {
},
},
},
[]*extensions.ReplicaSet{},
nil,
},
{
"Has old ReplicaSet",
[]runtime.Object{
&v1.PodList{
Items: []v1.Pod{
oldPod,
oldPod2,
generatePod(map[string]string{"name": "bar"}, "bar"),
generatePod(map[string]string{"name": "xyz"}, "xyz"),
existedPod,
generatePod(newDeployment.Spec.Template.Labels, "abc"),
},
},
&extensions.ReplicaSetList{
Items: []extensions.ReplicaSet{
oldRS2,
@ -376,12 +355,10 @@ func TestGetOldRCs(t *testing.T) {
for _, test := range tests {
fakeClient := &fake.Clientset{}
fakeClient = addListPodsReactor(fakeClient, test.objs[0])
fakeClient = addListRSReactor(fakeClient, test.objs[1])
fakeClient = addGetRSReactor(fakeClient, test.objs[1])
fakeClient = addUpdatePodsReactor(fakeClient)
fakeClient = addListRSReactor(fakeClient, test.objs[0])
fakeClient = addGetRSReactor(fakeClient, test.objs[0])
fakeClient = addUpdateRSReactor(fakeClient)
rss, _, err := GetOldReplicaSets(&newDeployment, fakeClient)
_, rss, err := GetOldReplicaSets(&newDeployment, fakeClient)
if err != nil {
t.Errorf("In test case %s, got unexpected error %v", test.test, err)
}
@ -558,6 +535,7 @@ func TestFindOldReplicaSets(t *testing.T) {
deployment := generateDeployment("nginx")
newRS := generateRS(deployment)
*(newRS.Spec.Replicas) = 1
newRS.Labels[extensions.DefaultDeploymentUniqueLabelKey] = "hash"
newRS.CreationTimestamp = later
@ -571,70 +549,54 @@ func TestFindOldReplicaSets(t *testing.T) {
oldRS.Status.FullyLabeledReplicas = *(oldRS.Spec.Replicas)
oldRS.CreationTimestamp = before
newPod := generatePodFromRS(newRS)
oldPod := generatePodFromRS(oldRS)
tests := []struct {
test string
deployment extensions.Deployment
rsList []*extensions.ReplicaSet
podList *v1.PodList
expected []*extensions.ReplicaSet
test string
deployment extensions.Deployment
rsList []*extensions.ReplicaSet
podList *v1.PodList
expected []*extensions.ReplicaSet
expectedRequire []*extensions.ReplicaSet
}{
{
test: "Get old ReplicaSets",
deployment: deployment,
rsList: []*extensions.ReplicaSet{&newRS, &oldRS},
podList: &v1.PodList{
Items: []v1.Pod{
newPod,
oldPod,
},
},
expected: []*extensions.ReplicaSet{&oldRS},
test: "Get old ReplicaSets",
deployment: deployment,
rsList: []*extensions.ReplicaSet{&newRS, &oldRS},
expected: []*extensions.ReplicaSet{&oldRS},
expectedRequire: nil,
},
{
test: "Get old ReplicaSets with no new ReplicaSet",
deployment: deployment,
rsList: []*extensions.ReplicaSet{&oldRS},
podList: &v1.PodList{
Items: []v1.Pod{
oldPod,
},
},
expected: []*extensions.ReplicaSet{&oldRS},
test: "Get old ReplicaSets with no new ReplicaSet",
deployment: deployment,
rsList: []*extensions.ReplicaSet{&oldRS},
expected: []*extensions.ReplicaSet{&oldRS},
expectedRequire: nil,
},
{
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: &v1.PodList{
Items: []v1.Pod{
newPod,
oldPod,
},
},
expected: []*extensions.ReplicaSet{&oldRS, &newRS},
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},
expected: []*extensions.ReplicaSet{&oldRS, &newRS},
expectedRequire: []*extensions.ReplicaSet{&newRS},
},
{
test: "Get empty old ReplicaSets",
deployment: deployment,
rsList: []*extensions.ReplicaSet{&newRS},
podList: &v1.PodList{
Items: []v1.Pod{
newPod,
},
},
expected: []*extensions.ReplicaSet{},
test: "Get empty old ReplicaSets",
deployment: deployment,
rsList: []*extensions.ReplicaSet{&newRS},
expected: nil,
expectedRequire: nil,
},
}
for _, test := range tests {
old, _, err := FindOldReplicaSets(&test.deployment, test.rsList, test.podList)
sort.Sort(controller.ReplicaSetsByCreationTimestamp(old))
requireRS, allRS, err := FindOldReplicaSets(&test.deployment, test.rsList)
sort.Sort(controller.ReplicaSetsByCreationTimestamp(allRS))
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)
if !reflect.DeepEqual(allRS, test.expected) || err != nil {
t.Errorf("In test case %q, expected %#v, got %#v: %v", test.test, test.expected, allRS, err)
}
// RSs are getting filtered correctly by rs.spec.replicas
if !reflect.DeepEqual(requireRS, test.expectedRequire) || err != nil {
t.Errorf("In test case %q, expected %#v, got %#v: %v", test.test, test.expectedRequire, requireRS, err)
}
}
}