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

Clean up pre-ControllerRef compatibility logic #43942

Merged
merged 1 commit into from
Apr 13, 2017
Merged

Clean up pre-ControllerRef compatibility logic #43942

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
110 changes: 6 additions & 104 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 @@ -513,25 +513,6 @@ func GetAllReplicaSets(deployment *extensions.Deployment, c clientset.Interface)
return oldRSes, allOldRSes, newRS, nil
}

// GetAllReplicaSetsV15 is a compatibility function that emulates the behavior
// from v1.5.x (list matching objects by selector) except that it leaves out
// objects that are explicitly marked as being controlled by something else.
func GetAllReplicaSetsV15(deployment *extensions.Deployment, c clientset.Interface) ([]*extensions.ReplicaSet, []*extensions.ReplicaSet, *extensions.ReplicaSet, error) {
rsList, err := ListReplicaSetsV15(deployment, rsListFromClient(c))
if err != nil {
return nil, nil, nil, err
}
oldRSes, allOldRSes, err := FindOldReplicaSets(deployment, rsList)
if err != nil {
return nil, nil, nil, err
}
newRS, err := FindNewReplicaSet(deployment, rsList)
if err != nil {
return nil, nil, nil, err
}
return oldRSes, allOldRSes, newRS, nil
}

// GetOldReplicaSets returns the old replica sets targeted by the given Deployment; get PodList and ReplicaSetList from client interface.
// 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 GetOldReplicaSets(deployment *extensions.Deployment, c clientset.Interface) ([]*extensions.ReplicaSet, []*extensions.ReplicaSet, error) {
Expand All @@ -552,17 +533,6 @@ func GetNewReplicaSet(deployment *extensions.Deployment, c clientset.Interface)
return FindNewReplicaSet(deployment, rsList)
}

// GetNewReplicaSetV15 is a compatibility function that emulates the behavior
// from v1.5.x (list matching objects by selector) except that it leaves out
// objects that are explicitly marked as being controlled by something else.
func GetNewReplicaSetV15(deployment *extensions.Deployment, c clientset.Interface) (*extensions.ReplicaSet, error) {
rsList, err := ListReplicaSetsV15(deployment, rsListFromClient(c))
if err != nil {
return nil, err
}
return FindNewReplicaSet(deployment, rsList)
}

// rsListFromClient returns an rsListFunc that wraps the given client.
func rsListFromClient(c clientset.Interface) rsListFunc {
return func(namespace string, options metav1.ListOptions) ([]*extensions.ReplicaSet, error) {
Expand Down Expand Up @@ -617,38 +587,9 @@ func ListReplicaSets(deployment *extensions.Deployment, getRSList rsListFunc) ([
return owned, nil
}

// ListReplicaSetsV15 is a compatibility function that emulates the behavior
// from v1.5.x (list matching objects by selector) except that it leaves out
// objects that are explicitly marked as being controlled by something else.
func ListReplicaSetsV15(deployment *extensions.Deployment, getRSList rsListFunc) ([]*extensions.ReplicaSet, error) {
namespace := deployment.Namespace
selector, err := metav1.LabelSelectorAsSelector(deployment.Spec.Selector)
if err != nil {
return nil, err
}
options := metav1.ListOptions{LabelSelector: selector.String()}
all, err := getRSList(namespace, options)
if err != nil {
return nil, err
}
// Since this function maintains compatibility with v1.5, the objects we want
// do not necessarily have ControllerRefs pointing to us.
// However, we can at least avoid interfering with other controllers that do
// use ControllerRef.
filtered := make([]*extensions.ReplicaSet, 0, len(all))
for _, rs := range all {
controllerRef := controller.GetControllerOf(rs)
if controllerRef != nil && controllerRef.UID != deployment.UID {
continue
}
filtered = append(filtered, rs)
}
return filtered, nil
}

// ListReplicaSetsInternalV15 is ListReplicaSetsV15 for internalextensions.
// TODO: Remove the duplicate when call sites are updated to ListReplicaSetsV15.
func ListReplicaSetsInternalV15(deployment *internalextensions.Deployment, getRSList func(string, metav1.ListOptions) ([]*internalextensions.ReplicaSet, error)) ([]*internalextensions.ReplicaSet, error) {
// ListReplicaSetsInternal is ListReplicaSets for internalextensions.
// TODO: Remove the duplicate when call sites are updated to ListReplicaSets.
func ListReplicaSetsInternal(deployment *internalextensions.Deployment, getRSList func(string, metav1.ListOptions) ([]*internalextensions.ReplicaSet, error)) ([]*internalextensions.ReplicaSet, error) {
namespace := deployment.Namespace
selector, err := metav1.LabelSelectorAsSelector(deployment.Spec.Selector)
if err != nil {
Expand All @@ -659,17 +600,13 @@ func ListReplicaSetsInternalV15(deployment *internalextensions.Deployment, getRS
if err != nil {
return nil, err
}
// Since this function maintains compatibility with v1.5, the objects we want
// do not necessarily have ControllerRefs pointing to us.
// However, we can at least avoid interfering with other controllers that do
// use ControllerRef.
// Only include those whose ControllerRef matches the Deployment.
filtered := make([]*internalextensions.ReplicaSet, 0, len(all))
for _, rs := range all {
controllerRef := controller.GetControllerOf(rs)
if controllerRef != nil && controllerRef.UID != deployment.UID {
continue
if controllerRef != nil && controllerRef.UID == deployment.UID {
filtered = append(filtered, rs)
}
filtered = append(filtered, rs)
}
return filtered, nil
}
Expand Down Expand Up @@ -708,41 +645,6 @@ func ListPods(deployment *extensions.Deployment, rsList []*extensions.ReplicaSet
return owned, nil
}

// ListPodsV15 is a compatibility function that emulates the behavior
// from v1.5.x (list matching objects by selector) except that it leaves out
// objects that are explicitly marked as being controlled by something else.
func ListPodsV15(deployment *extensions.Deployment, rsList []*extensions.ReplicaSet, getPodList podListFunc) (*v1.PodList, error) {
namespace := deployment.Namespace
selector, err := metav1.LabelSelectorAsSelector(deployment.Spec.Selector)
if err != nil {
return nil, err
}
options := metav1.ListOptions{LabelSelector: selector.String()}
podList, err := getPodList(namespace, options)
if err != nil {
return nil, err
}
// Since this function maintains compatibility with v1.5, the objects we want
// do not necessarily have ControllerRefs pointing to one of our ReplicaSets.
// However, we can at least avoid interfering with other controllers that do
// use ControllerRef.
rsMap := make(map[types.UID]bool, len(rsList))
for _, rs := range rsList {
rsMap[rs.UID] = true
}
filtered := make([]v1.Pod, 0, len(podList.Items))
for i := range podList.Items {
pod := &podList.Items[i]
controllerRef := controller.GetControllerOf(pod)
if controllerRef != nil && !rsMap[controllerRef.UID] {
continue
}
filtered = append(filtered, *pod)
}
podList.Items = filtered
return podList, nil
}

// EqualIgnoreHash returns true if two given podTemplateSpec are equal, ignoring the diff in value of Labels[pod-template-hash]
// We ignore pod-template-hash because the hash result would be different upon podTemplateSpec API changes
// (e.g. the addition of a new field will cause the hash code to change)
Expand Down
2 changes: 1 addition & 1 deletion pkg/kubectl/history.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func (h *DeploymentHistoryViewer) ViewHistory(namespace, name string, revision i
if err != nil {
return "", fmt.Errorf("failed to retrieve deployment %s: %v", name, err)
}
_, allOldRSs, newRS, err := deploymentutil.GetAllReplicaSetsV15(deployment, versionedClient)
_, allOldRSs, newRS, err := deploymentutil.GetAllReplicaSets(deployment, versionedClient)
if err != nil {
return "", fmt.Errorf("failed to retrieve replica sets from deployment %s: %v", name, err)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kubectl/rollback.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func simpleDryRun(deployment *extensions.Deployment, c clientset.Interface, toRe
return "", fmt.Errorf("failed to convert deployment, %v", err)
}
versionedClient := versionedClientsetForDeployment(c)
_, allOldRSs, newRS, err := deploymentutil.GetAllReplicaSetsV15(externalDeployment, versionedClient)
_, allOldRSs, newRS, err := deploymentutil.GetAllReplicaSets(externalDeployment, versionedClient)
if err != nil {
return "", fmt.Errorf("failed to retrieve replica sets from deployment %s: %v", deployment.Name, err)
}
Expand Down
9 changes: 3 additions & 6 deletions pkg/kubectl/stop.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,12 +358,9 @@ func (reaper *StatefulSetReaper) Stop(namespace, name string, timeout time.Durat
errList := []error{}
for i := range podList.Items {
pod := &podList.Items[i]
// Since the client must maintain compatibility with a v1.5 server,
// we can't assume the Pods will have ControllerRefs pointing to 'ss'.
// However, we can at least avoid interfering with other controllers
// that do use ControllerRef.
controllerRef := controller.GetControllerOf(pod)
if controllerRef != nil && controllerRef.UID != ss.UID {
// Ignore Pod if it's an orphan or owned by someone else.
if controllerRef == nil || controllerRef.UID != ss.UID {
continue
}
if err := pods.Delete(pod.Name, gracePeriod); err != nil {
Expand Down Expand Up @@ -458,7 +455,7 @@ func (reaper *DeploymentReaper) Stop(namespace, name string, timeout time.Durati
}

// Stop all replica sets belonging to this Deployment.
rss, err := deploymentutil.ListReplicaSetsInternalV15(deployment,
rss, err := deploymentutil.ListReplicaSetsInternal(deployment,
func(namespace string, options metav1.ListOptions) ([]*extensions.ReplicaSet, error) {
rsList, err := reaper.rsClient.ReplicaSets(namespace).List(options)
if err != nil {
Expand Down
46 changes: 0 additions & 46 deletions pkg/kubectl/stop_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,52 +525,6 @@ func TestDeploymentStop(t *testing.T) {
"get:replicasets", "update:replicasets", "get:replicasets",
"get:replicasets", "delete:replicasets", "delete:deployments"},
},
{
Name: "Deployment with single replicaset, no ControllerRef (old cluster)",
Objs: []runtime.Object{
&deployment, // GET
&extensions.ReplicaSetList{ // LIST
Items: []extensions.ReplicaSet{
// ReplicaSet that matches but with no ControllerRef.
{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: ns,
Labels: map[string]string{"k1": "v1"},
},
Spec: extensions.ReplicaSetSpec{
Template: template,
},
},
// ReplicaSet owned by something else (should be ignored).
{
ObjectMeta: metav1.ObjectMeta{
Name: "rs2",
Namespace: ns,
Labels: map[string]string{"k1": "v1"},
OwnerReferences: []metav1.OwnerReference{
{
APIVersion: extensions.SchemeGroupVersion.String(),
Kind: "Deployment",
Name: "somethingelse",
UID: uuid.NewUUID(),
Controller: &trueVar,
},
},
},
Spec: extensions.ReplicaSetSpec{
Template: template,
},
},
},
},
},
StopError: nil,
ExpectedActions: []string{"get:deployments", "update:deployments",
"get:deployments", "list:replicasets", "get:replicasets",
"get:replicasets", "update:replicasets", "get:replicasets",
"get:replicasets", "delete:replicasets", "delete:deployments"},
},
}

for _, test := range tests {
Expand Down