Commit 1bcb3811 authored by Kubernetes Submit Queue's avatar Kubernetes Submit Queue Committed by GitHub

Merge pull request #49355 from dhilipkumars/UTDeployment

Automatic merge from submit-queue deployment: SetNewReplicaSetAnnotations() should compare revisions as numbers than strings **What this PR does / why we need it**: 1) SetNewReplicaSetAnnotations() when deployment revision annotation is copied over to RS, it performs a string comparison instead of int comparison, due to this any revision beyond 9 might not get copied. 2) Slightly improves the coverage by adding UT for Annotation related functions 3) Upgrade the test suite to use go-langs sub-test, which is very useful while investigating UT related failures. ``` --- FAIL: TestAnnotationUtils (0.00s) --- FAIL: TestAnnotationUtils/SetNewReplicaSetAnnotations (0.00s) deployment_util_test.go:1283: Revision Expected=10 Obtained=9 deployment_util_test.go:1283: Revision Expected=11 Obtained=9 deployment_util_test.go:1283: Revision Expected=12 Obtained=9 deployment_util_test.go:1283: Revision Expected=13 Obtained=9 deployment_util_test.go:1283: Revision Expected=14 Obtained=9 deployment_util_test.go:1283: Revision Expected=15 Obtained=9 deployment_util_test.go:1283: Revision Expected=16 Obtained=9 deployment_util_test.go:1283: Revision Expected=17 Obtained=9 deployment_util_test.go:1283: Revision Expected=18 Obtained=9 deployment_util_test.go:1283: Revision Expected=19 Obtained=9 deployment_util_test.go:1283: Revision Expected=20 Obtained=9 --- PASS: TestAnnotationUtils/SetReplicasAnnotations (0.00s) --- PASS: TestAnnotationUtils/IsSaturated (0.00s) ``` **Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes # **Special notes for your reviewer**: **Release note**: None ```release-note ``` cc: @kargakis
parents 72b2a03d 4967bb73
...@@ -253,7 +253,22 @@ func SetNewReplicaSetAnnotations(deployment *extensions.Deployment, newRS *exten ...@@ -253,7 +253,22 @@ func SetNewReplicaSetAnnotations(deployment *extensions.Deployment, newRS *exten
// The newRS's revision should be the greatest among all RSes. Usually, its revision number is newRevision (the max revision number // The newRS's revision should be the greatest among all RSes. Usually, its revision number is newRevision (the max revision number
// of all old RSes + 1). However, it's possible that some of the old RSes are deleted after the newRS revision being updated, and // of all old RSes + 1). However, it's possible that some of the old RSes are deleted after the newRS revision being updated, and
// newRevision becomes smaller than newRS's revision. We should only update newRS revision when it's smaller than newRevision. // newRevision becomes smaller than newRS's revision. We should only update newRS revision when it's smaller than newRevision.
if oldRevision < newRevision {
oldRevisionInt, err := strconv.ParseInt(oldRevision, 10, 64)
if err != nil {
if oldRevision != "" {
glog.Warningf("Updating replica set revision OldRevision not int %s", err)
return false
}
//If the RS annotation is empty then initialise it to 0
oldRevisionInt = 0
}
newRevisionInt, err := strconv.ParseInt(newRevision, 10, 64)
if err != nil {
glog.Warningf("Updating replica set revision NewRevision not int %s", err)
return false
}
if oldRevisionInt < newRevisionInt {
newRS.Annotations[RevisionAnnotation] = newRevision newRS.Annotations[RevisionAnnotation] = newRevision
annotationChanged = true annotationChanged = true
glog.V(4).Infof("Updating replica set %q revision to %s", newRS.Name, newRevision) glog.V(4).Infof("Updating replica set %q revision to %s", newRS.Name, newRevision)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment