Skip to content

Commit

Permalink
Solves kubernetes#69376 and reverts PR kubernetes#69739
Browse files Browse the repository at this point in the history
When the ReplicaSet controller fetches a fresh version of the ReplicaSet from the server
it validates that the UID of the fresh matches the one in the cache.  If the
UID does not match, a race condition outlined in the issue must have occurred and the
controller should intepret the call to syncReplicaSet as a delete.

Reverts the merge that temporarily fixed the bug that would occasionally fail as
a result of this bug.
  • Loading branch information
runyontr committed Jan 15, 2019
1 parent f364b65 commit aa84e37
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 5 deletions.
13 changes: 12 additions & 1 deletion pkg/controller/replicaset/replica_set.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ const (
statusUpdateRetries = 1
)

var (
//ErrUIDNotMatch is returned when the ReplicaSet returned from the cluster doesn't have
// the same UID as the ReplicaSet in the cache.
ErrUIDNotMatch error = fmt.Errorf("Replicaset was found with new uid than cache")
)

// ReplicaSetController is responsible for synchronizing ReplicaSet objects stored
// in the system with actual running pods.
type ReplicaSetController struct {
Expand Down Expand Up @@ -614,6 +620,11 @@ func (rsc *ReplicaSetController) syncReplicaSet(key string) error {
// NOTE: filteredPods are pointing to objects from cache - if you need to
// modify them, you need to copy it first.
filteredPods, err = rsc.claimPods(rs, selector, filteredPods)
if isNewUIDError(err) {
klog.V(4).Infof("%v %v's UID in cache differed from server. This object was deleted.", rsc.Kind, key)
rsc.expectations.DeleteExpectations(key)
return nil
}
if err != nil {
return err
}
Expand Down Expand Up @@ -650,7 +661,7 @@ func (rsc *ReplicaSetController) claimPods(rs *apps.ReplicaSet, selector labels.
return nil, err
}
if fresh.UID != rs.UID {
return nil, fmt.Errorf("original %v %v/%v is gone: got uid %v, wanted %v", rsc.Kind, rs.Namespace, rs.Name, fresh.UID, rs.UID)
return nil, ErrUIDNotMatch
}
return fresh, nil
})
Expand Down
7 changes: 7 additions & 0 deletions pkg/controller/replicaset/replica_set_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ import (
podutil "k8s.io/kubernetes/pkg/api/v1/pod"
)

func isNewUIDError(err error) bool {
if err == ErrUIDNotMatch {
return true
}
return false
}

// updateReplicaSetStatus attempts to update the Status.Replicas of the given ReplicaSet, with a single GET/PUT retry.
func updateReplicaSetStatus(c appsclient.ReplicaSetInterface, rs *apps.ReplicaSet, newStatus apps.ReplicaSetStatus) (*apps.ReplicaSet, error) {
// This is the steady state. It happens when the ReplicaSet doesn't have any expectations, since
Expand Down
6 changes: 2 additions & 4 deletions test/cmd/apps.sh
Original file line number Diff line number Diff line change
Expand Up @@ -511,13 +511,11 @@ run_rs_tests() {
# Pre-condition: no replica set exists
kube::test::get_object_assert rs "{{range.items}}{{$id_field}}:{{end}}" ''
# Command
#TODO(mortent): Remove this workaround when ReplicaSet bug described in issue #69376 is fixed
local replicaset_name="frontend-no-cascade"
sed -r 's/^(\s*)(name\s*:\s*frontend\s*$)/\1name: '"${replicaset_name}"'/' hack/testdata/frontend-replicaset.yaml | kubectl create "${kube_flags[@]}" -f -
kubectl create -f hack/testdata/frontend-replicaset.yaml "${kube_flags[@]}"
# wait for all 3 pods to be set up
kube::test::wait_object_assert 'pods -l "tier=frontend"' "{{range.items}}{{$pod_container_name_field}}:{{end}}" 'php-redis:php-redis:php-redis:'
kube::log::status "Deleting rs"
kubectl delete rs "${replicaset_name}" "${kube_flags[@]}" --cascade=false
kubectl delete rs frontend "${kube_flags[@]}" --cascade=false
# Wait for the rs to be deleted.
kube::test::wait_object_assert rs "{{range.items}}{{$id_field}}:{{end}}" ''
# Post-condition: All 3 pods still remain from frontend replica set
Expand Down

0 comments on commit aa84e37

Please sign in to comment.